Add fast path to Parameter.assign
#10549
Merged
Conversation
This file contains hidden or 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 specialises `Parameter.assign` to more efficiently handle parameter assignment via `QuantumCircuit.assign_parameters`. The assignment can either be of `self`, and therefore should just return the value lifted to a `ParameterExpression`, or it's not of `self`, in which case we need to raise an error to match the superclass implementation. The overhead of all the checks that go on during the regular `ParameterExpression.assign` (delegating to `subs` or `bind`) are a sizeable chunk of the cost of assignment in PEC-style circuits.
|
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 5742085677
💛 - Coveralls |
Cryoris
reviewed
Aug 16, 2023
Comment on lines
+103
to
+104
| # This is the `super().bind` case, where we're required to return a `ParameterExpression`, | ||
| # so we need to lift the given value to a symbolic expression. |
There was a problem hiding this comment.
Why do we need to lift the value to a symbolic expression? In the final circuit keeping only the float would be even better than having the parameter expression, but I expect there's some type requirement I'm missing...
There was a problem hiding this comment.
In this PR at least, because that would be a different logical change. I think there's a few issues discussing your topic elsewhere on the Terra repo, though.
Pull Request Test Coverage Report for Build 6036367887
💛 - Coveralls |
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 specialises
Parameter.assignto more efficiently handle parameter assignment viaQuantumCircuit.assign_parameters. The assignment can either be ofself, and therefore should just return the value lifted to aParameterExpression, or it's not ofself, in which case we need to raise an error to match the superclass implementation.The overhead of all the checks that go on during the regular
ParameterExpression.assign(delegating tosubsorbind) are a sizeable chunk of the cost of assignment in PEC-style circuits.Details and comments
For the example in #10548 (and building on that branch), this PR takes the runtime from 3.51(6)s down to 2.66(2)s, which is better than the looped private-method performance of Terra 0.24.2.
For the case of inplace binding the whole set of parameters in one go, given in array form, this takes the runtime down from 2.86(2)s down to 1.79(2)s.
cc: @chriseclectic (again).