-
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
Finalise support for Numpy 2.0 #11999
Conversation
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 8836784161Details
💛 - Coveralls |
Fwiw, 2.0.0rc1 has been out for a while now, but I haven't had chance to loop back and actually update this yet. I will - I believe the only remaining change that needs to be done is to adapt all our implementations of There may be uses of |
This commit brings the Qiskit test suite to a passing state (with all optionals installed) with Numpy 2.0.0b1, building on previous commits that handled much of the rest of the changing requirements: - Qiskitgh-10890 - Qiskitgh-10891 - Qiskitgh-10892 - Qiskitgh-10897 - Qiskitgh-11023 Notably, this commit did not actually require a rebuild of Qiskit, despite us compiling against Numpy; it seems to happen that the C API stuff we use via `rust-numpy` (which loads the Numpy C extensions dynamically during module initialisation) hasn't changed. The main changes are: - adapting to the changed `copy=None` and `copy=False` semantics in `array` and `asarray`. - making sure all our implementers of `__array__` accept both `dtype` and `copy` arguments. Co-authored-by: Lev S. Bishop <18673315+levbishop@users.noreply.github.com>
As of Numpy 2.0, implementers of `__array__` are expected and required to have a signature def __array__(self, dtype=None, copy=None): ... In Numpys before 2.0, the `copy` argument will never be passed, and the expected signature was def __array__(self, dtype=None): ... Because of this, we have latitude to set `copy` in our implementations to anything we like if we're running against Numpy 1.x, but we should default to `copy=None` if we're running against Numpy 2.0. The semantics of the `copy` argument to `np.array` changed in Numpy 2.0. Now, `copy=False` means "raise a `ValueError` if a copy is required" and `copy=None` means "copy only if required". In Numpy 1.x, `copy=False` meant "copy only if required". In _both_ Numpy 1.x and 2.0, `ndarray.astype` takes a `copy` argument, and in both, `copy=False` means "copy only if required". In Numpy 2.0 only, `np.asarray` gained a `copy` argument with the same semantics as the `np.array` copy argument from Numpy 2.0. Further, the semantics of the `__array__` method changed in Numpy 2.0, particularly around copying. Now, Numpy will assume that it can pass `copy=True` and the implementer will handle this. If `copy=False` is given and a copy or calculation is required, then the implementer is required to raise `ValueError`. We have a few places where the `__array__` method may (or always does) calculate the array, so in all these, we must forbid `copy=False`. With all this in mind: this PR sets up all our implementers of `__array__` to either default to `copy=None` if they will never actually need to _use_ the `copy` argument within themselves (except perhaps to test if it was set by Numpy 2.0 to `False`, as Numpy 1.x will never set it), or to a compatibility shim `_numpy_compat.COPY_ONLY_IF_NEEDED` if they do naturally want to use it with those semantics. The pattern def __array__(self, dtype=None, copy=_numpy_compat.COPY_ONLY_IF_NEEDED): dtype = self._array.dtype if dtype is None else dtype return np.array(self._array, dtype=dtype, copy=copy) using `array` instead of `asarray` lets us achieve all the desired behaviour between the interactions of `dtype` and `copy` in a way that is compatible with both Numpy 1.x and 2.x.
I've updated all our implementers of The changed semantics of the def __array__(self, dtype=None, copy=_numpy_compat.COPY_ONLY_IF_NEEDED):
dtype = self._array.dtype if dtype is None else dtype
return np.array(self._array, dtype=dtype, copy=copy) pattern for objects that really are true implementers of |
import copy | ||
import copy as _copy |
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.
This kind of thing is necessary to stop pylint complaining about shadowing the outer variable in __array__
implementations, and generally felt like a decent thing to do anyway to make it clear - copy
is really a pretty common kwarg and I'm not wild that the Python standard library used such a common word as a module.
This will need backporting in some form or another to 0.46, I'd think. |
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.
Thanks for this PR, it mostly LGTM. I ran this PR locally on MacOS and ran into a couple of test failures - the first might be related to the up-to-date matplotlib
version I was using.
Test failures:
==============================
Failed 9 tests - output below:
==============================
test.python.circuit.test_equivalence.TestEquivalenceLibraryVisualization.test_equivalence_draw
----------------------------------------------------------------------------------------------
Captured traceback:
~~~~~~~~~~~~~~~~~~~
Traceback (most recent call last):
File "/Users/brandhsn/code/2024/numpy-qk/qiskit/test/python/circuit/test_equivalence.py", line 597, in test_equivalence_draw
self.assertImagesAreEqual(image, image_ref, 0.04)
File "/Users/brandhsn/code/2024/numpy-qk/qiskit/qiskit/utils/lazy_tester.py", line 165, in out
return function(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/brandhsn/code/2024/numpy-qk/qiskit/test/python/visualization/visualization.py", line 77, in assertImagesAreEqual
self.assertTrue(
File "/Users/brandhsn/.pyenv/versions/3.11.7/lib/python3.11/unittest/case.py", line 715, in assertTrue
raise self.failureException(msg)
AssertionError: False is not true : The images are different by 8.939036975018988% which is more than the allowed 4.0%
test.python.providers.test_backend_v2.TestBackendV2.test_qubit_properties
-------------------------------------------------------------------------
Captured traceback:
~~~~~~~~~~~~~~~~~~~
Traceback (most recent call last):
File "/Users/brandhsn/code/2024/numpy-qk/qiskit/test/python/providers/test_backend_v2.py", line 71, in test_qubit_properties
self.assertEqual([5487811175.818378, 5429298959.955691], [x.frequency for x in props])
File "/Users/brandhsn/.pyenv/versions/3.11.7/lib/python3.11/unittest/case.py", line 873, in assertEqual
assertion_func(first, second, msg=msg)
File "/Users/brandhsn/.pyenv/versions/3.11.7/lib/python3.11/unittest/case.py", line 1079, in assertListEqual
self.assertSequenceEqual(list1, list2, msg, seq_type=list)
File "/Users/brandhsn/.pyenv/versions/3.11.7/lib/python3.11/unittest/case.py", line 1061, in assertSequenceEqual
self.fail(msg)
File "/Users/brandhsn/.pyenv/versions/3.11.7/lib/python3.11/unittest/case.py", line 703, in fail
raise self.failureException(msg)
AssertionError: Lists differ: [5487811175.818378, 5429298959.955691] != [5487811175.8183775, 5429298959.955691]
First differing element 0:
5487811175.818378
5487811175.8183775
- [5487811175.818378, 5429298959.955691]
? ^
+ [5487811175.8183775, 5429298959.955691]
? ^^
test.python.providers.test_backend_v2.TestBackendV2.test_legacy_qubit_properties
--------------------------------------------------------------------------------
Captured traceback:
~~~~~~~~~~~~~~~~~~~
Traceback (most recent call last):
File "/Users/brandhsn/code/2024/numpy-qk/qiskit/test/python/providers/test_backend_v2.py", line 87, in test_legacy_qubit_properties
self.assertEqual([5487811175.818378, 5429298959.955691], [x.frequency for x in props])
File "/Users/brandhsn/.pyenv/versions/3.11.7/lib/python3.11/unittest/case.py", line 873, in assertEqual
assertion_func(first, second, msg=msg)
File "/Users/brandhsn/.pyenv/versions/3.11.7/lib/python3.11/unittest/case.py", line 1079, in assertListEqual
self.assertSequenceEqual(list1, list2, msg, seq_type=list)
File "/Users/brandhsn/.pyenv/versions/3.11.7/lib/python3.11/unittest/case.py", line 1061, in assertSequenceEqual
self.fail(msg)
File "/Users/brandhsn/.pyenv/versions/3.11.7/lib/python3.11/unittest/case.py", line 703, in fail
raise self.failureException(msg)
AssertionError: Lists differ: [5487811175.818378, 5429298959.955691] != [5487811175.8183775, 5429298959.955691]
First differing element 0:
5487811175.818378
5487811175.8183775
- [5487811175.818378, 5429298959.955691]
? ^
+ [5487811175.8183775, 5429298959.955691]
? ^^
test.python.utils.test_lazy_loaders.TestLazyDependencyTester.test_warns_on_internal_not_found_error
---------------------------------------------------------------------------------------------------
Captured traceback:
~~~~~~~~~~~~~~~~~~~
Traceback (most recent call last):
File "/Users/brandhsn/code/2024/numpy-qk/qiskit/test/python/utils/test_lazy_loaders.py", line 418, in test_warns_on_internal_not_found_error
with self.assertWarnsRegex(
File "/Users/brandhsn/.pyenv/versions/3.11.7/lib/python3.11/unittest/case.py", line 292, in __enter__
if getattr(v, '__warningregistry__', None):
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/brandhsn/.pyenv/versions/3.11.7/envs/numpytest/lib/python3.11/site-packages/numpy/linalg/linalg.py", line 8, in __getattr__
warnings.warn(
DeprecationWarning: The numpy.linalg.linalg has been made private and renamed to numpy.linalg._linalg. All public functions exported by it are available from numpy.linalg. Please use numpy.linalg.__warningregistry__ instead.
test.python.utils.test_deprecation.TestDeprecationDecorators.test_deprecate_arg_runtime_warning
-----------------------------------------------------------------------------------------------
Captured traceback:
~~~~~~~~~~~~~~~~~~~
Traceback (most recent call last):
File "/Users/brandhsn/code/2024/numpy-qk/qiskit/test/python/utils/test_deprecation.py", line 256, in test_deprecate_arg_runtime_warning
with self.assertWarnsRegex(DeprecationWarning, "arg1"):
File "/Users/brandhsn/.pyenv/versions/3.11.7/lib/python3.11/unittest/case.py", line 292, in __enter__
if getattr(v, '__warningregistry__', None):
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/brandhsn/.pyenv/versions/3.11.7/envs/numpytest/lib/python3.11/site-packages/numpy/linalg/linalg.py", line 8, in __getattr__
warnings.warn(
DeprecationWarning: The numpy.linalg.linalg has been made private and renamed to numpy.linalg._linalg. All public functions exported by it are available from numpy.linalg. Please use numpy.linalg.__warningregistry__ instead.
test.python.utils.test_deprecation.TestDeprecationDecorators.test_deprecate_function_runtime_warning
----------------------------------------------------------------------------------------------------
Captured traceback:
~~~~~~~~~~~~~~~~~~~
Traceback (most recent call last):
File "/Users/brandhsn/code/2024/numpy-qk/qiskit/test/python/utils/test_deprecation.py", line 359, in test_deprecate_function_runtime_warning
with self.assertWarnsRegex(DeprecationWarning, "Stop using my_func!"):
File "/Users/brandhsn/.pyenv/versions/3.11.7/lib/python3.11/unittest/case.py", line 292, in __enter__
if getattr(v, '__warningregistry__', None):
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/brandhsn/.pyenv/versions/3.11.7/envs/numpytest/lib/python3.11/site-packages/numpy/linalg/linalg.py", line 8, in __getattr__
warnings.warn(
DeprecationWarning: The numpy.linalg.linalg has been made private and renamed to numpy.linalg._linalg. All public functions exported by it are available from numpy.linalg. Please use numpy.linalg.__warningregistry__ instead.
test.python.visualization.timeline.test_generators.TestTimeslot.test_gen_bit_name
---------------------------------------------------------------------------------
Captured traceback:
~~~~~~~~~~~~~~~~~~~
Traceback (most recent call last):
File "/Users/brandhsn/code/2024/numpy-qk/qiskit/test/python/visualization/timeline/test_generators.py", line 243, in test_gen_bit_name
with self.assertWarnsRegex(UserWarning, "bits cannot be accurately named"):
File "/Users/brandhsn/.pyenv/versions/3.11.7/lib/python3.11/unittest/case.py", line 292, in __enter__
if getattr(v, '__warningregistry__', None):
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/brandhsn/.pyenv/versions/3.11.7/envs/numpytest/lib/python3.11/site-packages/numpy/linalg/linalg.py", line 8, in __getattr__
warnings.warn(
DeprecationWarning: The numpy.linalg.linalg has been made private and renamed to numpy.linalg._linalg. All public functions exported by it are available from numpy.linalg. Please use numpy.linalg.__warningregistry__ instead.
test.python.utils.test_deprecation.TestDeprecationDecorators.test_deprecate_arg_runtime_warning_variadic_args
-------------------------------------------------------------------------------------------------------------
Captured traceback:
~~~~~~~~~~~~~~~~~~~
Traceback (most recent call last):
File "/Users/brandhsn/code/2024/numpy-qk/qiskit/test/python/utils/test_deprecation.py", line 310, in test_deprecate_arg_runtime_warning_variadic_args
with self.assertWarnsRegex(DeprecationWarning, "my_kwarg"):
File "/Users/brandhsn/.pyenv/versions/3.11.7/lib/python3.11/unittest/case.py", line 292, in __enter__
if getattr(v, '__warningregistry__', None):
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/brandhsn/.pyenv/versions/3.11.7/envs/numpytest/lib/python3.11/site-packages/numpy/linalg/linalg.py", line 8, in __getattr__
warnings.warn(
DeprecationWarning: The numpy.linalg.linalg has been made private and renamed to numpy.linalg._linalg. All public functions exported by it are available from numpy.linalg. Please use numpy.linalg.__warningregistry__ instead.
test.python.utils.test_lazy_loaders.TestLazyDependencyTester.test_import_allows_attributes_failure
--------------------------------------------------------------------------------------------------
Captured traceback:
~~~~~~~~~~~~~~~~~~~
Traceback (most recent call last):
File "/Users/brandhsn/code/2024/numpy-qk/qiskit/test/python/utils/test_lazy_loaders.py", line 434, in test_import_allows_attributes_failure
with self.assertWarnsRegex(UserWarning, r"'builtins' imported, but attribute"):
File "/Users/brandhsn/.pyenv/versions/3.11.7/lib/python3.11/unittest/case.py", line 292, in __enter__
if getattr(v, '__warningregistry__', None):
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/brandhsn/.pyenv/versions/3.11.7/envs/numpytest/lib/python3.11/site-packages/numpy/linalg/linalg.py", line 8, in __getattr__
warnings.warn(
DeprecationWarning: The numpy.linalg.linalg has been made private and renamed to numpy.linalg._linalg. All public functions exported by it are available from numpy.linalg. Please use numpy.linalg.__warningregistry__ instead.
with package versions:
alabaster==0.7.16
astroid==2.14.2
asttokens==2.4.1
attrs==23.2.0
autopage==0.5.2
Babel==2.14.0
black==24.4.0
certifi==2024.2.2
charset-normalizer==3.3.2
click==8.1.7
cliff==4.6.0
cmd2==2.4.3
contourpy==1.2.1
coverage==7.4.4
cycler==0.12.1
ddt==1.7.2
decorator==5.1.1
dill==0.3.8
docutils==0.20.1
dulwich==0.22.1
executing==2.0.1
extras==1.0.0
fixtures==4.1.0
fonttools==4.51.0
hypothesis==6.100.1
idna==3.7
imagesize==1.4.1
ipython==8.23.0
iso8601==2.1.0
isort==5.13.2
jedi==0.19.1
Jinja2==3.1.3
joblib==1.4.0
kiwisolver==1.4.5
lazy-object-proxy==1.10.0
MarkupSafe==2.1.5
matplotlib==3.8.4
matplotlib-inline==0.1.7
mccabe==0.7.0
mpmath==1.3.0
mypy-extensions==1.0.0
numpy==2.0.0b1
packaging==24.0
pandas==2.2.2
parso==0.8.4
pathspec==0.12.1
pbr==6.0.0
pexpect==4.9.0
pillow==10.3.0
platformdirs==4.2.0
prettytable==3.10.0
prompt-toolkit==3.0.43
ptyprocess==0.7.0
pure-eval==0.2.2
Pygments==2.17.2
pylatexenc==2.10
pylint==2.16.2
pyparsing==3.1.2
pyperclip==1.8.2
python-dateutil==2.9.0.post0
python-subunit==1.4.4
pytz==2024.1
PyYAML==6.0.1
qiskit @ file:///Users/brandhsn/code/2024/numpy-qk/qiskit
reno==4.1.0
requests==2.31.0
ruff==0.2.0
rustworkx==0.14.2
scikit-learn==1.4.2
scipy==1.13.0
seaborn==0.13.2
six==1.16.0
snowballstemmer==2.2.0
sortedcontainers==2.4.0
Sphinx==7.1.2
sphinxcontrib-applehelp==1.0.8
sphinxcontrib-devhelp==1.0.6
sphinxcontrib-htmlhelp==2.0.5
sphinxcontrib-jsmath==1.0.1
sphinxcontrib-katex==0.9.9
sphinxcontrib-qthelp==1.0.7
sphinxcontrib-serializinghtml==1.1.10
stack-data==0.6.3
stestr==4.1.0
stevedore==5.2.0
symengine==0.11.0
sympy==1.12
testtools==2.7.1
threadpoolctl==3.4.0
tokenize-rt==5.2.0
tomlkit==0.12.4
traitlets==5.14.3
typing_extensions==4.11.0
tzdata==2024.1
urllib3==2.2.1
voluptuous==0.14.2
wcwidth==0.2.13
wrapt==1.16.0
Any suggestions here?
@@ -117,8 +119,9 @@ def with_gate_array(base_array): | |||
nonwritable = numpy.array(base_array, dtype=numpy.complex128) | |||
nonwritable.setflags(write=False) | |||
|
|||
def __array__(_self, dtype=None): | |||
return numpy.asarray(nonwritable, dtype=dtype) | |||
def __array__(_self, dtype=None, copy=_numpy_compat.COPY_ONLY_IF_NEEDED): |
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.
On the previous __array__
, the default of copy
is None
, here it is _numpy_compat.COPY_ONLY_IF_NEEDED
is that fine?
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 we pass the copy
argument directly to a Numpy function, we need to use the compatibility variable so that if a user has Numpy 1.x installed, they won't get errors (passing copy=None
is an error in Numpy 1.x). If we don't use the copy
argument, or we only look at it ourselves, it's fine to use None
. Numpy 1.x never sets the copy
argument to __array__
(because it doesn't exist in the protocol in 1.x).
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.
Oh, I just realised that the previous __array__
you're referring to is the one in qiskit.circuit
. That's actually just an example in the middle of a docstring, so matching the now-suggested Numpy API exactly is probably best.
The image similarity one is unrelated - it's a font issue on macOS. I had a very brief go at fixing it about a year ago, then just gave up because I couldn't be bothered and I've never seen those particular Numpy warnings before, but |
Sounds good to me.
The same warnings/errors also occur for |
Hmm, the warnings/errors thing is very weird - I can't reproduce it at all, even with Python 3.11.9 and your exact same set of packages, also on macOS. The only thing I could imagine is different about our environments is if you've got an ARM Mac (I've still got an old x86 one). At any rate, the warning is actually just being triggered by the Python standard-library test harness that does some pretty aggressive |
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! thanks!
Summary
This commit brings the Qiskit test suite to a passing state (with all optionals installed) with Numpy 2.0.0b1, building on previous commits that handled much of the rest of the changing requirements:
numpy.lib.scimath
with preferrednumpy.emath
#10892NormalizeRXAngles
#11023Notably, this commit did not actually require a rebuild of Qiskit, despite us compiling against Numpy; it seems to happen that the C API stuff we use via
rust-numpy
(which loads the Numpy C extensions dynamically during module initialisation) hasn't changed.Close #10889
Details and comments
The Numpy team are still planning a 2.0.0rc1, so there's still the possibility of a couple more changes to come, but given the tiny magnitude of changes since a few months ago when I last checked the compatibility, it's looking pretty promising for us.
I think there's potentially more latent issues in Qiskit around the changed behaviour of
numpy.array(copy=False)
- that used to mean "only copy if required", but in Numpy 2.0 that means "do not copy", andcopy=None
means "only copy if required". There aren't any instances of us hitting that in the test suite beyond the one fixed in this PR, but I'd be surprised if there aren't more ways we could hit it.Marking "on hold" until all our core dependencies have released compatible versions. Mostly, that's Numpy itself and Scipy, but we may want to consider Matplotlib and Pandas (used transitively by seaborn) since they're used in the
visualization
extra, which is very commonly installed.I verified this locally by building installing
numpy==2.0.0b1
, then building Scipy, scikit-learn, matplotlib and pandas from source against Numpy 2.0.I'll probably need to make a separate PR to verify the 0.46.x series works with Numpy 2.0 too.