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.

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

Remove dead code branch from Optimize1qDecomposition Python code #13188

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

jakelishman
Copy link
Member

Summary

Since 8e5fab6 (gh-12959), this branch has been dead, and the Rust method alluded to in it (compute_error_one_qubit_sequence) was removed. This is occasionally - but for some reason not always - detected by pylint, and it's a true error so should be removed.

In fact, almost all of the Python code in Optimize1qDecomposition is dead from the perspective of that transpiler pass. However, Optimize1qCommutation has no concept of safe API boundaries, and is in fact still using all of these methods (and the default UnitarySynthesis plugin does a little too). A follow-up commit can reorganise the code to fix this, but this commit is intended to be mergeable faster, to unblock a couple of PRs that are triggering the pylint failure.

Details and comments

This is the blocker on #13147 at the moment, and has occasionally appeared in other PRs. I'll follow this with a proper refactoring commit to sort out how the split between Optimize1qDecomposition, DefaultUnitarySynthesisPlugin and Optimize1qCommutation should work.

Since 8e5fab6 (Qiskitgh-12959), this branch has been dead, and the Rust
method alluded to in it (`compute_error_one_qubit_sequence`) was
removed.  This is occasionally - but for some reason not always -
detected by `pylint`, and it's a true error so should be removed.

In fact, almost all of the Python code in `Optimize1qDecomposition` is
dead from the perspective of that transpiler pass.  However,
`Optimize1qCommutation` has no concept of safe API boundaries, and is in
fact still using all of these methods (and the default
`UnitarySynthesis` plugin does a little too).  A follow-up commit can
reorganise the code to fix this, but this commit is intended to be
mergeable faster, to unblock a couple of PRs that are triggering the
pylint failure.
@jakelishman jakelishman added type: qa Issues and PRs that relate to testing and code quality Changelog: None Do not include in changelog mod: transpiler Issues and PRs related to Transpiler labels Sep 19, 2024
@jakelishman jakelishman added this to the 1.3.0 milestone Sep 19, 2024
@jakelishman jakelishman requested a review from a team as a code owner September 19, 2024 14:00
@qiskit-bot
Copy link
Collaborator

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

  • @Qiskit/terra-core

@mtreinish mtreinish enabled auto-merge September 19, 2024 14:04
@coveralls
Copy link

Pull Request Test Coverage Report for Build 10942556611

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • 12 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.01%) to 88.829%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 6 91.73%
crates/qasm2/src/parse.rs 6 97.61%
Totals Coverage Status
Change from base Build 10931978471: -0.01%
Covered Lines: 73479
Relevant Lines: 82720

💛 - Coveralls

@mtreinish mtreinish added this pull request to the merge queue Sep 19, 2024
Merged via the queue into Qiskit:main with commit 3100993 Sep 19, 2024
15 checks passed
@jakelishman jakelishman deleted the optimize1qdecomposition-lint branch September 19, 2024 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog mod: transpiler Issues and PRs related to Transpiler type: qa Issues and PRs that relate to testing and code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants