Add slow_test as it's no longer part of Qiskit 1.0 #141
Conversation
woodsp-ibm
added
the
stable backport potential
Pull Request Test Coverage Report for Build 7788009998
💛 - Coveralls |
|
Since I did this it seems there is a new failure in the 1.0 Monitor dues to the change in signature of |
|
Hmm., now its running tests, which it did not get that far on nightly CI, it seems there are a whole bunch of failures that did not happen a couple of days ago when I first pushed this PR. |
|
Seems the failures are due Estimator now only supporting SparsePauliOp in 1.0. So where tests had a SparsePauliOp and then done to_matrix() on this to create a matrix op, and tested with both, I dropped the conversion and test using that. In one place a different operator was created to check re-use, I converted that to a SparsePauliOp. Typehints on the algorithms for the operators supported ideally need updating since its no longer BaseOperator as per what the Estimator supported. Maybe this can be revisited along with #136. |
| @@ -48,7 +48,7 @@ class WhatAmI(Gate): | |||
| def __init__(self, angle): | |||
| super().__init__(name="whatami", num_qubits=2, params=[angle]) | |||
|
|
|||
| def inverse(self): | |||
| def inverse(self, annotated: bool = False): | |||
There was a problem hiding this comment.
This probably deserves a comment that can be removed once Qiskit 0.46 reaches eol.
There was a problem hiding this comment.
After a quick look at Qiskit/qiskit#11593 it looks like that argument is intended to live in 1.0+, so the comment wouldn't be necessary, right?
There was a problem hiding this comment.
That argument is new in 1.0 - and while adding it now is compatible with 0.46 as it was without causes lint to fail as you can see in the CI jobs which have recently been merged where the monitor jobs, on 1.0, failed (they are not required to pass to merge). Ie the extra param needs to stay there - it just happens to be backwards compatible with 0.46 doing it this way - as you can see it all passes - the main CI which is done using 0.46 presently and the monitor jobs using Qsikit main.
************* Module test.time_evolvers.test_pvqd
test/time_evolvers/test_pvqd.py:51:4: W0221: Number of parameters was 2 in 'Instruction.inverse' and is now 1 in overriding 'WhatAmI.inverse' method (arguments-differ)
And this is just a test case, not main code
There was a problem hiding this comment.
Ok, then I think that the comment is not necessary here.
I don't know whats the right thing here. If you use 0.46 then the hints are correct as the primitives support a wider set. If you use the new 1.0 then its really only then its just SparsePauliOp. Since most people arguably just use SparsePauliOp - the apps all do - maybe it does not matter too much. If you had code that used one of the other operators it would still work with 0.46 albeit that it raises a deprecation warning rather than fail as it does if you use 1.0. If we changed to SparsePauliOp only then maybe mypy would complain on your code with the other operators. |
(cherry picked from commit 2245288) # Conflicts: # test/optimizers/test_optimizer_aqgd.py
) Co-authored-by: Steve Wood <[email protected]> Co-authored-by: Elena Peña Tapia <[email protected]>
Summary
slow_testused to be part ofqiskit.testbut the wholetestfolder was removed for Qiskit 1.0. This adds a copy of that slow_test decorator locally.Additionally this updates other units testing for failures that started since this was created - see comments below for more detail.
Details and comments
The Monitor jobs, that run against Qiskit main branch, started failing last night after the removal.