Revert "Pin numpy version < 1.19.0 to unblock CI (#4599)" #4656
Conversation
This file contains 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
Since Qiskit#4613 is fixed it looks like we can remove the CI pin for the latest numpy release. Failures with numpy on test_qasm_2q_unitary haven't been reproducible locally, so there shouldn't be a blocker anymore to running tests with the latest numpy release. This reverts commit 6aca34a. Fixes Qiskit#4606
|
Oh, I guess the failure was on windows. Let me try to reproduce this locally... |
The root cause of the issue causing the failure on windows for 2q unitary decomposition of an identity matrix was a rounding issue. The U0r matrix being returned from decomp0() was [[1.00000000e+00+0.00000000e+00j 2.77555756e-17-2.77555756e-17j] [2.77555756e-17-2.77555756e-17j 1.00000000e+00+0.00000000e+00j]] instead of the expected [[1+0j + 0+0j] [0+0j + 1+0j]] this throws off later stages that use the decomposition result. This commit fixes this issue by rounding the output matrices from decomp0() to 13 decimal places (which is what is used for atol and rtol other places in the module) to avoid this type of rounding error.
1ucian0
reviewed
Jul 12, 2020
1ucian0
approved these changes
Jul 12, 2020
mtreinish
added a commit
to mtreinish/qiskit-core
that referenced
this pull request
Aug 4, 2020
In Qiskit#4656 we added rounding to the output of the decomp0 method to handle a case where differing FP precision on windows environments was causing an expected result in running two_qubit_cnot_decompose on np.eye(4) with numpy 1.19.x installed leading to a hard failure in the qasm tests. This seemed to reliably unblock testing and make unit tests work reliably. However, that original fix from Qiskit#4656 was superseded by Qiskit#4835 which was a fix for a more general issue with the reproducibility of the decompositions and reverted. Since Qiskit#4835 has merged we've been seeing an uptick in the failure rate on the same unitary qasm test that Qiskit#4656 fixed, so the change in Qiskit#4835 was not actually sufficient for the windows case. This commit restores the fix from Qiskit#4656 to unblock CI and fix the reproducability of the decompositions across systems. Fixes Qiskit#4856
kdk
pushed a commit
that referenced
this pull request
Aug 4, 2020
In #4656 we added rounding to the output of the decomp0 method to handle a case where differing FP precision on windows environments was causing an expected result in running two_qubit_cnot_decompose on np.eye(4) with numpy 1.19.x installed leading to a hard failure in the qasm tests. This seemed to reliably unblock testing and make unit tests work reliably. However, that original fix from #4656 was superseded by #4835 which was a fix for a more general issue with the reproducibility of the decompositions and reverted. Since #4835 has merged we've been seeing an uptick in the failure rate on the same unitary qasm test that #4656 fixed, so the change in #4835 was not actually sufficient for the windows case. This commit restores the fix from #4656 to unblock CI and fix the reproducability of the decompositions across systems. Fixes #4856
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
Since #4613 is fixed it looks like we can remove the CI pin for the
latest numpy release. Failures with numpy on
test_qasm_2q_unitaryonly occur on windows with the latest numpy release and look to be
caused by a rounding/precision difference in that environment. This
fixes those by rounding the results from 2q unitary decomposition
to 13 decimal places where the failure was occuring.
This reverts commit 6aca34a.
Details and comments
Fixes #4606