Deprecate the unit and duration attributes #13224
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
This commit deprecates the unit and duration attributes for QuantumCircuit and Instruction. These attributes will be removed in Qiskit 2.0 as they're not needed anymore and are adding a lot of complexity to the circuit data model as they're mutable state and extra memory slots that we need to keep around. The better model for tracking instruction duration is in the Target as it's inherently a property of the backend running the instructions. For the unittests this commit globally ignores the deprecation warning raised by this commit because internally we access the now deprecated access quite frequently as we need to check it when adding instructions to circuits to populate the rust data model.
mtreinish
added
Changelog: Deprecation
|
One or more of the following people are relevant to this code:
|
Pull Request Test Coverage Report for Build 11050184582Details
💛 - Coveralls |
ElePT
reviewed
Sep 26, 2024
releasenotes/notes/deprecate-unit-duration-48b76c957bac5691.yaml
Outdated
Show resolved
Hide resolved
releasenotes/notes/deprecate-unit-duration-48b76c957bac5691.yaml
Outdated
Show resolved
Hide resolved
releasenotes/notes/deprecate-unit-duration-48b76c957bac5691.yaml
Outdated
Show resolved
Hide resolved
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
ElePT
approved these changes
Sep 26, 2024
cameron-d28
pushed a commit
to cameron-d28/qiskit
that referenced
this pull request
Sep 26, 2024
* Deprecate the unit and duration attributes This commit deprecates the unit and duration attributes for QuantumCircuit and Instruction. These attributes will be removed in Qiskit 2.0 as they're not needed anymore and are adding a lot of complexity to the circuit data model as they're mutable state and extra memory slots that we need to keep around. The better model for tracking instruction duration is in the Target as it's inherently a property of the backend running the instructions. For the unittests this commit globally ignores the deprecation warning raised by this commit because internally we access the now deprecated access quite frequently as we need to check it when adding instructions to circuits to populate the rust data model. * Apply suggestions from code review Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com> --------- Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
mtreinish
added a commit
to mtreinish/qiskit-core
that referenced
this pull request
Sep 30, 2024
The unit and duration attributes of QuantumCircuit and Instruction were introduced in Qiskit#13224 however the way the PR went about this was a bit to abbreviated and it had several downsides. The most noticable was that it introduced a significant runtime regression as the rust internals were accessing the deprecated getter. This is fixed so that the rust code doesn't try to access any deprecated attributes. However, more importantly there were several edge cases not accoutned for in the PR. Mainly, DAGCircuit's matching attribute were not deprecated, but then also specific use cases of the attributes were not addressed. The primary driver of the per instruction duration and unit were the timeline visualization which didn't have a method to function without these data attributes. This commit addresses this by adding a new target argument that is used to provide a backend's target which is the source of truth for durations in a post unit/duration `Instruction`. The other notable edge case is the `Delay` instruction which will need to retain the duration and unit attributes as the fields are integral for that instruction. The `Delay` is updated to handle them explicitly outside of the `Instrution` data model to avoid complications around the deprecation.
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 7, 2024
* Improve deprecation of unit and duration The unit and duration attributes of QuantumCircuit and Instruction were introduced in #13224 however the way the PR went about this was a bit to abbreviated and it had several downsides. The most noticable was that it introduced a significant runtime regression as the rust internals were accessing the deprecated getter. This is fixed so that the rust code doesn't try to access any deprecated attributes. However, more importantly there were several edge cases not accoutned for in the PR. Mainly, DAGCircuit's matching attribute were not deprecated, but then also specific use cases of the attributes were not addressed. The primary driver of the per instruction duration and unit were the timeline visualization which didn't have a method to function without these data attributes. This commit addresses this by adding a new target argument that is used to provide a backend's target which is the source of truth for durations in a post unit/duration `Instruction`. The other notable edge case is the `Delay` instruction which will need to retain the duration and unit attributes as the fields are integral for that instruction. The `Delay` is updated to handle them explicitly outside of the `Instrution` data model to avoid complications around the deprecation. * Apply suggestions from code review Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com> * Fix lint * Mention the target in constructor deprecation message --------- Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
ElePT
added a commit
to ElePT/qiskit
that referenced
this pull request
Oct 9, 2024
* Deprecate the unit and duration attributes This commit deprecates the unit and duration attributes for QuantumCircuit and Instruction. These attributes will be removed in Qiskit 2.0 as they're not needed anymore and are adding a lot of complexity to the circuit data model as they're mutable state and extra memory slots that we need to keep around. The better model for tracking instruction duration is in the Target as it's inherently a property of the backend running the instructions. For the unittests this commit globally ignores the deprecation warning raised by this commit because internally we access the now deprecated access quite frequently as we need to check it when adding instructions to circuits to populate the rust data model. * Apply suggestions from code review Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com> --------- Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.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.
Summary
This commit deprecates the unit and duration attributes for QuantumCircuit and Instruction. These attributes will be removed in Qiskit 2.0 as they're not needed anymore and are adding a lot of complexity to the circuit data model as they're mutable state and extra memory slots that we need to keep around. The better model for tracking instruction duration is in the Target as it's inherently a property of the backend running the instructions.
For the unittests this commit globally ignores the deprecation warning raised by this commit because internally we access the now deprecated access quite frequently as we need to check it when adding instructions to circuits to populate the rust data model.
Details and comments