Fix tests failing against Qiskit main due to change in delay behavior #1509
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A long-standing bug in qpy leads to the units of delay instructions being overwritten to `dt`. See: Qiskit/qiskit#10662 Some experiments test that their circuits are round-trip serializable through qpy (perhaps not as necessary as it once was as the room for sticking arbitrary Python into circuits is reduced with the move to Rust). The T1 and Tphi circuits have delays and so were affected by the qpy bug. However, there is a second bug related to delays which canceled this bug out for those tests. `Delay.__eq__` does not check that the units are the same for the two delay instructions, so the loss of the original unit did not matter for the tests. Following this change: Qiskit/qiskit#13486 Comparing circuits with delays now includes the delay units in the determination of equality. `Delay.__eq__` itself still does not consider the unit but some part of adding the delay to a circuit converts it into a Rust object that does check the unit. Here the tests are given a backend with a `dt` value so that the experiments will generate circuits with delays in `dt` and so still have delays with the same `dt` after the qpy roundtrip.
|
I opened Qiskit/qiskit#13812 for the delay comparison bug. It looks like there is some other unrelated bug with fake backends to address as well. |
wshanks
added a commit
to wshanks/qiskit-experiments
that referenced
this pull request
Feb 17, 2025
…qiskit-community#1509) A long-standing bug in qpy leads to the units of delay instructions being overwritten to `dt`. See: Qiskit/qiskit#10662 Some experiments test that their circuits are round-trip serializable through qpy (perhaps not as necessary as it once was as the room for sticking arbitrary Python into circuits is reduced with the move to Rust). The T1 and Tphi circuits have delays and so were affected by the qpy bug. However, there is a second bug related to delays which canceled this bug out for those tests. `Delay.__eq__` does not check that the units are the same for the two delay instructions, so the loss of the original unit did not matter for the tests. Following this change: Qiskit/qiskit#13486 Comparing circuits with delays now includes the delay units in the determination of equality. `Delay.__eq__` itself still does not consider the unit but some part of adding the delay to a circuit converts it into a Rust object that does check the unit. Here the tests are given a backend with a `dt` value so that the experiments will generate circuits with delays in `dt` and so still have delays with the same `dt` after the qpy roundtrip.
wshanks
added
the
backport stable potential
mergify bot
pushed a commit
that referenced
this pull request
Feb 25, 2025
…#1509) A long-standing bug in qpy leads to the units of delay instructions being overwritten to `dt`. See: Qiskit/qiskit#10662 Some experiments test that their circuits are round-trip serializable through qpy (perhaps not as necessary as it once was as the room for sticking arbitrary Python into circuits is reduced with the move to Rust). The T1 and Tphi circuits have delays and so were affected by the qpy bug. However, there is a second bug related to delays which canceled this bug out for those tests. `Delay.__eq__` does not check that the units are the same for the two delay instructions, so the loss of the original unit did not matter for the tests. Following this change: Qiskit/qiskit#13486 Comparing circuits with delays now includes the delay units in the determination of equality. `Delay.__eq__` itself still does not consider the unit but some part of adding the delay to a circuit converts it into a Rust object that does check the unit. Here the tests are given a backend with a `dt` value so that the experiments will generate circuits with delays in `dt` and so still have delays with the same `dt` after the qpy roundtrip. (cherry picked from commit ad2341d)
mergify bot
pushed a commit
that referenced
this pull request
Feb 25, 2025
…#1509) A long-standing bug in qpy leads to the units of delay instructions being overwritten to `dt`. See: Qiskit/qiskit#10662 Some experiments test that their circuits are round-trip serializable through qpy (perhaps not as necessary as it once was as the room for sticking arbitrary Python into circuits is reduced with the move to Rust). The T1 and Tphi circuits have delays and so were affected by the qpy bug. However, there is a second bug related to delays which canceled this bug out for those tests. `Delay.__eq__` does not check that the units are the same for the two delay instructions, so the loss of the original unit did not matter for the tests. Following this change: Qiskit/qiskit#13486 Comparing circuits with delays now includes the delay units in the determination of equality. `Delay.__eq__` itself still does not consider the unit but some part of adding the delay to a circuit converts it into a Rust object that does check the unit. Here the tests are given a backend with a `dt` value so that the experiments will generate circuits with delays in `dt` and so still have delays with the same `dt` after the qpy roundtrip. (cherry picked from commit ad2341d)
wshanks
added a commit
that referenced
this pull request
Feb 25, 2025
… (backport #1509) (#1520) A long-standing bug in qpy leads to the units of delay instructions being overwritten to `dt`. See: Qiskit/qiskit#10662 Some experiments test that their circuits are round-trip serializable through qpy (perhaps not as necessary as it once was as the room for sticking arbitrary Python into circuits is reduced with the move to Rust). The T1 and Tphi circuits have delays and so were affected by the qpy bug. However, there is a second bug related to delays which canceled this bug out for those tests. `Delay.__eq__` does not check that the units are the same for the two delay instructions, so the loss of the original unit did not matter for the tests. Following this change: Qiskit/qiskit#13486 Comparing circuits with delays now includes the delay units in the determination of equality. `Delay.__eq__` itself still does not consider the unit but some part of adding the delay to a circuit converts it into a Rust object that does check the unit. Here the tests are given a backend with a `dt` value so that the experiments will generate circuits with delays in `dt` and so still have delays with the same `dt` after the qpy roundtrip.<hr>This is an automatic backport of pull request #1509 done by [Mergify](https://mergify.com). Co-authored-by: Will Shanks <willshanks@us.ibm.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A long-standing bug in qpy leads to the units of delay instructions being overwritten to
dt. See:Qiskit/qiskit#10662
Some experiments test that their circuits are round-trip serializable through qpy (perhaps not as necessary as it once was as the room for sticking arbitrary Python into circuits is reduced with the move to Rust). The T1 and Tphi circuits have delays and so were affected by the qpy bug. However, there is a second bug related to delays which canceled this bug out for those tests.
Delay.__eq__does not check that the units are the same for the two delay instructions, so the loss of the original unit did not matter for the tests. Following this change:Qiskit/qiskit#13486
Comparing circuits with delays now includes the delay units in the determination of equality.
Delay.__eq__itself still does not consider the unit but some part of adding the delay to a circuit converts it into a Rust object that does check the unit.Here the tests are given a backend with a
dtvalue so that the experiments will generate circuits with delays indtand so still have delays with the samedtafter the qpy roundtrip.