ENH: Implement most linalg operations for 0x0 matrices #8368
Conversation
56bd2cd to
1ef37a5
Compare
|
Tests should probably be added.
|
| @@ -1142,6 +1144,8 @@ def eig(a): | |||
| _assertNdSquareness(a) | |||
There was a problem hiding this comment.
eig would previously produce a cryptic ValueError: cannot remove a zero-sized axis from an iterator, since it omitted the check in other places.
It's not clear to me why NpyIter_RemoveAxis does not allow a zero-sized axis to be removed. At the very least, it should allow all but the last zero-sized axis to be removed, I think?
There was a problem hiding this comment.
I have an old PR, which would remove this error, but mostly was never merged, because it required cleaning up of all linalg functions to make sure they don't crash within lapack....
There was a problem hiding this comment.
Mind digging it out? Presumably it required cleaning them up C-side, so this PR is not a substitute
There was a problem hiding this comment.
Only takes a search of open PRs by me: #3861
There was a problem hiding this comment.
Eric, is my PR removing that weird error in some way useful here, or is it orthogonal. Frankly, I am not sure myself whether or not it helps here, considering that lapack probably cannot handle 0d anyway.
There was a problem hiding this comment.
My guess is rather: Once we put this one in, I can reactivate my old one....
There was a problem hiding this comment.
Lapack actually seems able to handle 0x0 (not 0d) in a lot of cases, but needs more careful argument preparation, that we are not doing. In particular, I think I can make lstsq work with it. I think your pr removing that weird error would become useful for the vectorized ops though.
There was a problem hiding this comment.
jup, and I guess we can reduce it to those deletions....
|
This should be considered blocked by #8369. The passing tests are spurious. Once that PR is done and merged, I'll rebase this over the improved tests |
|
Ah, likely. But the thing is, even if one lapack does, who knows what
MKL will do etc. Unless its specified somewhere its supported we should
probably expect it not to be supported?
|
|
@eric-wieser This needs documentation in the 1.13 release notes. |
|
Also, the first three commits should be squashed. |
|
@charris: Will do, once the blocking PR is sorted. Was planning to squash the commits when rebasing then |
|
☔ The latest upstream changes (presumably #8369) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Probably want to squash the fixups together with the rebase. |
b129bfe to
b023b5b
Compare
|
@charris: Yep, done. Now that the tests check for failures in the right places, I've changed them to not check for failures here any more too.
|
numpy/linalg/linalg.py
Outdated
| @@ -1659,7 +1663,8 @@ def pinv(a, rcond=1e-15 ): | |||
|
|
|||
| """ | |||
| a, wrap = _makearray(a) | |||
| _assertNoEmpty2d(a) | |||
| if product(a.shape[-2:]) == 0: | |||
| return wrap(empty(a.shape[:-2] + (a.shape[-1], a.shape[-2]), dtype=a.dtype)) | |||
There was a problem hiding this comment.
Line too long. I'd split out the shape computation into a new line or use
a:shape[:-2] + a.shape[-1:-3:-1].
There was a problem hiding this comment.
Never a fan of the three-part slice, so I'll split this
numpy/linalg/linalg.py
Outdated
| _assertRankAtLeast2(a) | ||
| _assertNdSquareness(a) | ||
| _assertFinite(a) | ||
| t, result_t = _commonType(a) | ||
| if product(a.shape[-2:]) == 0: |
There was a problem hiding this comment.
How about an isempty2d function for this?
There was a problem hiding this comment.
Good thinking - will do
numpy/linalg/linalg.py
Outdated
| _assertRankAtLeast2(a) | ||
| _assertNdSquareness(a) | ||
| _assertFinite(a) | ||
| t, result_t = _commonType(a) | ||
| if product(a.shape[-2:]) == 0: | ||
| return empty(a.shape[-1:], dtype=result_t), wrap(empty(a.shape, dtype=result_t)) |
There was a problem hiding this comment.
I'd just split this into two lines for computing the returned object, then the return. It takes up a bit more space but shortens the line and is clearer (IMHO).
Same for other long lines.
a325fa8 to
3c541ee
Compare
|
@charris: All good now? |
|
Could use a comment in the 1.13.0 release notes, probably under improvements. |
Fixes numpy#8212 det: return ones of the right shape slogdet: return sign=ones, log=zeros of the right shape pinv, eigvals(h?), eig(h?): return empty array(s?) of the right shape svd & qr: not implemented, due to complex return value rules
|
Darn, always forget that. Sorted - just needs passing tests |
|
Passing! |
|
Just a note on style, we avoid camelcase except for classes. The |
|
Thanks Eric. |
Fixes #8212
Changes:
eigvals: return an empty array of the right shape
det: return ones of the right shape
slogdet: return sign=ones, log=zeros of the right shape
pinv: return empty array of the right shape
svd & qr: not implemented, due to complex return value rules