DOC: Enumerate the differences between numpy and numpy.array_api #21260
Conversation
…y.array_api This is a more organized presentation of the various "Note" comments in numpy/array_api. In particular, each difference is notated as to whether it is a strictness difference (no change in NumPy needed), a compatible change (NumPy can change in a backwards compatible way), or a breaking change (NumPy would need to break backwards compatibility to match the spec).
There was a problem hiding this comment.
This looks great and is super useful, thanks @asmeurer! I have only a couple of small comments about the document.
Given that this is very useful, I recommend merging it very soon. Any discussion on making code changes to NumPy should be done elsewhere. Let's focus the review here on whether the document is correct, well-written and useful to have in the NumPy html docs.
|
For some reason the CI link to the artifact is broken. Here is the current rendered version: https://output.circle-artifacts.com/output/job/d6f9451b-164d-4a89-ad56-024550656ddf/artifacts/0/doc/build/html/reference/array_api.html |
Do we have any tests that fail before, pass after those changes? Do we test the array_api? |
| @@ -380,7 +384,7 @@ def vecdot(x1: Array, x2: Array, /, *, axis: int = -1) -> Array: | |||
|
|
|||
| # The type for ord should be Optional[Union[int, float, Literal[np.inf, | |||
| # -np.inf]]] but Literal does not support floating-point literals. | |||
| def vector_norm(x: Array, /, *, axis: Optional[Union[int, Tuple[int, int]]] = None, keepdims: bool = False, ord: Optional[Union[int, float]] = 2) -> Array: | |||
| def vector_norm(x: Array, /, *, axis: Optional[Union[int, Tuple[int, ...]]] = None, keepdims: bool = False, ord: Optional[Union[int, float]] = 2) -> Array: | |||
There was a problem hiding this comment.
If a line is 81 characters and the linter complains we can let it slide. But this one is 155 characters ...
There was a problem hiding this comment.
Please let's not have that conversation again, we decided this in the original PR and this change does not touch line length. This diff is fine.
There was a problem hiding this comment.
Nice! Generally, should probably just merge this as soon as you think it is ready. It is a great overview to see that most difference are indeed small. Besides the "big" ones (promotion, scalars), most indeed seem like small changes.
(I am not too interested in the "strictness" changes, but it is probably good to list them together – in a sense we have a dual role of informing about the array_api as well as how the main namespace is incompatible to the "Array API".)
There is a quite large amount of "compatible additions" that we probably have to figure out what to do about if we want them all in the main namespace. Especially if old name need to stay around.
E.g. the asin, etc. ones I guess we need to keep the old names around for a very long time if we want to rename them. But maybe try to hide them away a bit?
(From a NumPy perspective, I would have almost liked np.bitwise.left_shift rather than np.bitwise_left_shift for example.)
doc/source/reference/array_api.rst
Outdated
| - **Breaking** | ||
| - | ||
| * - ``pinv`` has an ``rtol`` keyword argument instead of ``rcond`` | ||
| - **Compatible** |
There was a problem hiding this comment.
I bet the new default is better, but it this is a breaking change since it is a change of the default value? The new name itself is a compatible change of course.
There was a problem hiding this comment.
There was some discussion about this (also for matrix_rank). See data-apis/array-api#211 and data-apis/array-api#216.
There was a problem hiding this comment.
This is breaking indeed, but as breaks go it's as small as they get.
There was a problem hiding this comment.
Fine to mark as breaking here I'd say, pointing out that the default changes from 1e-15 to None, which means max(M, N) * eps.
There was a problem hiding this comment.
I marked these as compatible because the name of the keyword argument also changed.
There was a problem hiding this comment.
That doesn't matter here right? A np.linalg.pinv(x) call would start behaving differently, so that is backwards incompatible. If you were thinking that we'd keep rcond and rtol, that wouldn't quite make sense.
There was a problem hiding this comment.
It is an incompatible change and it could at the very least break the reproducibility of workflows (results change and I am not sure how to transition smoothly). So it should be marked/discussed as such. In principle it has the potential to be a large change, but I doubt it is in practice.
I would expect that the number of affected users will be rather small and that it should almost never cause wrong results; so it would rather only break reproducibility. And more often than not, it hopefully will "fix" results also.
Plus, reproducibility may well already be a game of luck for those that would be affected.
There was a problem hiding this comment.
Ah, you're right of course. The keyword rename can be done compatibly but the default change can't.
There was a problem hiding this comment.
The same reasoning also applies matrix_rank, and to the stable flag of sort and argsort.
| the behavior with Python scalars). | ||
| * - No casting unsigned integer dtypes to floating dtypes (e.g., ``int64 + | ||
| uint64 -> float64``. | ||
| - **Strictness** |
There was a problem hiding this comment.
I suppose this is probably even in violation, rather than just "strictness"? Most promotion things in NumPy are "strictness", but this one I am not sure, since it goes from in to float?
There was a problem hiding this comment.
I think it's not breaking. There is no uint64 + int64 promotion rule, which means it is undefined behavior. Libraries may decide to raise an exception, cast to float64, cast to int64, or do whatever else they like.
There was a problem hiding this comment.
OK. I thought it might disallow such "weird" ones (rather than leaving it fully open). I guess that distinction would only mainly matter if there is a chance that you may want to specify contradicting promotions it in the future.
(For me it could make sense to specify that int + uint may never promote to a non-integral, but I guess NumPy is just weird here, and hopefully we may eventually deprecate or warn about this one anyway...)
There was a problem hiding this comment.
I guess that distinction would only mainly matter if there is a chance that you may want to specify contradicting promotions it in the future.
That's indeed the one/main reason to specify particular exceptions or "implementing this is not allowed". We've tried to stay away from specifying exceptions or error handling type things, because it's impossible to be comprehensive (in addition for it to be difficult to align libraries' behavior here).
| arrays. | ||
| - **Breaking** | ||
| - For example, ``np.array(0., dtype=float32)*np.array(0., | ||
| dtype=float64)`` is ``float32``. Note that this is value-based casting |
There was a problem hiding this comment.
The example is meant to use **, and maybe can just be considered a bug, since it does not use the typical promotion rules. But then, if we change promotion rules, this should just align anyway, so it hardly matters.
There was a problem hiding this comment.
I can't tell what's a bug or not when it comes to value-based casting. It definitely is odd that __pow__ is the only operator that does this.
doc/source/reference/array_api.rst
Outdated
| - No cross-kind casting. No value-based casting on scalars. | ||
| * - ``stack`` has different default casting rules from ``np.stack`` | ||
| - **Strictness** | ||
| - No cross-kind casting. No value-based casting on scalars. |
There was a problem hiding this comment.
| - No cross-kind casting. No value-based casting on scalars. | |
| - No promotion to a common dtype. No value-based casting on scalars. |
I am not sure if stack will never use value-based casting currently? The function calls asarray() upfront, so I think we would most likely also not use scalar-logic in the future?
The reason is, that I think if one input is scalar, all inputs must be 0-D. For concatenation it can differ, although only for axis=None – which I am not quite sure you actually want to support.
There was a problem hiding this comment.
stack and concat do allow promoting, but cross-kind promoting is unspecified ("strictness"). c.f. https://data-apis.org/array-api/latest/API_specification/generated/signatures.manipulation_functions.stack.html and https://data-apis.org/array-api/latest/API_specification/generated/signatures.manipulation_functions.concat.html
According to my comments in the code, incorrect promotion was an issue for concat. But I guess this is not so for stack (I misread the code there; stack does call result_type, but only for the error).
There was a problem hiding this comment.
Sounds good, might be nice to mention the fact that it only matters for axis=None for concat, but doesn't matter too much. (Which makes this really a very niche usage.)
doc/source/reference/array_api.rst
Outdated
| - The spec does not have array scalars, only 0-D arrays. It is not clear | ||
| if NumPy's scalars deviate from the spec 0-D array behavior in any ways | ||
| other than the ones outlined in | ||
| :ref:`array_api-type-promotion-differences`. |
There was a problem hiding this comment.
They mostly do not deviate in promotion :) (although I bet there are some subtleties in power or so). Because the weird promotion rules also apply to 0-D arrays...
They are mainly immutable and hashable (if you are constrained to numeric dtypes anyway). But of course what to do with scalars may well be the next big annoying thing after the promotion issues...
There was a problem hiding this comment.
Right, I wasn't clear if there were any other odd differences. Immutability shouldn't be a problem because strictly speaking the spec allows it (some libraries like Jax are completely immutable, c.f. https://data-apis.org/array-api/latest/design_topics/copies_views_and_mutation.html).
There was a problem hiding this comment.
I changed this to "strictness" for now. I believe that other than type promotion (which is discussed elsewhere), scalars can duck type as 0-D arrays sufficiently for the spec. If this is ever disproved, we can update this. It's hard to know for sure without plugging them into the test suite somehow, unless you are aware of any other differences that could affect things.
| - ??? | ||
| - | ||
| * - ``diagonal`` operates on the last two axes. | ||
| - **Breaking** |
There was a problem hiding this comment.
But you are moving it to linalg, which may well be a way to do the transition in NumPy. I guess in a sense it is breaking, because having np.diagonal and np.linalg.diagonal do different things may be too confusing to keep both.
There was a problem hiding this comment.
I hadn't considered that. I'll leave it as "breaking" but add a note about this.
This is definitely one of the worst "breaking" cases here for NumPy. NumPy's diagonal is completely spec noncompliant, and not in a small way either, since the semantics completely differ when you have rank > 2. I don't know if the current behavior is deprecatable either.
doc/source/reference/array_api.rst
Outdated
| - The ``norm`` function has been omitted from the array API and split | ||
| into ``matrix_norm`` for matrix norms and ``vector_norm`` for vector | ||
| norms. Note that ``vector_norm`` supports any number of axes, whereas | ||
| ``np.norm`` only supports a single axis for vector norms. |
There was a problem hiding this comment.
| ``np.norm`` only supports a single axis for vector norms. | |
| ``np.linalg.norm`` only supports a single axis for vector norms. |
| two axes. See `the spec definition | ||
| <https://data-apis.org/array-api/latest/API_specification/generated/signatures.linear_algebra_functions.matrix_transpose.html#signatures.linear_algebra_functions.matrix_transpose>`__ | ||
| * - ``outer`` only supports 1-dimensional arrays. | ||
| - **Breaking** |
There was a problem hiding this comment.
Is this a requirement in the standard, or do you allow higher dimensions?
There was a problem hiding this comment.
@kgryte and I were discussing this internally. It was his view that it is incompatible, but maybe he can comment here.
There was a problem hiding this comment.
Current NumPy behavior flattens arrays with more than 1 dimension. We should consider this incompatible, especially as the consortium is likely to move forward with support for batching: data-apis/array-api#242. If 242 moves forward, NumPy's behavior would be non-compliant. And as such, I think it is appropriate here to indicate that the spec entails a breaking change. This can be revisited should the consortium move in a different direction.
There was a problem hiding this comment.
Thanks, I didn't realize np.outer used raveling. np.ufunc.outer result is a.shape + b.shape I think. Adding an axis argument and changing its default might work, but then breaking is definitely right for now anyway.
| - Notes | ||
| * - ``sum`` and ``prod`` always upcast ``float32`` to ``float64`` when | ||
| ``dtype=None``. | ||
| - **Breaking** |
There was a problem hiding this comment.
I assume no matter what axis= is? This one seems almost one of the larger changes to me.
There was a problem hiding this comment.
Yes, there's no restriction on axis. It's not clear to me that the axis should matter? It wasn't ever mentioned in the discussion as far as I can tell.
There was a problem hiding this comment.
I faintly remember seeing the discussion and being surprised by it before, but not diving into it and thinking seriously about it. I feel this change may be something to run by a wider audience. I guess it may not be massive from a back-compat point of view since it only affects float16, float32, and complex64.
Also: I think this is at least the default float/integer precision when the input is float/integer? Or do integer inputs return floats?
There was a problem hiding this comment.
The default output dtype is based on the input dtype: spec.
If the input is an integer dtype, the default output dtype is the default integer dtype provided the default integer dtype has sufficient precision.
Similarly, if the input is a float dtype, the default output dtype is the default floating-point dtype, provided the default floating-point dtype has sufficient precision.
In short, when dtype=None, the output array should never have a dtype with less precision than the input dtype.
| * - ``cross`` does not broadcast its arguments. | ||
| - ??? | ||
| - | ||
| * - ``cross`` does not allow size 2 vectors (only size 3). | ||
| - ??? | ||
| - |
There was a problem hiding this comment.
I've opened an issue concerning linalg.cross behavior: data-apis/array-api#415. The outcome of that discussion should have bearing on potential backward compatibility concerns.
There was a problem hiding this comment.
As discussed in data-apis/array-api#415, allowing size 2 vectors is likely to never be supported in the specification given difficulties such support presents in, e.g., JIT compilation (e.g., #13718). Accordingly, we should mark this NumPy behavior as backward incompatible with the spec.
Re: broadcasting. My sense is that we may be moving toward alignment on NumPy broadcasting behavior, but this is still TBD. Atm, I don't think we need to mark NumPy's current broadcasting behavior as backward incompatible.
This applies even of the name changed, because it affects the case where no keyword is passed.
|
Discussion has quieted down. Ready to merge? |
|
I'm OK with merging. There were a few open questions on some of the points ( |
|
OK, let's get this in. Thanks @asmeurer , |
These items were not clear in the original PR numpy#21260 but have since been clarified.
These items were not clear in the original PR numpy#21260 but have since been clarified.
This is a more organized presentation of the various "Note" comments in numpy/array_api.
In particular, each difference is notated as to whether it is a strictness difference (no change in NumPy needed), a compatible change (NumPy can change in a backwards compatible way), or a breaking change (NumPy would need to break backwards compatibility to match the spec).
This is related to the discussion in #21135. I'm hopeful that this can help the NumPy developers better understand exactly how NumPy would need to change in order for the top-level
numpynamespace to become array API compliant.I've also included a few minor fixes to numpy.array_api.
Note that, as outlined in the original pull request (#18585), this does not include any documentation for the array API itself. It would be a good idea to do this, but it is a harder problem, because each function would need a docstring. For now, the spec serves as the best reference for
numpy.array_api.…and vecdot