-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fix #6706 – Update sources for compatibility with NumPy 2 #6724
Merged
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
mhucka
requested review from
wcourtney,
vtomole,
verult and
a team
as code owners
September 11, 2024 03:07
mhucka
changed the title
WIP: fix #6706 – Update sources for compatibility with NumPy 2
Fix #6706 – Update sources for compatibility with NumPy 2
Sep 16, 2024
In NumPy 2 (and possibly earlier versions), lines 478-480 produced a deprecation warning: ``` DeprecationWarning: In future, it will be an error for 'np.bool' scalars to be interpreted as an index ``` This warning is somewhat misleading: it _is_ the case that Booleans are involved, but they are not being used as indices. The fields `rs`, `xs`, and `zs` of CliffordTableau as defined in file `cirq-core/cirq/qis/clifford_tableau.py` have type `Optional[np.ndarray]`, and the values in the ndarray have NumPy type `bool` in practice. The protocol buffer version of CliffordTableau defined in file `cirq-google/cirq_google/api/v2/program_pb2.pyi` defines those fields as `collections.abc.Iterable[builtins.bool]`. At first blush, you might think they're arrays of Booleans in both cases, but unfortunately, there's a wrinkle: Python defines its built-in `bool` type as being derived from `int` (see PEP 285), while NumPy explicitly does _not_ drive its `bool` from its integer class (see <https://numpy.org/doc/2.0/reference/arrays.scalars.html#numpy.bool>). The warning about converting `np.bool` to index values (i.e., integers) probably arises when the `np.bool` values in the ndarray are coerced into Python Booleans. At first, I thought the obvious solution would be to use `np.asarray` to convert the values to `builtins.bool`, but this did not work: ``` >>> import numpy as np >>> import builtins >>> arr = np.array([True, False], dtype=np.bool) >>> arr array([ True, False]) >>> type(arr[0]) <class 'numpy.bool'> >>> newarr = np.asarray(arr, dtype=builtins.bool) >>> newarr array([ True, False]) >>> type(newarr[0]) <class 'numpy.bool'> ``` They still end up being NumPy bools. Some other variations on this approach all failed to produce proper Python Booleans. In the end, what worked was to use `map()` to apply `builtins.bool` to every value in the incoming arrays. This may not be as efficient as possible; a possible optimization for the future is to look for a more efficient way to cast the types, or avoid having to do it at all.
The NumPy 2 Migration Guide [explicitly recommends changing](https://numpy.org/doc/stable/numpy_2_0_migration_guide.html#adapting-to-changes-in-the-copy-keyword) constructs of the form ```python np.array(state, copy=False) ``` to ```python np.asarray(state) ```
NumPy 2 raises deprecation warnings about converting an ndarray with dimension > 0 of values likle `[[0]]` to a scalar value like `0`. The solution is to get the value using `.item()`.
mhucka
force-pushed
the
mhucka-numpy2-migration
branch
from
September 19, 2024 17:19
1e0816f
to
7bfa7ea
Compare
This adds a new option to make NumPy warn about data promotion behavior that has changed in NumPy 2. This new promotion can lead to lower precision results when working with floating-point scalars, and errors or overflows when working with integer scalars. Invoking pytest with `--warn-numpy-data-promotion` will cause warnings warnings to be emitted when dissimilar data types are used in an operation in such a way that NumPy ends up changing the data type of the result value. Although this new option for Cirq's pytest code is most useful during Cirq's migration to NumPy 2, the flag will likely remain for some time afterwards too, because developers will undoubtely need time to adjust to the new NumPy behavior. For more information about the NumPy warning enabled by this option, see <https://numpy.org/doc/2.0/numpy_2_0_migration_guide.html#changes-to-numpy-data-type-promotion>.
This updates the minimum NumPy version requirement to 2.0, and updates a few other packages to versions that are compatible with NumPy 2.0. Note: NumPy 2.1 was released 3 weeks ago, but at this time, Cirq can only upgrade to 2.0. This is due to the facts that (a) Google's internal codebase is moving to NumPy 2.0.2, and not 2.1 yet; and (b) conflicts arise with some other packages used by Cirq if NumPy 2.1 is required right now. These considerations will no doubt change in the near future, at which time we can update Cirq to use NumPy 2.1 or higher.
One of the changes in NumPy 2 is to the [behavior of type promotion](https://numpy.org/devdocs/numpy_2_0_migration_guide.html#changes-to-numpy-data-type-promotion). A possible negative impact of the changes is that some operations involving scalar types can lead to lower precision, or even overflow. For example, `uint8(100) + 200` previously (in Numpy < 2.0) produced a `unit16` value, but now results in a `unit8` value and an overflow _warning_ (not error). This can have an impact on Cirq. For example, 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 in cases like this, the proper solution is probably to make sure that where values are summed or otherwise numerically manipulated, `uint16` or larger values are ensured. NumPy 2 offers a new option (`np._set_promotion_state("weak_and_warn")`) to produce warnings where data types are changed. Commit 6cf50eb adds a new command-line to our pytest framework, such that running ```bash check/pytest --warn-numpy-data-promotion ``` will turn on this NumPy setting. Running `check/pytest` with this option enabled revealed quite a lot of warnings. The present commit changes code in places where those warnings were raised, in an effort to eliminate as many of them as possible. It is certainly the case that not all of the type promotion warnings are meaningful. Unfortunately, I found it sometimes difficult to be sure of which ones _are_ meaningful, in part because Cirq's code has many layers and uses ndarrays a lot, and understanding the impact of a type demotion (say, from `float64` to `float32`) was difficult for me to do. In view of this, I wanted to err on the side of caution and try to avoid losses of precision. The principles followed in the changes are roughly the following: * Don't worry about warnings about changes from `complex64` to `complex128`, as this obviously does not reduce precision. * If a warning involves an operation using an ndarray, change the code to try to get the actual data type of the data elements in the array rather than use a specific data type. This is the reason some of the changes look like the following, where it reaches into an ndarray to get the dtype of an element and then later uses the `.type()` method of that dtype to cast the value of something else: ```python dtype = args.target_tensor.flat[0].dtype ..... args.target_tensor[subspace] *= dtype.type(x) ``` * In cases where the above was not possible, or where it was obvious what the type must always be, the changes add type casts with explicit types like `complex(x)` or `np.float64(x)`. It is likely that this approach resulted in some unnecessary up-promotion of values and may have impacted run-time performance. Some simple overall timing of `check/pytest` did not reveal a glaring negative impact of the changes, but that doesn't mean real applications won't be impacted. Perhaps a future review can evaluate whether speedups are possible.
This commit for one file implements a minor refactoring of 3 test functions to make them all use similar idioms (for greater ease of reading) and to address the same NumPy 2 data promotion warnings for the remaining files in commit eeeabef.
Mypy flagged a couple of the previous data type declaration changes as being incompatible with expected types. Changing them to satisfy mypy did not affect Numpy data type promotion warnings.
* Sync with new API for checking device family in qcs-sdk-python, Ref: rigetti/qcs-sdk-rust#463 in isa.pyi * Require qcs-sdk-python-0.20.1 which introduced the new family API Fixes quantumlib#6732
Pytest was happy with the previous approach to declaring the value types in a couple of expressions, but mypy was not. This new version satisfies both.
As a consequence of [NEP 51](https://numpy.org/neps/nep-0051-scalar-representation.html#nep51), the string representation of scalar numbers changed in NumPy 2 to include type information. This affected printing Cirq circuit diagrams: instead seeing numbers like 1.5, you would see `np.float64(1.5)` and similar. The solution is to avoid getting the repr output of NumPy scalars directly, and instead doing `.item()` on them before passing them to `format()` or other string-producing functions.
The recent changes support NumPy 2 (as long as cirq-rigetti is removed manually), but they don't require NumPy 2. We can maintain compatibility with Numpy 1.x.
Bumps [serve-static](https://github.com/expressjs/serve-static) and [express](https://github.com/expressjs/express). These dependencies needed to be updated together. Updates `serve-static` from 1.15.0 to 1.16.2 - [Release notes](https://github.com/expressjs/serve-static/releases) - [Changelog](https://github.com/expressjs/serve-static/blob/v1.16.2/HISTORY.md) - [Commits](expressjs/serve-static@v1.15.0...v1.16.2) Updates `express` from 4.19.2 to 4.21.0 - [Release notes](https://github.com/expressjs/express/releases) - [Changelog](https://github.com/expressjs/express/blob/4.21.0/History.md) - [Commits](expressjs/express@4.19.2...4.21.0) --- updated-dependencies: - dependency-name: serve-static dependency-type: indirect - dependency-name: express dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Michael Hucka <mhucka@caltech.edu>
mhucka
force-pushed
the
mhucka-numpy2-migration
branch
from
September 19, 2024 17:35
7bfa7ea
to
5caaebc
Compare
In the current version of pytest (8.3.3) with the pytest-asyncio module version 0.24.0, we see the following warnings at the beginning of a pytest run: ``` warnings.warn(PytestDeprecationWarning(_DEFAULT_FIXTURE_LOOP_SCOPE_UNSET)) ..../lib/python3.10/site-packages/pytest_asyncio/plugin.py:208: PytestDeprecationWarning: The configuration option "asyncio_default_fixture_loop_scope" is unset. The event loop scope for asynchronous fixtures will default to the fixture caching scope. Future versions of pytest-asyncio will default the loop scope for asynchronous fixtures to function scope. Set the default fixture loop scope explicitly in order to avoid unexpected behavior in the future. Valid fixture loop scopes are: "function", "class", "module", "package", "session" ``` A [currently-open issue and discussion over in the pytest-asyncio repo](pytest-dev/pytest-asyncio#924) suggests that this is an undesired side-effect of a recent change in pytest-asyncio and is not actually a significant warning. Moreover, the discussion suggests the warning will be removed or changed in the future. In the meantime, the warning is confusing because it makes it sound like something is wrong. This simple PR silences the warning by adding a suitable pytest init flag to `pyproject.toml'.
Flagged by pylint.
mhucka
force-pushed
the
mhucka-numpy2-migration
branch
from
September 20, 2024 02:33
30681a0
to
0daa139
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6724 +/- ##
==========================================
- Coverage 97.83% 97.83% -0.01%
==========================================
Files 1077 1077
Lines 92524 92537 +13
==========================================
+ Hits 90523 90535 +12
- Misses 2001 2002 +1 ☔ View full report in Codecov by Sentry. |
In commit eb98361 I added the import of kahypar, which (at least at the time) appeared to have been imported by Quimb. Double-checking this import in clean environments reveals that in fact, nothing depends on kahypar. Taking it out via a separate commit because right now this package is causing our GitHub actions for commit checks to fail, and I want to leave a record of what caused the failures and how they were resolved.
mhucka
force-pushed
the
mhucka-numpy2-migration
branch
from
September 20, 2024 13:17
9315f7f
to
6f8b8a7
Compare
Instead of summing int8 ones count them.
Handle np2 numeric types without outputting their dtype.
This also enables the hash_from_pickle_test.py with numpy-2.
* allow numpy-1.24 which is still in the NEP-29 support window per https://numpy.org/neps/nep-0029-deprecation_policy.html * require `scipy~=1.8` as scipy-1.8 is the first version that has wheels for Python 3.10
pavoljuhas
approved these changes
Sep 20, 2024
harry-phasecraft
pushed a commit
to PhaseCraft/Cirq
that referenced
this pull request
Oct 31, 2024
…uantumlib#6724) * Explicitly convert NumPy ndarray of np.bool to Python bool In NumPy 2 (and possibly earlier versions), lines 478-480 produced a deprecation warning: ``` DeprecationWarning: In future, it will be an error for 'np.bool' scalars to be interpreted as an index ``` This warning is somewhat misleading: it _is_ the case that Booleans are involved, but they are not being used as indices. The fields `rs`, `xs`, and `zs` of CliffordTableau as defined in file `cirq-core/cirq/qis/clifford_tableau.py` have type `Optional[np.ndarray]`, and the values in the ndarray have NumPy type `bool` in practice. The protocol buffer version of CliffordTableau defined in file `cirq-google/cirq_google/api/v2/program_pb2.pyi` defines those fields as `collections.abc.Iterable[builtins.bool]`. At first blush, you might think they're arrays of Booleans in both cases, but unfortunately, there's a wrinkle: Python defines its built-in `bool` type as being derived from `int` (see PEP 285), while NumPy explicitly does _not_ drive its `bool` from its integer class (see <https://numpy.org/doc/2.0/reference/arrays.scalars.html#numpy.bool>). The warning about converting `np.bool` to index values (i.e., integers) probably arises when the `np.bool` values in the ndarray are coerced into Python Booleans. At first, I thought the obvious solution would be to use `np.asarray` to convert the values to `builtins.bool`, but this did not work: ``` >>> import numpy as np >>> import builtins >>> arr = np.array([True, False], dtype=np.bool) >>> arr array([ True, False]) >>> type(arr[0]) <class 'numpy.bool'> >>> newarr = np.asarray(arr, dtype=builtins.bool) >>> newarr array([ True, False]) >>> type(newarr[0]) <class 'numpy.bool'> ``` They still end up being NumPy bools. Some other variations on this approach all failed to produce proper Python Booleans. In the end, what worked was to use `map()` to apply `builtins.bool` to every value in the incoming arrays. This may not be as efficient as possible; a possible optimization for the future is to look for a more efficient way to cast the types, or avoid having to do it at all. * Avoid a construct deprecated in NumPy 2 The NumPy 2 Migration Guide [explicitly recommends changing](https://numpy.org/doc/stable/numpy_2_0_migration_guide.html#adapting-to-changes-in-the-copy-keyword) constructs of the form ```python np.array(state, copy=False) ``` to ```python np.asarray(state) ``` * Avoid implicitly converting 2-D arrays of single value to scalars NumPy 2 raises deprecation warnings about converting an ndarray with dimension > 0 of values likle `[[0]]` to a scalar value like `0`. The solution is to get the value using `.item()`. * Add pytest option --warn-numpy-data-promotion This adds a new option to make NumPy warn about data promotion behavior that has changed in NumPy 2. This new promotion can lead to lower precision results when working with floating-point scalars, and errors or overflows when working with integer scalars. Invoking pytest with `--warn-numpy-data-promotion` will cause warnings warnings to be emitted when dissimilar data types are used in an operation in such a way that NumPy ends up changing the data type of the result value. Although this new option for Cirq's pytest code is most useful during Cirq's migration to NumPy 2, the flag will likely remain for some time afterwards too, because developers will undoubtely need time to adjust to the new NumPy behavior. For more information about the NumPy warning enabled by this option, see <https://numpy.org/doc/2.0/numpy_2_0_migration_guide.html#changes-to-numpy-data-type-promotion>. * Update requirements to use NumPy 2 This updates the minimum NumPy version requirement to 2.0, and updates a few other packages to versions that are compatible with NumPy 2.0. Note: NumPy 2.1 was released 3 weeks ago, but at this time, Cirq can only upgrade to 2.0. This is due to the facts that (a) Google's internal codebase is moving to NumPy 2.0.2, and not 2.1 yet; and (b) conflicts arise with some other packages used by Cirq if NumPy 2.1 is required right now. These considerations will no doubt change in the near future, at which time we can update Cirq to use NumPy 2.1 or higher. * Address NumPy 2 data type promotion warnings One of the changes in NumPy 2 is to the [behavior of type promotion](https://numpy.org/devdocs/numpy_2_0_migration_guide.html#changes-to-numpy-data-type-promotion). A possible negative impact of the changes is that some operations involving scalar types can lead to lower precision, or even overflow. For example, `uint8(100) + 200` previously (in Numpy < 2.0) produced a `unit16` value, but now results in a `unit8` value and an overflow _warning_ (not error). This can have an impact on Cirq. For example, 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 in cases like this, the proper solution is probably to make sure that where values are summed or otherwise numerically manipulated, `uint16` or larger values are ensured. NumPy 2 offers a new option (`np._set_promotion_state("weak_and_warn")`) to produce warnings where data types are changed. Commit 6cf50eb adds a new command-line to our pytest framework, such that running ```bash check/pytest --warn-numpy-data-promotion ``` will turn on this NumPy setting. Running `check/pytest` with this option enabled revealed quite a lot of warnings. The present commit changes code in places where those warnings were raised, in an effort to eliminate as many of them as possible. It is certainly the case that not all of the type promotion warnings are meaningful. Unfortunately, I found it sometimes difficult to be sure of which ones _are_ meaningful, in part because Cirq's code has many layers and uses ndarrays a lot, and understanding the impact of a type demotion (say, from `float64` to `float32`) was difficult for me to do. In view of this, I wanted to err on the side of caution and try to avoid losses of precision. The principles followed in the changes are roughly the following: * Don't worry about warnings about changes from `complex64` to `complex128`, as this obviously does not reduce precision. * If a warning involves an operation using an ndarray, change the code to try to get the actual data type of the data elements in the array rather than use a specific data type. This is the reason some of the changes look like the following, where it reaches into an ndarray to get the dtype of an element and then later uses the `.type()` method of that dtype to cast the value of something else: ```python dtype = args.target_tensor.flat[0].dtype ..... args.target_tensor[subspace] *= dtype.type(x) ``` * In cases where the above was not possible, or where it was obvious what the type must always be, the changes add type casts with explicit types like `complex(x)` or `np.float64(x)`. It is likely that this approach resulted in some unnecessary up-promotion of values and may have impacted run-time performance. Some simple overall timing of `check/pytest` did not reveal a glaring negative impact of the changes, but that doesn't mean real applications won't be impacted. Perhaps a future review can evaluate whether speedups are possible. * NumPy 2 data promotion + minor refactoring This commit for one file implements a minor refactoring of 3 test functions to make them all use similar idioms (for greater ease of reading) and to address the same NumPy 2 data promotion warnings for the remaining files in commit eeeabef. * Adjust dtypes per mypy warnings Mypy flagged a couple of the previous data type declaration changes as being incompatible with expected types. Changing them to satisfy mypy did not affect Numpy data type promotion warnings. * Fix Rigetti check for Aspen family device kind (quantumlib#6734) * Sync with new API for checking device family in qcs-sdk-python, Ref: rigetti/qcs-sdk-rust#463 in isa.pyi * Require qcs-sdk-python-0.20.1 which introduced the new family API Fixes quantumlib#6732 * Adjustment for mypy: change 2 places where types are declared Pytest was happy with the previous approach to declaring the value types in a couple of expressions, but mypy was not. This new version satisfies both. * Avoid getting NumPy dtypes in printed (string) scalar values As a consequence of [NEP 51](https://numpy.org/neps/nep-0051-scalar-representation.html#nep51), the string representation of scalar numbers changed in NumPy 2 to include type information. This affected printing Cirq circuit diagrams: instead seeing numbers like 1.5, you would see `np.float64(1.5)` and similar. The solution is to avoid getting the repr output of NumPy scalars directly, and instead doing `.item()` on them before passing them to `format()` or other string-producing functions. * Don't force Numpy 2; maintain compatibility with 1 The recent changes support NumPy 2 (as long as cirq-rigetti is removed manually), but they don't require NumPy 2. We can maintain compatibility with Numpy 1.x. * Bump serve-static and express in /cirq-web/cirq_ts (quantumlib#6731) Bumps [serve-static](https://github.com/expressjs/serve-static) and [express](https://github.com/expressjs/express). These dependencies needed to be updated together. Updates `serve-static` from 1.15.0 to 1.16.2 - [Release notes](https://github.com/expressjs/serve-static/releases) - [Changelog](https://github.com/expressjs/serve-static/blob/v1.16.2/HISTORY.md) - [Commits](expressjs/serve-static@v1.15.0...v1.16.2) Updates `express` from 4.19.2 to 4.21.0 - [Release notes](https://github.com/expressjs/express/releases) - [Changelog](https://github.com/expressjs/express/blob/4.21.0/History.md) - [Commits](expressjs/express@4.19.2...4.21.0) --- updated-dependencies: - dependency-name: serve-static dependency-type: indirect - dependency-name: express dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Michael Hucka <mhucka@caltech.edu> * Silence pytest warnings about asyncio fixture scope In the current version of pytest (8.3.3) with the pytest-asyncio module version 0.24.0, we see the following warnings at the beginning of a pytest run: ``` warnings.warn(PytestDeprecationWarning(_DEFAULT_FIXTURE_LOOP_SCOPE_UNSET)) ..../lib/python3.10/site-packages/pytest_asyncio/plugin.py:208: PytestDeprecationWarning: The configuration option "asyncio_default_fixture_loop_scope" is unset. The event loop scope for asynchronous fixtures will default to the fixture caching scope. Future versions of pytest-asyncio will default the loop scope for asynchronous fixtures to function scope. Set the default fixture loop scope explicitly in order to avoid unexpected behavior in the future. Valid fixture loop scopes are: "function", "class", "module", "package", "session" ``` A [currently-open issue and discussion over in the pytest-asyncio repo](pytest-dev/pytest-asyncio#924) suggests that this is an undesired side-effect of a recent change in pytest-asyncio and is not actually a significant warning. Moreover, the discussion suggests the warning will be removed or changed in the future. In the meantime, the warning is confusing because it makes it sound like something is wrong. This simple PR silences the warning by adding a suitable pytest init flag to `pyproject.toml'. * Fix wrong number of arguments to reshape() Flagged by pylint. * Fix formatting issues flagged by check/format-incremental * Add coverage tests for changes in format_real() * Remove import of kahypar after all In commit eb98361 I added the import of kahypar, which (at least at the time) appeared to have been imported by Quimb. Double-checking this import in clean environments reveals that in fact, nothing depends on kahypar. Taking it out via a separate commit because right now this package is causing our GitHub actions for commit checks to fail, and I want to leave a record of what caused the failures and how they were resolved. * Simplify proper_repr * No need to use bool from builtins * Restore numpy casting to the state as in main * Fix failing test_run_repetitions_terminal_measurement_stochastic Instead of summing int8 ones count them. * Simplify CircuitDiagramInfoArgs.format_radians Handle np2 numeric types without outputting their dtype. * `.item()` already collapses dimensions and converts to int * Exclude cirq_rigetti from json_serialization_test when using numpy-2 This also enables the hash_from_pickle_test.py with numpy-2. * pytest - apply warn_numpy_data_promotion option before test collection * Add temporary requirements file for NumPy-2.0 * Adjust requirements for cirq-core * allow numpy-1.24 which is still in the NEP-29 support window per https://numpy.org/neps/nep-0029-deprecation_policy.html * require `scipy~=1.8` as scipy-1.8 is the first version that has wheels for Python 3.10 --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Pavol Juhas <juhas@google.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.
This is a set of changes necessary to address issue #6706 and support Cirq migration to NumPy 2. The result makes Cirq compatible with NumPy 2 and 1, with the exception of the cirq-rigetti module, which at this time has an incompatible requirement for NumPy 1.
The changes target NumPy 2.0.2 rather than NumPy 2.1. At this time, some package dependency conflicts arise from other packages used by Cirq when NumPy 2.1 is required. This currently limits us to 2.0.2. (Note for the Google Quantum team: Google's internal codebase is about to move to NumPy 2.0.2, not 2.1, so the inability of supporting NumPy 2.1 is not a problem with respect to this impending change.)
The rest of this text summarizes the changes in this PR.
Avoid a construct deprecated in NumPy 2
The NumPy 2 Migration Guide explicitly recommends changing constructs of the form
np.array(state, copy=False)
tonp.asarray(state)
.Avoid implicitly converting 2-D arrays of single value to scalars
NumPy 2 raises deprecation warnings about converting an ndarray with dimension > 0 of values likle
[[0]]
to a scalar value like0
. The solution is to retrieve the value using.item()
instead.Address change in NumPy string representation of scalars
As a consequence of NEP 51, the string representation of scalar numbers changed in NumPy 2 to include type information. This affected printing Cirq circuit diagrams: instead seeing numbers like
1.5
, you would seenp.float64(1.5)
and similar.The solution is to use
.item()
on scalars before passing them to anything that needs to use the scalar's string representation (viastr
or__repr__()
). So, for example, ifx
is annp.int64
object,x.item()
returns the Python object, and then the string form of that looks normal.Explicitly convert NumPy
ndarray
ofnp.bool
to Pythonbool
In NumPy 2 (and possibly earlier versions), lines 478-480 in
cirq-google/cirq_google/api/v2/program_pb2.pyi
produced a deprecation warning:This warning is somewhat misleading: while it is the case that Booleans are involved, they are not being used as indices.
The fields
rs
,xs
, andzs
of CliffordTableau as defined in filecirq-core/cirq/qis/clifford_tableau.py
have typeOptional[np.ndarray]
, and the values in the ndarray have NumPy typebool
in practice. The protocol buffer version of CliffordTableau defined in filecirq-google/cirq_google/api/v2/program_pb2.pyi
defines those fields ascollections.abc.Iterable[builtins.bool]
. At first blush, you might think they're arrays of Booleans in both cases, but unfortunately, there's a wrinkle: Python defines its built-inbool
type as being derived fromint
(see PEP 285), while NumPy explicitly does not drive itsbool
from its integer class (see https://numpy.org/doc/2.0/reference/arrays.scalars.html#numpy.bool). The warning about convertingnp.bool
to index values (i.e., integers) probably arises when thenp.bool
values in the ndarray are coerced into Python Booleans.At first, I thought the obvious solution would be to use
np.asarray
to convert the values tobuiltins.bool
, but this did not work:They still end up being NumPy bools. Some other variations on this approach all failed to produce proper Python Booleans. In the end, what worked was to use
map()
to applybuiltins.bool
to every value in the incoming arrays. This may not be as efficient as possible; a possible optimization for the future is to look for a more efficient way to cast the types, or avoid having to do it at all.Address changes in NumPy data type promotion
Note added 2024-09-20: Pavol reasoned convincingly that it would be better to pull the non-essential NumPy 2 type warnings to a separate PR at a later date, and focus this PR on only essential compatibility issues. Pavol amended the PR accordingly. Consequently, the changes described in this section are mostly not part of the final PR. This text is being left in place because it provides details that may be useful in the future PR.
One of the changes in NumPy 2 is to the behavior of type promotion. A possible negative impact of the changes is that some operations involving scalar types can lead to lower precision, or even overflow. For example,
uint8(100) + 200
previously (in Numpy < 2.0) produced aunit16
value, but now results in aunit8
value and an overflow warning (not error). This can have an impact on Cirq. For example, in Cirq, simulator measurement result values areuint8
'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 thanuint8
, so in cases like this, the proper solution is probably to make sure that where values are summed or otherwise numerically manipulated,uint16
or larger values are ensured.NumPy 2 offers a new option (
np._set_promotion_state("weak_and_warn")
) to produce warnings where data types are changed. Commit 6cf50eb adds a new command-line to our pytest framework, such that runningwill turn on this NumPy setting. Running
check/pytest
with this option enabled revealed quite a lot of warnings. The present commit changes code in places where those warnings were raised, in an effort to eliminate as many of them as possible.It is certainly the case that not all of the type promotion warnings are meaningful. Unfortunately, I found it sometimes difficult to be sure of which ones are meaningful, in part because Cirq's code has many layers and uses ndarrays a lot, and understanding the impact of a type demotion (say, from
float64
tofloat32
) was difficult for me to do. In view of this, I wanted to err on the side of caution and try to avoid losses of precision. The principles followed in the changes are roughly the following:Don't worry about warnings about changes from
complex64
tocomplex128
, as this obviously does not reduce precision.If a warning involves an operation using an ndarray, change the code to try to get the actual data type of the data elements in the array rather than use a specific data type. This is the reason some of the changes look like the following, where it reaches into an ndarray to get the dtype of an element and then later uses the
.type()
method of that dtype to cast the value of something else:In cases where the above was not possible, or where it was obvious what the type must always be, the changes add type casts with explicit types like
complex(x)
ornp.float64(x)
.It is likely that this approach resulted in some unnecessary up-promotion of values and may have impacted run-time performance. Some simple overall timing of
check/pytest
did not reveal a glaring negative impact of the changes, but that doesn't mean real applications won't be impacted. Perhaps a future review can evaluate whether speedups are possible.