BUG: Fix pinv for stacked matrices #8827

eric-wieser · 2017-03-25T00:01:50Z

Duplicate of #4678

eric-wieser · 2017-03-25T00:07:10Z

numpy/linalg/tests/test_linalg.py

@@ -423,25 +423,6 @@ def test_generalized_empty_herm_cases(self):
            exclude={'none'})


-def dot_generalized(a, b):


Ahead of its time...

Except not, as it turns out. This doesn't broadcast, but instead tries to work out which are vectors and which are matrices. Guess we're keeping it, for now

Funny, I had exactly the same thought -- couldn't you use matmul instead -- but then realised it was actually different (in the actual expression, I might have used einsum instead, but it is not really important at all).

eric-wieser · 2017-03-25T00:08:58Z

numpy/linalg/linalg.py

@@ -226,6 +227,22 @@ def _assertNoEmpty2d(*arrays):
        if _isEmpty2d(a):
            raise LinAlgError("Arrays cannot be empty")

+def transpose(a):
+    """
+    Transpose each matrix in a stack of matrices.


TODO: expose this publicly? (#7495)

Well, there is at least one implementation that could (eventually) be abandoned if this is exposed: https://github.com/astropy/astropy/blob/master/astropy/coordinates/matrix_utilities.py#L32

Any reason for calling the method and not the free function?

Also, i'm increasingly considering using astropy for completely non-astrophysics related things in future - your units sounds more actively-developed than Pint, and rotation matrices are something I end up writing too often.

Regarding the function before the one you link - #8719

The method is the real thing; the function just turns the input into an array if needed and then calls the method. https://github.com/numpy/numpy/blob/master/numpy/core/fromnumeric.py#L458

Yep, I know :). Guess people are unlikely to pass around lists of lists

p.s. In the astropy case this is only used internally in cases where I know the inputs are arrays; if we exposed this publically in numpy, I think using the function makes more sense.

We need something like this, but I think it should be an array method as well. Maybe something like a.M(atrix)T(ranspose. When the *.T method was proposed there was discussion of what it should do, transposing matrices in a stack was one of ideas. I could also see a vector transpose, a.VT which just appends 1 to the dimensions. These ideas need discussion on the list.

Should I add a leading underscore to the name for now?

@eric-wieser That would help distinguish it from np.transpose and avoid some confusion. Long term, it probably needs a different name.

At any rate, this isn't going to appear in the docs at the moment, so isn't really public API yet.

I'll send something around the mailing list requesting a name at some point. I think we can start off with the conservative assert ndim >= 2 semantics initially, and put off the decision in #7495 and #9530

mhvk · 2017-03-25T01:03:50Z

numpy/linalg/linalg.py

@@ -2008,13 +2020,13 @@ def lstsq(a, b, rcond=-1):
                resids = array([sum((ravel(bstar)[n:])**2)],
                               dtype=result_real_t)
    else:
-        x = array(transpose(bstar)[:n,:], dtype=result_t, copy=True)
+        x = array(nd_transpose(bstar)[:n,:], dtype=result_t, copy=True)


If you're changing it anyway, maybe just do bstar.T? So much clearer...

Of course, since bstar is 2-D, you can also just leave it as it was...

I mean, I plan to throw this all out in #8720 anyway...

The only reason I changed this line is because I wanted to use the name transpose for something else.

charris · 2017-04-13T02:44:29Z

doc/release/1.13.0-notes.rst

@@ -159,6 +159,10 @@ being iterated over.
 For consistency with ``ndarray`` and ``broadcast``, ``d.ndim`` is a shorthand
 for ``len(d.shape)``.

+``matrix_rank`` and ``pinv`` in ``np.linalg`` now works for stacked arrays
+--------------------------------------------------------------------------


works -> work

charris · 2017-04-13T02:56:23Z

numpy/linalg/linalg.py

+        matrices was added in Numpy 1.13.0.
+    rcond : (...) arr_like of float
+        Cutoff for small singular values.
+        Singular values smaller (in modulus) than


How about "less than" rather than smaller. The singular values are always non-negative reals.

Even for complex matrices.

Note that I haven't actually changed these comments, just adjusted their indentation.

The singular values are always non-negative reals.

Is this the case even under numerical error?

~~Either way, the description right now matches the implementation. If taking the modulus is not required, then we should remove it from the doc and implementation at the same time (and in another PR).~~ Edit: I'm wrong

Note that "modulus" was added here in d1572e2 by @pv, so looks like an intentional clarification

Puzzling -- as @charris notes, singular values are by definition positive real, so reading something about a modulus would seem only confusing.

homu · 2017-04-21T18:27:38Z

☔ The latest upstream changes (presumably #8886) made this pull request unmergeable. Please resolve the merge conflicts.

homu · 2017-05-10T14:53:35Z

☔ The latest upstream changes (presumably #9050) made this pull request unmergeable. Please resolve the merge conflicts.

eric-wieser · 2017-07-13T22:14:48Z

Updated to refer to 1.14 not 1.13 in release notes

charris · 2017-08-16T21:24:01Z

@eric-wieser Want to rebase this? I'm trying to get all the linalg fixes/enhancements in.

eric-wieser · 2017-08-16T23:21:26Z

Yes, but I can't until I regain access to a git installation in two weeks - so maybe someone else should.

eric-wieser · 2017-08-16T23:46:42Z

Conflict is just in the imports

Fixes numpy#8826

eric-wieser · 2017-09-13T06:25:48Z

@charris: Ok, rebased. Ready to go in?

charris · 2017-09-13T18:32:02Z

I'm putting this in as is. Thanks Eric. If you want to change the name of transpose go ahead and make another PR.

eric-wieser added 00 - Bug component: numpy.linalg labels Mar 25, 2017

eric-wieser force-pushed the fix-pinv branch from 5135951 to 62ea4af Compare March 25, 2017 00:04

eric-wieser commented Mar 25, 2017

View reviewed changes

eric-wieser mentioned this pull request Mar 25, 2017

ENH: ndarray.T2 for 2D transpose. #7495

Open

eric-wieser force-pushed the fix-pinv branch from 62ea4af to b61fbbb Compare March 25, 2017 00:45

mhvk reviewed Mar 25, 2017

View reviewed changes

eric-wieser mentioned this pull request Mar 25, 2017

ENH: Make np.linalg.pinv behave like a gufunc #4678

Closed

eric-wieser force-pushed the fix-pinv branch 2 times, most recently from 8e02ce0 to ad36b66 Compare March 25, 2017 09:47

eric-wieser mentioned this pull request Apr 13, 2017

WIP: Collection miscellaneous updates for 1.13.0 release. #8937

Closed

eric-wieser force-pushed the fix-pinv branch from ad36b66 to 09da799 Compare April 13, 2017 02:12

charris reviewed Apr 13, 2017

View reviewed changes

eric-wieser force-pushed the fix-pinv branch from 43683e8 to cd25e62 Compare April 13, 2017 09:29

eric-wieser mentioned this pull request Jul 13, 2017

ENH: add ND generalizations to np.gradient #9409

Open

eric-wieser force-pushed the fix-pinv branch from deb66b3 to 305e2ca Compare July 13, 2017 22:11

eric-wieser force-pushed the fix-pinv branch 2 times, most recently from 8b29605 to bffaf0d Compare July 13, 2017 22:16

eric-wieser added 3 commits September 12, 2017 23:22

MAINT: Remove use of int as boolean

2d66d07

BUG: Fix pinv on matrix stacks

bab798d

Fixes numpy#8826

ENH: Broadcast the second argument of matrix_rank to match pinv

219d2a5

eric-wieser force-pushed the fix-pinv branch from 7b6f25d to 219d2a5 Compare September 13, 2017 06:25

Merge branch 'master' into fix-pinv

ebe2cfb

mhvk mentioned this pull request Sep 13, 2017

Feature request: T method for transposition applying to 1D vectors #9530

Closed

charris merged commit f7be36b into numpy:master Sep 13, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Fix pinv for stacked matrices #8827

BUG: Fix pinv for stacked matrices #8827

eric-wieser commented Mar 25, 2017 •

edited

Loading

eric-wieser Mar 25, 2017

eric-wieser Mar 25, 2017 •

edited

Loading

mhvk Mar 25, 2017

eric-wieser Mar 25, 2017 •

edited

Loading

mhvk Mar 25, 2017

eric-wieser Mar 25, 2017 •

edited

Loading

mhvk Mar 25, 2017

eric-wieser Mar 25, 2017 •

edited

Loading

mhvk Mar 25, 2017

charris Sep 13, 2017

eric-wieser Sep 13, 2017

charris Sep 13, 2017

eric-wieser Sep 13, 2017

mhvk Mar 25, 2017

mhvk Mar 25, 2017

eric-wieser Mar 25, 2017 •

edited

Loading

eric-wieser Apr 13, 2017

charris Apr 13, 2017

charris Apr 13, 2017

charris Apr 13, 2017

eric-wieser Apr 13, 2017 •

edited

Loading

eric-wieser Apr 13, 2017

mhvk Apr 13, 2017

homu commented Apr 21, 2017

homu commented May 10, 2017

eric-wieser commented Jul 13, 2017

charris commented Aug 16, 2017

eric-wieser commented Aug 16, 2017

eric-wieser commented Aug 16, 2017

eric-wieser commented Sep 13, 2017

charris commented Sep 13, 2017

		@@ -423,25 +423,6 @@ def test_generalized_empty_herm_cases(self):
		exclude={'none'})


		def dot_generalized(a, b):

BUG: Fix pinv for stacked matrices #8827

BUG: Fix pinv for stacked matrices #8827

Conversation

eric-wieser commented Mar 25, 2017 • edited Loading

Choose a reason for hiding this comment

eric-wieser Mar 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eric-wieser Mar 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eric-wieser Mar 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eric-wieser Mar 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eric-wieser Mar 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eric-wieser Apr 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

homu commented Apr 21, 2017

homu commented May 10, 2017

eric-wieser commented Jul 13, 2017

charris commented Aug 16, 2017

eric-wieser commented Aug 16, 2017

eric-wieser commented Aug 16, 2017

eric-wieser commented Sep 13, 2017

charris commented Sep 13, 2017

eric-wieser commented Mar 25, 2017 •

edited

Loading

eric-wieser Mar 25, 2017 •

edited

Loading

eric-wieser Mar 25, 2017 •

edited

Loading

eric-wieser Mar 25, 2017 •

edited

Loading

eric-wieser Mar 25, 2017 •

edited

Loading

eric-wieser Mar 25, 2017 •

edited

Loading

eric-wieser Apr 13, 2017 •

edited

Loading