8000 MAINT: Fix computation of numpy.array_api.linalg.vector_norm by asmeurer · Pull Request #21084 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content

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

Merged
merged 4 commits into from
Jun 16, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 35 additions & 9 deletions numpy/array_api/linalg.py
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
Expand Down Expand Up @@ -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()
Copy link
Member
@seberg seberg Feb 18, 2022

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.)

Copy link
Member Author

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 split norm into vector_norm and matrix_norm in the array API).

Copy link
Member

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, so ravel() covers the a[None] path just as well.

_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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do have a normalize_axis_tuple helper in numpy/core/numeric.py, maybe worthwhile?

Copy link
Member Author

Choose a reason for hiding this comment

The 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
Copy link
Member

Choose a reason for hiding this comment

The 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']
2 changes: 1 addition & 1 deletion 7DDA numpy/linalg/linalg.py
Original file line number Diff line number Diff line change
Expand Up @@ -2558,7 +2558,7 @@ def norm(x, ord=None, axis=None, keepdims=False):
else:
absx = abs(x)
absx **= ord
ret = add.reduce(absx, axis=axis, keepdims=keepdims)
ret = asarray(add.reduce(absx, axis=axis, keepdims=keepdims))
ret **= (1 / ord)
return ret
elif len(axis) == 2:
Expand Down
0