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

Fix circuit drawers mutating input circuits #8741

Merged
merged 1 commit into from
Sep 14, 2022

Conversation

jakelishman
Copy link
Member

Summary

The wire management in the circuit drawers could accidentally mutate the qubits in the input circuit. The root cause of this is because QuantumCircuit.qubits (and .clbits) are properties that expose the internal, mutable lists that back them directly. This is a larger failing of the QuantumCircuit API, but one that is slightly tricky to walk back without backwards-incompatible changes or performance regressions.

Details and comments

Fix #8739. No release note because this regression was introduced by #8173, which hasn't been released yet.

The wire management in the circuit drawers could accidentally mutate the
qubits in the input circuit.  The root cause of this is because
`QuantumCircuit.qubits` (and `.clbits`) are properties that expose the
internal, mutable lists that back them directly.  This is a larger
failing of the `QuantumCircuit` API, but one that is slightly tricky to
walk back without backwards-incompatible changes or performance
regressions.
@jakelishman jakelishman added Changelog: None Do not include in changelog mod: visualization qiskit.visualization labels Sep 14, 2022
@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

Copy link
Contributor

@taalexander taalexander left a comment

Choose a reason for hiding this comment

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

Can confirm this fixes my issue.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 3049410146

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.004%) to 84.334%

Files with Coverage Reduction New Missed Lines %
qiskit/pulse/library/waveform.py 3 89.36%
Totals Coverage Status
Change from base Build 3048445128: -0.004%
Covered Lines: 57957
Relevant Lines: 68723

💛 - Coveralls

@mergify mergify bot merged commit a4c8df0 into Qiskit:main Sep 14, 2022
@jakelishman jakelishman deleted the stop-drawers-mutating-circuits branch September 21, 2022 22:56
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: visualization qiskit.visualization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drawing a circuit with idle_wires=False causes the circuit to break in future draw/qpy events
4 participants