API: Add matrix_norm, vector_norm, vecdot and matrix_transpose [Array API]
#25155
Conversation
4f392bf to
0f6b8b1
Compare
0f6b8b1 to
32526b4
Compare
|
The |
|
|
||
| * `numpy.vecdot` and `numpy.linalg.vecdot` | ||
|
|
||
| * `numpy.matrix_transpose` and `numpy.linalg.matrix_transpose` |
There was a problem hiding this comment.
Can you add a sentence or two describing what these functions do?
There was a problem hiding this comment.
Sure! I added one sentence descriptions from Array API definitions.
8102ddb to
665d6c6
Compare
matrix_norm, vector_norm, vecdot and matrix_transpose [Array API]matrix_norm, vector_norm, vecdot and matrix_transpose [Array API]
numpy/_core/fromnumeric.py
Outdated
| transpose : Generic transpose method. | ||
|
|
||
| """ | ||
| x = asarray(x) |
There was a problem hiding this comment.
Similar to the discussion in the other PR - should this be asanyarray?
There was a problem hiding this comment.
I think there was roughly an agreement in #25101 (comment) that asanyarray can be used in place of asarray.
I replaced it for functions added here and outer that has been merged.
b3011a7 to
daa0350
Compare
| x1_ = np.moveaxis(x1_, axis, -1) | ||
| x2_ = np.moveaxis(x2_, axis, -1) | ||
|
|
||
| res = x1_[..., None, :] @ x2_[..., None] |
There was a problem hiding this comment.
@seberg mentioned on the triage call today that this is supposed to do the hermitian conjugate according to the array API standard - does the matmul operator do that?
There was a problem hiding this comment.
And if so, probably worth adding a test that has complex vectors.
There was a problem hiding this comment.
Right, a conjugate was missing - I added it (and a test) according to Array API.
Note: np.vdot does conjugate on the first parameter (if it's complex), where np.vecdot does it on the second one (I mentioned it in the vecdot's docs).
There was a problem hiding this comment.
no, the numpy way is correct, the array-api way is just a bug.
There was a problem hiding this comment.
Ah, right (I see that pytorch also performs conjugate on the first argument). Let me change that here and fix it in the Array API.
There was a problem hiding this comment.
Can you open an issue that this should have a dedicated ufunc? BLAS should have kernels which perform the complex conjugation on the fly, so at least for the complex conjugating case, this is really not ideal.
There was a problem hiding this comment.
Note that wikipedia disagrees: the conjugate of the second argument is taken: https://en.wikipedia.org/wiki/Dot_product#Complex_vectors
There was a problem hiding this comment.
Mathematicians and physicists have opposite conventions. Mathematicians use sesquilinear forms, physicists follow the Dirac bracket convention (I don't know who actually started the convention). It can make teaching awkward ...
There was a problem hiding this comment.
But then BLAS agrees with what you have, so I guess wikipedia is confused too... https://netlib.org/lapack/explore-html/d1/dcc/group__dot_ga5c189335a4e6130a2206c190579b1571.html#ga5c189335a4e6130a2206c190579b1571
daa0350 to
f7b5640
Compare
f7b5640 to
dd913aa
Compare
|
Alright, let's pull this one in. Thanks @mtsokol! One note: in the release notes the code literals should be wrapped in double backticks, not single backticks. Can you go through all the release notes you've added and make sure they're all formatted correctly? Merging this anyway since that's a minor issue that can be fixed separately. |
@ngoldbaum I did this formatting on purpose. Double backticks give code format, where single backticks create a link to NumPy member's documentation. For example see: https://github.com/numpy/numpy/blob/4b5ae68c78a222eba2e07cc1e34bd29d5cfeb30d/doc/release/upcoming_changes/22539.deprecation.rst?plain=1 from a PR shipped with 1.25.0 release. which resulted in: 
|
|
Ah, thanks for the explanation! Gotta love sphinx/rst trivia like that. |
| f"but they are: {x1_shape[axis]} and {x2_shape[axis]}." | ||
| ) | ||
|
|
||
| x1_, x2_ = np.broadcast_arrays(x1, x2) |
There was a problem hiding this comment.
Darn, another one. Would it still be possible to add subok=True here to allow subclasses to "just work"?
| ndim = builtins.max(x1.ndim, x2.ndim) | ||
| x1_shape = (1,)*(ndim - x1.ndim) + tuple(x1.shape) | ||
| x2_shape = (1,)*(ndim - x2.ndim) + tuple(x2.shape) | ||
| if x1_shape[axis] != x2_shape[axis]: |
There was a problem hiding this comment.
Note that this use of axis is contrary to how a gufunc would interpret it: there axis is taken for every element separately before broadcasting, i.e., for vectors with shapes (3, 4) and (3, 2, 4) and axis=0, the output would have shape (2, 4). You can see this from
from numpy._core.umath_tests import inner1d
a = np.arange(12).reshape(3, 4)
b = np.arange(24).reshape(3, 2, 4)
inner1d(a, b, axis=0).shape
# (2, 4)
I think we should stick with this, otherwise this is going to be very confusing in the future (alternatively, one would need to change how axis and axes are interpreted in gufuncs...)
Hi @rgommers @ngoldbaum,
This PR contains the last Array API compatibility update for
numpy.linalg. It adds:numpy.linalg.matrix_normnumpy.linalg.vector_normnumpy.vecdotandnumpy.linalg.vecdotnumpy.matrix_transposeandnumpy.linalg.matrix_transpose