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

Finish deprecation of unset asyncio_default_fixture_loop_scope #924

Open
seifertm opened this issue Aug 26, 2024 · 9 comments
Open

Finish deprecation of unset asyncio_default_fixture_loop_scope #924

seifertm opened this issue Aug 26, 2024 · 9 comments

Comments

@seifertm
Copy link
Contributor

Pytest-asyncio v0.24 introduced the configuration option asyncio_default_fixture_loop_scope which controls the event loop in which async fixtures are run. In order to allow partial migration of test suites, users have the option to leave the option unset. In this case, the event loop scope of async fixtures defaults to the fixture caching scope, unless explicitly specified. However, when asyncio_default_fixture_loop_scope is unset, pytest-asyncio will emit a deprecation warning.

The goal of this issue is to remove the option to have asyncio_default_fixture_loop_scope unset. The default value should be funciton. The pytest-asyncio test suite should be adjusted accordingly, so that asyncio_default_fixture_loop_scope is not set explicitly, unless necessary.

@jamesbraza
Copy link

In the meantime, how can one suppress the PytestDeprecationWarning that pops up? I am trying to configure a filterwarnings entry in pyproject.toml's [tool.pytest.ini_options], but I can't figure out the syntax. Here is what I am trying, but it isn't working:

[tool.pytest.ini_options]
filterwarnings = [
    'ignore:configuration option "asyncio_default_fixture_loop_scope" is unset',
]

I also don't want to use pytest -p no:warnings.

felddy added a commit to cisagov/cyhy-kevsync that referenced this issue Aug 29, 2024
@ixenion
Copy link

ixenion commented Aug 30, 2024

In the meantime, how can one suppress the PytestDeprecationWarning that pops up? I am trying to configure a filterwarnings entry in pyproject.toml's [tool.pytest.ini_options], but I can't figure out the syntax. Here is what I am trying, but it isn't working:

[tool.pytest.ini_options]
filterwarnings = [
    'ignore:configuration option "asyncio_default_fixture_loop_scope" is unset',
]

I also don't want to use pytest -p no:warnings.

Have the same issue.
You can try to make pyproject.toml file in the root of your project which contains:

[tool.pytest.ini_options]
asyncio_default_fixture_loop_scope = "function"

And It worked for me.
But I dont like this approach.
May be there is a way to add something in tests subdir?

@jamesbraza
Copy link

jamesbraza commented Aug 30, 2024

Yeah I also don't want to add that asyncio_default_fixture_loop_scope value to all my pyproject.toml. I would rather just silence the warning.

I played around a bunch more today, and still couldn't get filterwarnings to work. I think the issue is that the deprecation warning comes from a pytest plugin, which gets loaded before filterwarnings takes effect. I thus opened pytest-dev/pytest#12756 to have this fixed upstream in pytest, then I think the filterwarnings route can work 👌

@scosman
Copy link

scosman commented Sep 3, 2024

You can also set it in your pytest.ini to silence warnings, but agreed would be nice to properly default without warning.

[pytest]
asyncio_mode=auto
asyncio_default_fixture_loop_scope="function"

@nierob
Copy link

nierob commented Sep 4, 2024

In our project we want to use one event loop for all the tests. The tests are written in an "event loop safe" way. Nothing should leak from a test to a test and if it does it hints a much bigger problem, than just a test isolation issue. That also means that there is some shared state, like db connection, but it is fine and expected. To achieve that we set:

@pytest.fixture(scope="session")
def event_loop(request) -> typing.Generator:
    loop = asyncio.get_event_loop()
    yield loop
    loop.close()

Now we hit the warning about unset asyncio_default_fixture_loop_scope, it looks like a config that could replace our custom fixture. So we added pyproject.toml config:

[tool.pytest.ini_options]
testpaths = [
    "tests/",
]
addopts = "--durations=12  --disable-socket --allow-hosts=localhost,127.0.0.1,::1 --strict-markers"
...
asyncio_default_fixture_loop_scope = "session"
asyncio_mode = "auto"  # See pytest-asyncio plug-in

The operation replaced the warning with:

Replacing the event_loop fixture with a custom implementation is deprecated
  and will lead to errors in the future.
  If you want to request an asyncio event loop with a scope other than function
  scope, use the "scope" argument to the asyncio mark when marking the tests.
  If you want to return different types of event loops, use the event_loop_policy
  fixture.

Great so far, but now removing our fixtures causes problems:

RuntimeError('Event loop is closed')

So something is off, the event loop should be closed at the process exit only, unless I misunderstood the feature. The error goes away if I mark the tests with an explicit:

@pytest.mark.asyncio(loop_scope="session")

but then what is the point of asyncio_default_fixture_loop_scope ?

@ixenion
Copy link

ixenion commented Sep 4, 2024

Cant get rid of asyncio warning.
Iv removed pytest.ini, pyproject.toml etc.
Added conftest.py with content nierob shown,
Marked async tests explicitly with

@pytest.mark.asyncio(loop_scope="session")

And nothing changed - still getting pytest-asyncio warning.

Should v0.24.0 fix that? I dont get it...

mhucka added a commit to mhucka/Cirq that referenced this issue Sep 15, 2024
With Pytest version 8.3.3 and pytest-asyncio 0.24.0 in Python 3.10
(and possibly other versions), we get this deprecation warning when
running `check/pytest`:

```
[...]/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"
```

This warning is essentially useless for our purposes, and only causes
unnecessary confusion about what's going on. Based on the discussion
at <pytest-dev/pytest-asyncio#924>, a simple
way to silence the deprecation warning is to add an option to
`pyproject.toml`.
rumpelsepp added a commit to Fraunhofer-AISEC/gallia that referenced this issue Sep 17, 2024
@seifertm
Copy link
Contributor Author

@jamesbraza Thanks for pointing to the pytest issue! I wasn't aware of that.

@nierob The asyncio_default_fixture_loop_scope setting only affects async fixtures, not async tests. A similar configuration option for async tests is very much desired and tracked in #793. That means the asyncio_default_fixture_loop_scope setting and the @pytest.mark.asyncio(loop_scope="…") are independent. Other than that, your understanding of the feature is very much correct and removing the event_loop fixture implementation is the correct thing to do.

If it's any help, I'd like to refer you to one of the migration guides and to a how-to about running all tests in a session-wide loop

@ixenion Let me know if these how-tos help you solve the DeprecationWarning.

mhucka added a commit to mhucka/Cirq that referenced this issue Sep 19, 2024
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'.
mhucka added a commit to mhucka/Cirq that referenced this issue Sep 19, 2024
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'.
mhucka added a commit to quantumlib/Cirq that referenced this issue Sep 19, 2024
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'.
mhucka added a commit to quantumlib/Cirq that referenced this issue Sep 20, 2024
* 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 (#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 #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 (#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>
@nierob
Copy link

nierob commented Sep 23, 2024

Thank you for the explanation! I did not know that fixtures follows a different configuration then tests.

In addition to asyncio_default_fixture_loop_scope I implemented pytest_collection_modifyitems to mark all tests with scoped event loop, that allowed me to remove my own loop fixture and all warnings are gone. I'm looking forward to #793 to remove pytest_collection_modifyitems call in future too 🤗 !

tumidi added a commit to questionpy-org/questionpy-sdk that referenced this issue Oct 14, 2024
tumidi added a commit to questionpy-org/questionpy-server that referenced this issue Oct 14, 2024
felddy added a commit to cisagov/cyhy-kevsync that referenced this issue Oct 18, 2024
tumidi added a commit to questionpy-org/questionpy-sdk that referenced this issue Oct 22, 2024
tumidi added a commit to questionpy-org/questionpy-server that referenced this issue Oct 22, 2024
@rysev-a
Copy link

rysev-a commented Oct 30, 2024

I use async fixture with config asyncio_default_fixture_loop_scope="function",
and has error

    def _retrieve_scope_root(item: Union[Collector, Item], scope: str) -> Collector:
        node_type_by_scope = {
            "class": Class,
            "module": Module,
            "package": Package,
            "session": Session,
        }
>       scope_root_type = node_type_by_scope[scope]
E       KeyError: '"function"'

Help to remove quotes this way, (if use async fixtures):

You can also set it in your pytest.ini to silence warnings, but agreed would be nice to properly default without warning.

[pytest]
asyncio_mode=auto
asyncio_default_fixture_loop_scope="function"
 asyncio_default_fixture_loop_scope=function

harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this issue 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
Projects
None yet
Development

No branches or pull requests

7 participants
@scosman @nierob @rysev-a @seifertm @jamesbraza @ixenion and others