-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Added circuit start table to the QPY file header #14571
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
Added circuit start table to the QPY file header #14571
Conversation
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 following people are relevant to this code:
|
Pull Request Test Coverage Report for Build 15735763409Warning: 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
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for doing this work!
Can you also update the format description in qiskit/qpy/__init__.py
?
Thank you @kevinhartman for the suggestions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just a few more small comments.
Co-authored-by: Kevin Hartman <kevin@hart.mn>
Co-authored-by: Kevin Hartman <kevin@hart.mn>
Thanks a lot @kevinhartman. Please let me know if any other changes are required from my end 🙂. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few minor suggestions/tweaks.
Co-authored-by: Kevin Hartman <kevin@hart.mn>
Co-authored-by: Kevin Hartman <kevin@hart.mn>
Co-authored-by: Kevin Hartman <kevin@hart.mn>
Thank you so much @kevinhartman for all of your suggestions. |
releasenotes/notes/qpy-circuit-start-table-ac5d23c239bc4c68.yaml
Outdated
Show resolved
Hide resolved
… update. Co-authored-by: Kevin Hartman <kevin@hart.mn>
… entry (instead on [0]) Co-authored-by: Kevin Hartman <kevin@hart.mn>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me.
Thanks for working on this and for the updates, @velocityraptor7085!
Summary
Details and comments
The approach used includes the following changes:
CIRCUIT_TABLE_ENTRY_PACK
andCIRCUIT_TABLE_ENTRY_SIZE
to represent anuint64_t
(in theformats.py
file).QPY_VERSION
to16
(incommon.py
).dump
function to do the following (if the version is >= 16):number_of_circuits * CIRCUIT_TABLE_ENTRY_SIZE
bytes initially.byte_offsets
for each circuit before serializing them (viabinary_io.write_circuit()
).byte_offsets
for each circuit after serializing every circuit.load
function to do the following (if the version is >= 16):program_offsets
after reading the header.binary_io.read_circuit()
).