-
Notifications
You must be signed in to change notification settings - Fork 71
Replaced GroverOperator usage by grover_operator #253
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
base: main
Are you sure you want to change the base?
Conversation
Pull Request Test Coverage Report for Build 17765604336Details
💛 - 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.
The grover_operator
function has only been introduced in Qiskit v1.3, but the qiskit-algorithms
repo is currently stated to be compatible with qiskit>=1.0
. To avoid a breaking change and remain compatible with Qiskit 1.0 we would have to treat both GroverOperator
and grover_operator
.
|
||
# construct the grover operator | ||
return GroverOperator(oracle, self.state_preparation) | ||
return grover_operator_builder(oracle, self.state_preparation) |
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.
We could use qiskit.__version__
to check whether to return GroverOperator
or grover_operator
here. This should also be documented then
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 order not to introduce a breaking change then, is it OK if the function uses GroverOperator
if the Qiskit version is 2.1.2
or older, and otherwise uses grover_operator
? That way, it would prevent a breaking change from users upgrading qiskit-algorithms
without upgrading qiskit
.
This of course assume that the next qiskit-algorithms
release is done before Qiskit 2.2.0
is out! Or we could test for < 3.0
, since this is when GroverOperator
is removed if I'm not mistaken. What are your views on this?
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.
Good questions -- my preference would be to use the grover_operator
function whenever possible (i.e. qiskit>=1.3
) since it's more efficient than the GroverOperator
. But maybe it's safest to just sell this as bugfix and start using grover_operator
with qiskit>=2.1
since the GroverOperator
was deprecated in 2.1.
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 went for the latter and thus updated the release note to categorize this as a bug fix. Does that work for you?
Summary
Fixes #76 and fixes #241. The
AmplificationProblem
andEstimationProblem
classes now use thegrover_operator
function instead of the deprecatedGroverOperator
class to build their respective oracle. The tests have been adapted to test for both, and the tutorial uses the new function.The access to attributes of
GroverOperator
has been replaced with that ofAmplificationProblem
.