Skip to content

Fix behavior of HLS plugins after preserve_order=False #14539

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

Merged
merged 1 commit into from
Jun 5, 2025

Conversation

ElePT
Copy link
Contributor

@ElePT ElePT commented Jun 5, 2025

Summary

The HLS plugins for PauliEvolutionGates were modifying the original circuit's gate.synthesis.preserve_order attribute, which would result in an unexpected behavior of the pass manager when run multiple times. The bug can be fixed by restoring the property in the plugin after it being used in .synthesize.

Details and comments

@ElePT ElePT requested a review from a team as a code owner June 5, 2025 13:02
@qiskit-bot
Copy link
Collaborator

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

  • @Qiskit/terra-core

@ElePT ElePT added 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 labels Jun 5, 2025
@coveralls
Copy link

Pull Request Test Coverage Report for Build 15467749371

Details

  • 8 of 8 (100.0%) changed or added relevant lines in 1 file are covered.
  • 8 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.02%) to 88.058%

Files with Coverage Reduction New Missed Lines %
crates/transpiler/src/passes/unitary_synthesis.rs 1 94.57%
crates/circuit/src/symbol_expr.rs 2 75.39%
crates/qasm2/src/lex.rs 5 92.23%
Totals Coverage Status
Change from base Build 15464547777: 0.02%
Covered Lines: 82901
Relevant Lines: 94144

💛 - Coveralls

@ElePT ElePT added this to the 2.0.3 milestone Jun 5, 2025
Copy link
Member

@alexanderivrii alexanderivrii left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix! I am wondering how this bug was discovered in the first place.

@alexanderivrii alexanderivrii added this pull request to the merge queue Jun 5, 2025
@ElePT
Copy link
Contributor Author

ElePT commented Jun 5, 2025

@alexanderivrii I was drafting an exercise for a workshop trying different synthesis strategies and I noticed that preserve_order=False would change the results of the whole exercise even with a fixed seed.

Merged via the queue into Qiskit:main with commit 2982004 Jun 5, 2025
28 checks passed
mergify bot pushed a commit that referenced this pull request Jun 5, 2025
(cherry picked from commit 2982004)
github-merge-queue bot pushed a commit that referenced this pull request Jun 5, 2025
(cherry picked from commit 2982004)

Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
@1ucian0
Copy link
Member

1ucian0 commented Jun 6, 2025

Is this affecting 1.4?

@ElePT
Copy link
Contributor Author

ElePT commented Jun 6, 2025

@Mergifyio backport stable/1.4

Copy link
Contributor

mergify bot commented Jun 6, 2025

backport stable/1.4

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Jun 6, 2025
(cherry picked from commit 2982004)

# Conflicts:
#	qiskit/transpiler/passes/synthesis/hls_plugins.py
github-merge-queue bot pushed a commit that referenced this pull request Jun 12, 2025
…4539) (#14547)

* Fix HLS preserve order (#14539)

(cherry picked from commit 2982004)

# Conflicts:
#	qiskit/transpiler/passes/synthesis/hls_plugins.py

* Update hls_plugins.py

---------

Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
rahaman-quantum pushed a commit to rahaman-quantum/qiskit that referenced this pull request Jun 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

5 participants