Skip to content

Fix classical bit mapping in HLS pass #14597

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 4 commits into from
Jun 16, 2025
Merged

Conversation

ElePT
Copy link
Contributor

@ElePT ElePT commented Jun 13, 2025

Summary

This PR fixes a bug in HLS where classical registers were mapped to the indices in within the high level object they were defined on, instead of the corresponding index in the global circuit.

Fixes #14569 .

Details and comments

The bug was introduced in 2.0, so the fix should be backported to the stable/2.0 branch, but not stable/1.4.

ElePT and others added 3 commits June 13, 2025 18:19
@ElePT ElePT requested a review from a team as a code owner June 13, 2025 16:22
@qiskit-bot
Copy link
Collaborator

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

  • @Qiskit/terra-core

@ElePT ElePT added the stable backport potential The bug might be minimal and/or import enough to be port to stable label Jun 13, 2025
@coveralls
Copy link

coveralls commented Jun 13, 2025

Pull Request Test Coverage Report for Build 15686757882

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

  • 5 of 5 (100.0%) changed or added relevant lines in 1 file are covered.
  • 33 unchanged lines in 6 files lost coverage.
  • Overall coverage decreased (-0.03%) to 87.978%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/expr.rs 1 94.23%
crates/transpiler/src/passes/elide_permutations.rs 2 96.88%
qiskit/circuit/library/pauli_evolution.py 2 96.88%
crates/qasm2/src/lex.rs 3 92.23%
crates/circuit/src/symbol_expr.rs 7 73.64%
crates/qasm2/src/parse.rs 18 96.22%
Totals Coverage Status
Change from base Build 15635869582: -0.03%
Covered Lines: 83144
Relevant Lines: 94505

💛 - Coveralls

@mtreinish mtreinish added this to the 2.0.3 milestone Jun 13, 2025
@mtreinish mtreinish added the Changelog: Bugfix Include in the "Fixed" section of the changelog label Jun 13, 2025
Previously the cargs were being written to a Vec<usize> as an
intermediate storage, however we are just returning them directly and
don't need to allocate or change the types. This commit just works with
the slice returned from the interner directly which both simplifies the
code and should also be faster.
@mtreinish mtreinish enabled auto-merge June 16, 2025 16:47
@mtreinish
Copy link
Member

@Mergifyio backport stable/2.0

Copy link
Contributor

mergify bot commented Jun 16, 2025

backport stable/2.0

✅ Backports have been created

@mtreinish mtreinish added this pull request to the merge queue Jun 16, 2025
Merged via the queue into Qiskit:main with commit 9ec464a Jun 16, 2025
26 checks passed
mergify bot pushed a commit that referenced this pull request Jun 16, 2025
* Map classical bits from block to outer circuit

Co-authored-by: Alexander Ivrii <alexi@il.ibm.com>

* Add regression test

* Add reno

* Avoid allocating cargs

Previously the cargs were being written to a Vec<usize> as an
intermediate storage, however we are just returning them directly and
don't need to allocate or change the types. This commit just works with
the slice returned from the interner directly which both simplifies the
code and should also be faster.

---------

Co-authored-by: Alexander Ivrii <alexi@il.ibm.com>
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
(cherry picked from commit 9ec464a)
mergify bot pushed a commit that referenced this pull request Jun 16, 2025
* Map classical bits from block to outer circuit

Co-authored-by: Alexander Ivrii <alexi@il.ibm.com>

* Add regression test

* Add reno

* Avoid allocating cargs

Previously the cargs were being written to a Vec<usize> as an
intermediate storage, however we are just returning them directly and
don't need to allocate or change the types. This commit just works with
the slice returned from the interner directly which both simplifies the
code and should also be faster.

---------

Co-authored-by: Alexander Ivrii <alexi@il.ibm.com>
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
(cherry picked from commit 9ec464a)
github-merge-queue bot pushed a commit that referenced this pull request Jun 16, 2025
* Map classical bits from block to outer circuit



* Add regression test

* Add reno

* Avoid allocating cargs

Previously the cargs were being written to a Vec<usize> as an
intermediate storage, however we are just returning them directly and
don't need to allocate or change the types. This commit just works with
the slice returned from the interner directly which both simplifies the
code and should also be faster.

---------



(cherry picked from commit 9ec464a)

Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
Co-authored-by: Alexander Ivrii <alexi@il.ibm.com>
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
github-merge-queue bot pushed a commit that referenced this pull request Jun 16, 2025
* Map classical bits from block to outer circuit



* Add regression test

* Add reno

* Avoid allocating cargs

Previously the cargs were being written to a Vec<usize> as an
intermediate storage, however we are just returning them directly and
don't need to allocate or change the types. This commit just works with
the slice returned from the interner directly which both simplifies the
code and should also be faster.

---------



(cherry picked from commit 9ec464a)

Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
Co-authored-by: Alexander Ivrii <alexi@il.ibm.com>
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
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.

Results of sampling circuit with custom unitary don't match between qiskit 1.4 and qiskit 2
4 participants