-
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
Pending-deprecate library circuits with gate representations #13232
Conversation
One or more of the following people are relevant to this code:
|
which is pending deprecation, but that's fine as GroverOperator will also pend deprecation in the same timeframe for 1.3
Pull Request Test Coverage Report for Build 11688246257Warning: 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.
In principle the PR looks good, I only wonder if the test with the warning filter was an oversight, and left a few tiny comments.
qc = QuantumCircuit(q, name=self.name) | ||
for i in range(self.num_qubits): | ||
for j in range(i + 1, self.num_qubits): | ||
# if theta was just a single angle, use that, otherwise use the correct index |
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.
# if theta was just a single angle, use that, otherwise use the correct index | |
# if theta is just a single angle, use that, otherwise use the correct index |
@@ -196,10 +196,12 @@ def __init__(self, *_args, **_kwargs): | |||
module=r"qiskit\..*", | |||
message=r".*precision loss in QFT.*", | |||
) | |||
warnings.filterwarnings("ignore", category=PendingDeprecationWarning) |
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.
Do we fail on pending deprecation warnings? I am surprised, I thought we didn't.
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.
Ah, no we don't but here we have a context that catches all warnings and checks that none are emitted (to check that there is no precision loss) -- so we have to filter out the pending deprecation warning 🙂
qc = QuantumCircuit(q, name=self.name) | ||
for i in range(self.num_qubits): | ||
for j in range(i + 1, self.num_qubits): | ||
# if theta was just a single angle, use that, otherwise use the correct index | ||
theta = thetas if isinstance(thetas, ParameterValueType) else thetas[i][j] |
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.
I believe that it's this line that is currently failing in the unit tests. The error raised is:
TypeError: Subscripted generics cannot be used with class and instance checks
Maybe you can check the shape of the input instead?
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.
Allright, LGTM!
Summary
Part of #13046: Library circuits that already have a gate/instruction version are pending-deprecated. These includes:
QFT
Diagonal
Permutation
GMS
Details and comments
As part of this cleanup,
LinearFunction
is streamlined with all other gates by deprecating the customsynthesize
method in favor of callingdefinition
(or compiling the circuit).No reno as all deprecation are only pending.