-
-
Notifications
You must be signed in to change notification settings - Fork 11.1k
MAINT: Fix computation of numpy.array_api.linalg.vector_norm #21084
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
Changes from 2 commits
71fe19a
6af507b
284a7c0
54e1184
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
from __future__ import annotations | ||
|
||
from ._dtypes import _floating_dtypes, _numeric_dtypes | ||
from ._manipulation_functions import reshape | ||
from ._array_object import Array | ||
|
||
from typing import TYPE_CHECKING | ||
|
@@ -391,18 +392,43 @@ def vector_norm(x: Array, /, *, axis: Optional[Union[int, Tuple[int, int]]] = No | |
if x.dtype not in _floating_dtypes: | ||
raise TypeError('Only floating-point dtypes are allowed in norm') | ||
|
||
# np.linalg.norm tries to do a matrix norm | ||
a = x._array | ||
if axis is None: | ||
a = a.flatten() | ||
axis = 0 | ||
# Note: np.linalg.norm() doesn't handle dimension 0 arrays | ||
if a.ndim == 0: | ||
a = a[None] | ||
else: | ||
a = a.flatten() | ||
_axis = 0 | ||
elif isinstance(axis, tuple): | ||
# Note: The axis argument supports any number of axes, whereas norm() | ||
# only supports a single axis for vector norm. | ||
rest = tuple(i for i in range(a.ndim) if i not in axis) | ||
# Note: The axis argument supports any number of axes, whereas | ||
# np.linalg.norm() only supports a single axis for vector norm. | ||
normalized_axis = [i if i >= 0 else i + a.ndim for i in axis] | ||
rest = tuple(i for i in range(a.ndim) if i not in normalized_axis) | ||
newshape = axis + rest | ||
a = np.transpose(a, newshape).reshape((np.prod([a.shape[i] for i in axis]), *[a.shape[i] for i in rest])) | ||
axis = 0 | ||
return Array._new(np.linalg.norm(a, axis=axis, keepdims=keepdims, ord=ord)) | ||
|
||
a = np.transpose(a, newshape).reshape( | ||
(np.prod([a.shape[i] for i in axis], dtype=int), *[a.shape[i] for i in rest])) | ||
_axis = 0 | ||
else: | ||
_axis = axis | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We do have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I should have guessed there are probably already helpers in NumPy to do some of these things. I'll see if I can find anything that makes this simpler. |
||
|
||
res = Array._new(np.linalg.norm(a, axis=_axis, ord=ord)) | ||
|
||
if keepdims: | ||
# We can't reuse np.linalg.norm(keepdims) because of the hacks above | ||
# to avoid matrix norm logic. | ||
shape = list(x.shape) | ||
if axis is None: | ||
_axis = range(x.ndim) | ||
elif isinstance(axis, int): | ||
_axis = (axis,) | ||
else: | ||
_axis = axis | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might put this together with the setup (i.e. figuring out the result shape there? But looks fine. |
||
for i in _axis: | ||
shape[i] = 1 | ||
res = reshape(res, tuple(shape)) | ||
|
||
return res | ||
|
||
__all__ = ['cholesky', 'cross', 'det', 'diagonal', 'eigh', 'eigvalsh', 'inv', 'matmul', 'matrix_norm', 'matrix_power', 'matrix_rank', 'matrix_transpose', 'outer', 'pinv', 'qr', 'slogdet', 'solve', 'svd', 'svdvals', 'tensordot', 'trace', 'vecdot', 'vector_norm'] |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand,
flatten
should always return a 1-D array?EDIT: Actually, why not
.ravel()
?flatten
forces a copy and that is really not necessary (ravel copies more often than you would expect too, but at least not always.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I never realized that about
flatten
. We have to make it a 1-D array to force it to do a vector norm (this weird shape-based behavior is why we splitnorm
intovector_norm
andmatrix_norm
in the array API).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but
np.array(0).ravel()
(or flatten) also is 1D, this is not<= 1
but== 1
, soravel()
covers thea[None]
path just as well.