-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Remove the classical_function
module and all tweedledum
-related code
#13815
Remove the classical_function
module and all tweedledum
-related code
#13815
Conversation
One or more of the following people are relevant to this code:
|
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. I only wonder if we should keep some of the removed tests?
@@ -1371,105 +1370,5 @@ def test_cswap_reg_reg_inv(self): | |||
self.assertEqual(instruction_set[1].qubits, (self.qr[1], self.qr2[1], self.qr3[1])) | |||
self.assertEqual(instruction_set[2].operation.params, []) | |||
|
|||
|
|||
class TestStandardMethods(QiskitTestCase): |
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.
can we do these tests with PhaseOracle w/o tweedledum ?
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.
Yes, actually tweedledum was not really needed there. I returned the test.
return a and b and not c | ||
|
||
circuit = grover_oracle.synth(registerless=False) | ||
self.assertEqual(str(circuit_drawer(circuit, output="text", initial_state=True)), expected) |
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.
can we still do this tests with grover or phase oracle w/o tweedledum ?
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.
It doesn't make sense, since this is a test for registerless synthesis, which is currently not supported by the oracles.
|
classical_function
module and all tweedledum
-related codeclassical_function
module and all tweedledum
-related code
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.
LGTM, just one note on the reno 🙂 (also black is failing)
releasenotes/notes/remove_classical_function-7d9057bc5c77d268.yaml
Outdated
Show resolved
Hide resolved
…yaml Co-authored-by: Julien Gacon <gaconju@gmail.com>
Pull Request Test Coverage Report for Build 13551023854Details
💛 - 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 Qiskit#13815 the usage of the tweedledum library was removed from Qiskit. The classes that used this functionality was deprecated in 1.4.0, but the optional library checker was not deprecated. These optionals checkers are part of the public API and would have needed a deprecation before removal as there may be downstream users of the checker. This commit restores the checker, but alters the docstring to make it clear that tweedledum isn't used by Qiskit anymore. If we want to remove the checker in 3.0 we can deprecate it in 2.1.0 or another release in the 2.x series. Although, there is basically no real cost for keeping it around so it's probably not worth the effort.
In #13815 the usage of the tweedledum library was removed from Qiskit. The classes that used this functionality was deprecated in 1.4.0, but the optional library checker was not deprecated. These optionals checkers are part of the public API and would have needed a deprecation before removal as there may be downstream users of the checker. This commit restores the checker, but alters the docstring to make it clear that tweedledum isn't used by Qiskit anymore. If we want to remove the checker in 3.0 we can deprecate it in 2.1.0 or another release in the 2.x series. Although, there is basically no real cost for keeping it around so it's probably not worth the effort.
Summary
Removes from qiskit the
classical_function
module and all code that relies on thetweedledum
library.Closes #13755
Details and comments
The
tweedledum
library is not compatible with new versions of Python, so reliance on it has to be dropped in Qiskit 2.0. Since theclassical_function
module basically servers as a qiskit wrapper fortweedledum
functionality, we remove it from qiskit, after its deprecation in #13786.To preserve the main functionality of
classical_function
, i.e. the ability to generate phase and bit-flip oracles from boolean expressions, we add a simple,tweedledum
-independent implementation in #13769, which is placed inqiskit.synthesis.boolean
instead of theclassical_function
module. This PR relies on #13769 being merged, otherwise the existing tests forPhaseOracle
will not pass.