Skip to content
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

Update sources for compatibility with NumPy 2 #6706

Open
pavoljuhas opened this issue Aug 20, 2024 · 7 comments
Open

Update sources for compatibility with NumPy 2 #6706

pavoljuhas opened this issue Aug 20, 2024 · 7 comments
Assignees
Labels
kind/health For CI/testing/release process/refactoring/technical debt items triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add

Comments

@pavoljuhas
Copy link
Collaborator

Description of the issue

Let us get our sources compatible with recently released NumPy 2

Ref: https://numpy.org/doc/stable/numpy_2_0_migration_guide.html

Cirq version

1.5.0.dev at d94c457

@pavoljuhas pavoljuhas added the kind/health For CI/testing/release process/refactoring/technical debt items label Aug 20, 2024
@pavoljuhas pavoljuhas self-assigned this Aug 20, 2024
@NoureldinYosri NoureldinYosri added the triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add label Aug 21, 2024
@pavoljuhas pavoljuhas assigned mhucka and unassigned pavoljuhas Sep 4, 2024
@mhucka
Copy link
Member

mhucka commented Sep 9, 2024

Starting some notes.

A find-grep through the Cirq codebase reveals slightly over 500 files containing import numpy or from numpy.

find . ! -path '*/.git/*' \
       ! -path '*/__pycache__/*' \
       ! -path '*/.*_cache/*' \
       ! -name '*.pyc' \
       -type f -print0 \
       | xargs -0 -e egrep -n "import numpy|from numpy"

Doing a find-grep to search for instances of "np." or "numpy." finds a total of approximately 8000 lines. This includes Jupyter notebooks, JSON files, and Markdown files.

In addition to the NumPy migration guide, there is also a separate document with NumPy 2.0-specific advice.

NumPy version 2.1 has some additional changes since the 2.0 release, so we need to look at those too: NumPy 2.1.0 Release Notes.

Past related issues and PRs:

@pavoljuhas
Copy link
Collaborator Author

The primarily goal here is to fix all numpy API calls that were removed or changed in 2.0,
so that cirq does not break for the users if they upgrade to numpy-2.
Per migration guide, this can be done automatically using ruff check --select NPY201 --fix .
The changes should be then checked manually. I think ruff can also work with ipynb files,
but I am not sure if it needs an extra option or does so by default in ruff check ....

Finally, we should run check/pytest and verify if there are any numpy-related deprecation warnings.

We should perhaps also add ruff to our CI checks following https://numpy.org/doc/stable/numpy_2_0_migration_guide.html#ruff-plugin, to prevent numpy-1 APIs creeping back in. That should however go to a separate PR.

PS: I have quickly glanced over 2.1.0 release notes and there does not seem much relevant for our code.
At any rate, it should not have any breaking changes as a minor release.

@mhucka
Copy link
Member

mhucka commented Sep 9, 2024

(Continuing notes.)

Incredibly enough,

ruff check --verbose --select NPY201 --fix .

does not find anything to change. (This was so surprising that I tried a number of variations to make sure ruff was actually checking things!)

The Python requirements file that installs NumPy is cirq-core/requirements.txt. It limits the versions to the 1.x line. Changing the specification to use NumPy 2.1 leads to problems with SciPy; increasing its version specification (to 1.1) on the same requirements file gets us further, and then only two more things lead to installation failures:

  • cirq-rigetti
  • cirq-core/cirq/contrib/quimb

Temporarily moving them out of the way (and removing references to them in a couple of places) then allows installation to proceed and check/pytest to be run.

@mhucka
Copy link
Member

mhucka commented Sep 9, 2024

If we temporarily ignore the modules mentioned above, the check/pytest errors that remain are almost all due to a difference in how Numpy 2 prints representations of scalars. It now prints, e.g., "np.int64(3)" instead of "3". This in turn shows up as a difference in how circuit diagrams are printed, which causes tests to fail.

Numpy 2 has a configuration option to control that behavior (https://numpy.org/doc/stable/reference/generated/numpy.set_printoptions.html#numpy-set-printoptions), and setting numpy.set_printoptions(legacy="1.25") makes check/pytest succeed.

We need to change the rendering code for circuits so that it prints these np data values as strings (without setting a global configuration parameter).

@mhucka
Copy link
Member

mhucka commented Sep 11, 2024

One of the changes in NumPy 2 is to type promotion. The table at https://numpy.org/neps/nep-0050-scalar-promotion.html#t6 summarizes the changes. Particularly notable for Cirq is cases involving uint8. For example, uint8(100) + 200 previously produced a unit16 value but now results in a unit8 value and an overflow warning (not error). In Cirq, simulator measurement result values are uint8's, and in some places, arrays of values are summed. This leads to overflows if the sum > 128.

It would not be appropriate to change measurement values to be larger than uint8, so the most correct solution is probably to make sure that where values are summed or otherwise numerically manipulated, unit16 or larger values are ensured.

Additional info:

@mhucka
Copy link
Member

mhucka commented Sep 15, 2024

A conflict exists in the versions of NumPy needed by second-level dependencies.

Quimb requires Numba, and while Quimb itself is compatible with NumPy 2.1, if you have NumPy 2.1 installed, a pip install of Numba 0.60 (the latest version) results in pip uninstalling Numpy 2.1 and installing 2.0.2 instead. Trying to force NumPy 2.1 seems to result in pip installation errors involving llvm. (The Numba sources don't seem to require NumPy 2.0 per se, so I think what's happening is Numba requires something else, and that leads to conflicts.)

Now, Cirq tests pass even with NumPy 2.0.x, so one option is to accept NumPy > 2.0 and < 2.1 for now, request Numba to support NumPy 2.1, and update our requirements files later. This would also align with the internal Google codebase, because the upcoming internal transition to NumPy 2 is going to use version 2.0.1.

@mhucka
Copy link
Member

mhucka commented Sep 16, 2024

All tests pass locally for me now, except for cirq-rigetti. Unfortunately, PyQuil (used by cirq-rigetti) limits the version of NumPy to < 2.0, so it is impossible to satisfy the requirement for importing it for cirq-rigetti.

This cirq-rigetti issue is also the reason the CI checks are currently failing for this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/health For CI/testing/release process/refactoring/technical debt items triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add
Projects
None yet
Development

No branches or pull requests

3 participants