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

Propagate DAGCircuit.name in ApplyLayout #13910

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jakelishman
Copy link
Member

This has always been absent, but transpile masked the problem by assigning the output circuit names by overwriting the output_names input kwarg if it wasn't set. generate_preset_pass_manager didn't have the same logic, so it was observable that ApplyLayout didn't propagate the name.

We could have added similar name-propagation logic to generate_preset_pass_manager, but really the bug is in ApplyLayout; passes should propagate the metadata.

Summary

Details and comments

This has always been absent, but `transpile` masked the problem by
assigning the output circuit names by overwriting the `output_names`
input kwarg if it wasn't set.  `generate_preset_pass_manager` didn't
have the same logic, so it was observable that `ApplyLayout` didn't
propagate the name.

We could have added similar name-propagation logic to
`generate_preset_pass_manager`, but really the bug is in `ApplyLayout`;
passes _should_ propagate the metadata.
@jakelishman jakelishman 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 mod: transpiler Issues and PRs related to Transpiler labels Feb 21, 2025
@jakelishman jakelishman added this to the 1.4.1 milestone Feb 21, 2025
@jakelishman jakelishman requested a review from a team as a code owner February 21, 2025 22:33
@qiskit-bot
Copy link
Collaborator

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

  • @Qiskit/terra-core

@coveralls
Copy link

Pull Request Test Coverage Report for Build 13466371567

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • 5 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.005%) to 88.099%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/expr.rs 1 94.23%
crates/qasm2/src/lex.rs 4 91.98%
Totals Coverage Status
Change from base Build 13463422620: 0.005%
Covered Lines: 78383
Relevant Lines: 88971

💛 - Coveralls

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 mod: transpiler Issues and PRs related to Transpiler 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.

3 participants