Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix some bugs in loading Solovay Kitaev decompositions #12579

Merged
merged 5 commits into from
Jun 26, 2024
Merged

Fix some bugs in loading Solovay Kitaev decompositions #12579

merged 5 commits into from
Jun 26, 2024

Conversation

Cryoris
Copy link
Contributor

@Cryoris Cryoris commented Jun 14, 2024 •
edited
Loading

Summary

Fixes #12576 and some other small bugs in storing & loading basic approximations for the Solovay Kitaev algorithm.

Details and comments

@Cryoris
fix store & load
1f1b884 
- fix access via .item()
- fix storing of global phase
- fix storing ofgate sequence labels
@qiskit-bot
Copy link
Collaborator

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core

@Cryoris Cryoris added stable backport potential The bug might be minimal and/or import enough to be port to stable Changelog: Bugfix Include in the "Fixed" section of the changelog labels Jun 14, 2024
@Cryoris Cryoris mentioned this pull request Jun 14, 2024
Closed
@coveralls
Copy link

coveralls commented Jun 14, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9516403218

Details

  • 9 of 10 (90.0%) changed or added relevant lines in 2 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.06%) to 89.775%
Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/synthesis/discrete_basis/solovay_kitaev.py 8 9 88.89%
Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 2 93.38%
Totals Coverage Status
Change from base Build 9514566193: 0.06%
Covered Lines: 63438
Relevant Lines: 70663
💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 17, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9546816461

Details

  • 9 of 10 (90.0%) changed or added relevant lines in 2 files are covered.
  • 5 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.03%) to 89.746%
Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/synthesis/discrete_basis/solovay_kitaev.py 8 9 88.89%
Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 5 92.37%
Totals Coverage Status
Change from base Build 9514566193: 0.03%
Covered Lines: 63417
Relevant Lines: 70663
💛 - Coveralls

ElePT
ElePT reviewed Jun 21, 2024
View reviewed changes
releasenotes/notes/fix-sk-load-from-file-02c6eabbbd7fcda3.yaml Outdated Show resolved Hide resolved
qiskit/synthesis/discrete_basis/solovay_kitaev.py
if isinstance(matrix_and_phase, tuple):
matrix, global_phase = matrix_and_phase
else:
matrix, global_phase = matrix_and_phase, 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is setting the global phase to zero equivalent to the previous code which didn't set the global phase in the sequence?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly 🙂

@coveralls
Copy link

coveralls commented Jun 21, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9614416465

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 6 of 7 (85.71%) changed or added relevant lines in 2 files are covered.
  • 809 unchanged lines in 51 files lost coverage.
  • Overall coverage increased (+0.05%) to 89.758%
Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/synthesis/discrete_basis/solovay_kitaev.py 5 6 83.33%
Files with Coverage Reduction New Missed Lines %
qiskit/converters/circuit_to_gate.py 1 97.44%
qiskit/dagcircuit/dagdepnode.py 1 76.81%
qiskit/compiler/assembler.py 1 96.13%
qiskit/converters/circuit_to_instruction.py 1 98.21%
qiskit/assembler/assemble_schedules.py 1 98.35%
qiskit/qasm2/export.py 1 98.5%
qiskit/pulse/configuration.py 2 96.08%
qiskit/circuit/library/hamiltonian_gate.py 2 83.33%
qiskit/transpiler/passes/optimization/template_matching/template_substitution.py 2 92.67%
qiskit/visualization/state_visualization.py 3 58.02%
Totals Coverage Status
Change from base Build 9514566193: 0.05%
Covered Lines: 63527
Relevant Lines: 70776
💛 - Coveralls

ElePT
ElePT approved these changes Jun 25, 2024
View reviewed changes
Copy link
Contributor

@ElePT ElePT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ElePT ElePT added this pull request to the merge queue Jun 25, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 25, 2024
@Cryoris Cryoris added this pull request to the merge queue Jun 26, 2024
@Cryoris
Copy link
Contributor Author

Cryoris commented Jun 26, 2024

@Mergifyio backport stable/0.46 stable/1.1

@mergify Mergify
Copy link
Contributor

mergify bot commented Jun 26, 2024 •
edited
Loading

backport stable/0.46 stable/1.1

✅ Backports have been created

Merged via the queue into Qiskit:main with commit 6974b45 Jun 26, 2024
15 checks passed
mergify bot pushed a commit that referenced this pull request Jun 26, 2024
@Cryoris @mergify
Fix some bugs in loading Solovay Kitaev decompositions (#12579)
0d320c9 
* fix store & load

- fix access via .item()
- fix storing of global phase
- fix storing ofgate sequence labels

* undangle a dangling print

* fix import order

* Update releasenotes/notes/fix-sk-load-from-file-02c6eabbbd7fcda3.yaml

Co-authored-by: Elena Peña Tapia <[email protected]>

---------

Co-authored-by: Elena Peña Tapia <[email protected]>
(cherry picked from commit 6974b45)
mergify bot pushed a commit that referenced this pull request Jun 26, 2024
@Cryoris @mergify
Fix some bugs in loading Solovay Kitaev decompositions (#12579)
e3ee2f3 
* fix store & load

- fix access via .item()
- fix storing of global phase
- fix storing ofgate sequence labels

* undangle a dangling print

* fix import order

* Update releasenotes/notes/fix-sk-load-from-file-02c6eabbbd7fcda3.yaml

Co-authored-by: Elena Peña Tapia <[email protected]>

---------

Co-authored-by: Elena Peña Tapia <[email protected]>
(cherry picked from commit 6974b45)
This was referenced Jun 26, 2024
Merged
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jun 26, 2024
@mergify @Cryoris
Fix some bugs in loading Solovay Kitaev decompositions (#12579) (#12664)
f8c9dcd 
* fix store & load

- fix access via .item()
- fix storing of global phase
- fix storing ofgate sequence labels

* undangle a dangling print

* fix import order

* Update releasenotes/notes/fix-sk-load-from-file-02c6eabbbd7fcda3.yaml

Co-authored-by: Elena Peña Tapia <[email protected]>

---------

Co-authored-by: Elena Peña Tapia <[email protected]>
(cherry picked from commit 6974b45)

Co-authored-by: Julien Gacon <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Jun 26, 2024
@mergify @Cryoris
Fix some bugs in loading Solovay Kitaev decompositions (#12579) (#12665)
6cc5796 
* fix store & load

- fix access via .item()
- fix storing of global phase
- fix storing ofgate sequence labels

* undangle a dangling print

* fix import order

* Update releasenotes/notes/fix-sk-load-from-file-02c6eabbbd7fcda3.yaml

Co-authored-by: Elena Peña Tapia <[email protected]>

---------

Co-authored-by: Elena Peña Tapia <[email protected]>
(cherry picked from commit 6974b45)

Co-authored-by: Julien Gacon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ElePT ElePT ElePT approved these changes

@alexanderivrii alexanderivrii Awaiting requested review from alexanderivrii alexanderivrii is a code owner

@ShellyGarion ShellyGarion Awaiting requested review from ShellyGarion ShellyGarion is a code owner

Assignees
No one assigned
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Solovay Kitaev Decomposition doesn't correctly load saved approximations from file
4 participants
@Cryoris @qiskit-bot @coveralls @ElePT