-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 QPY support for control flow instructions (backport #7584) #7606
Merged
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
* Fix QPY support for control flow instructions This commit fixes support for using qpy serialization with circuits that contained control flow instructions. Previously, if you attempted to serialize a circuit that contained control flow instructions it would error as qpy didn't support embedding the circuit blocks as parameters for the instructions. This has been fixed and other missing pieces for fully reconstructing circuits with control flow were added. This requires bumping the version string to v4 because to accurately reconstruct control flow circuits we needed to be able to represent registers that were only partially present in a circuit fully. V4 also adds a representation for ranges which didn't exist in earlier versions. Fixes #7583 * Fix handling of standalone register bits in circuit This commit fixes the failing tests by correcting the handling of standalone register bits that are part of a circuit (without their register). Previosuly, if the circuit was solely composed of register bits but the circuit didn't contain that register, qpy deserialization would incorrectly add the register to the circuit. This corrects this by having the deserialization function treat a register that's not in circuit in the same manner as if it were an out of order register and add each bit one at a time. * Add test for for loop with iterator * Add backwards compat tests for control flow * Finish release notes * Fix docs build * Apply suggestions from code review Co-authored-by: Jake Lishman <jake@binhbar.com> * Add handling for ContinueLoopOp too * Cleanup note about register index array type change * Use unique doc reference names for qpy module * Update qpy version 3 ref in release note * Switch tuple to support any qpy type instead of just ints This commit changes the tuple formatting to work with any parameter type that qpy currently supports, not just ints which is what for loop currently uses a tuple for. This should provide some future proofing as it enables tuples of other types, which is being considered in the future, to work without modification. * Update another broken sphinx ref * Fix last dangling broken sphinx ref * Fix yet another dangling sphinx ref * Fix failing test * Set fixed Python hash seed in qpy compat tests This commit updates the qpy compat test runner script to set a fixed hash seed. The control flow builders internally use sets which means that to get consistent ordering between python processes we need to ensure the hash seeds are the same. Without this the argument order might differ between creating the circuit for qpy serialization and reconstructing the circuit in another process by deserializing that qpy data. Setting a fixed hash seed between processes ensures this type of failure won't happen as the set order will be consistent between multiple runs of Python. * Update qiskit/circuit/qpy_serialization.py Co-authored-by: Jake Lishman <jake@binhbar.com> * Fix docstring for TUPLE payload This commit fixes an oversight in earlier commits where the documentation for the TUPLE payload wasn't actually finished wasn't really coherent. This commit finishes the documentation to explain how a tuple is serialized in qpy. * Add test case using nested tuples * Directly construct WhileLoopOp in qpy compat tests In an earlier commit we set a fixed (but still randomly generated) python hash seed to force multiple invocations of python to use the same hash seed. This was done in order to fix a non-deterministic failure we were seeing with the while loop circuit. The while loop context builder uses a set internally and is sensitive to argument ordering. However, just hash seed randomization doesn't fix the issue in practice because the difference isn't between set order strictly, but also that set order matches the circuit insertion order for what gets written in QPY. To address this and make the tests work stably this commit switches away from using the builder interface and instead manually constructs the while loop and appends it to the circuit with a fixed argument order. This should hopefully make the compat tests run consistently and fix the occasional failures we're seeing in CI. * Fix lint * Add test case for serializing an empty tuple instruction parameter * Avoid builder interface for qpy compat tests Just as was changed for the while loop generator function in the qpy compat tests this commit updates the other control flow example circuits to not use the builder interface. The same non-determinism in the ordering of arguments applies to all the control flow instructions. So to avoid spurious failures we should explicitly set a arg order and not use the builder interface. Co-authored-by: Jake Lishman <jake@binhbar.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 05cbc44)
jakelishman
approved these changes
Feb 2, 2022
jakelishman
added
automerge
Changelog: Bugfix
Include in the "Fixed" section of the changelog
labels
Feb 2, 2022
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.
This is an automatic backport of pull request #7584 done by Mergify.
Mergify commands and options
More conditions and actions can be found in the documentation.
You can also trigger Mergify actions by commenting on this pull request:
@Mergifyio refresh
will re-evaluate the rules@Mergifyio rebase
will rebase this PR on its base branch@Mergifyio update
will merge the base branch into this PR@Mergifyio backport <destination>
will backport this PR on<destination>
branchAdditionally, on Mergify dashboard you can:
Finally, you can contact us on https://mergify.com