Fix deprecated Numpy logic in NormalizeRXAngles#11023
Merged
Merged
Conversation
This new pass added in Qiskitgh-10634 uses some deprecated Numpy properties and has some slightly fragile exception-based logic when the required properties can be directly tested. This code issues warnings with Numpy 1.25+, which is currently not used by CI due to Qiskitgh-10305.
jakelishman
added
the
Changelog: None
Collaborator
|
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 6533835675
💛 - Coveralls |
mtreinish
previously approved these changes
Oct 16, 2023
Member
mtreinish
left a comment
mtreinish
left a comment
There was a problem hiding this comment.
LGTM, I think this makes sense to me. TBH, I'm getting a bit lost in the logic on the old side but since it's passing tests I think we are fine.
ElePT
approved these changes
Oct 23, 2023
Contributor
ElePT
left a comment
ElePT
left a comment
There was a problem hiding this comment.
Approving to get it back to the merge queue.
mtreinish
added
the
stable backport potential
mergify Bot
pushed a commit
that referenced
this pull request
Oct 23, 2023
* Fix deprecated Numpy logic in `NormalizeRXAngles` This new pass added in gh-10634 uses some deprecated Numpy properties and has some slightly fragile exception-based logic when the required properties can be directly tested. This code issues warnings with Numpy 1.25+, which is currently not used by CI due to gh-10305. * Fix return value (cherry picked from commit 6bf90fa)
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Oct 23, 2023
* Fix deprecated Numpy logic in `NormalizeRXAngles` This new pass added in gh-10634 uses some deprecated Numpy properties and has some slightly fragile exception-based logic when the required properties can be directly tested. This code issues warnings with Numpy 1.25+, which is currently not used by CI due to gh-10305. * Fix return value (cherry picked from commit 6bf90fa) Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
jakelishman
added a commit
to jakelishman/qiskit-terra
that referenced
this pull request
Mar 12, 2024
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.
jakelishman
added a commit
to jakelishman/qiskit-terra
that referenced
this pull request
Apr 15, 2024
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>
jakelishman
added a commit
to jakelishman/qiskit-terra
that referenced
this pull request
Apr 16, 2024
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>
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Apr 25, 2024
* Finalise support for Numpy 2.0 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: - gh-10890 - gh-10891 - gh-10892 - gh-10897 - gh-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> * Update `__array__` methods for Numpy 2.0 compatibility 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. * fixing numerical issues on mac-arm * Change error to match Numpy --------- Co-authored-by: Lev S. Bishop <18673315+levbishop@users.noreply.github.com> Co-authored-by: Sebastian Brandhofer <148463728+sbrandhsn@users.noreply.github.com>
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Jun 18, 2024
* Finalise support for Numpy 2.0 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: - gh-10890 - gh-10891 - gh-10892 - gh-10897 - gh-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> * Update `__array__` methods for Numpy 2.0 compatibility 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. --------- Co-authored-by: Lev S. Bishop <18673315+levbishop@users.noreply.github.com> Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This new pass added in gh-10634 uses some deprecated Numpy properties and has some slightly fragile exception-based logic when the required properties can be directly tested. This code issues warnings with Numpy 1.25+, which is currently not used by CI due to gh-10305.
Details and comments
This should fix the common errors in #11020. There's still a Linux / Numpy 1.26.1 failure in unitary synthesis that's new, and it's not immediately clear to me if that's related or not.
I submitted this as a separate PR to #11020 because I'd like the history / CI to record that we're not sacrificing compatibility with Numpy 1.24 to add compatibility with 1.26.