Minor improvement of pinv2 and pinvh #393

Merged
merged 2 commits into from Jan 4, 2013

Conversation

Projects
None yet
2 participants
Contributor

akhmerov commented Jan 1, 2013

Avoid matrix operations on array elements that are multiplied by 0
in the process of calculation.

@akhmerov akhmerov Improve pinv2 and pinvh.
Avoid matrix operations on array elements that are multiplied by 0
in the process of calculation.
7e4298d

@jakevdp jakevdp commented on an outdated diff Jan 1, 2013

scipy/linalg/basic.py
- B = np.transpose(np.conjugate(np.dot(u * psigma_diag, vh)))
+ B = np.transpose(np.conjugate(np.dot(u[:, : rank] * psigma_diag, vh[: rank])))
@jakevdp

jakevdp Jan 1, 2013

Member

PEP8: line is too long

Member

jakevdp commented Jan 1, 2013

Looks great - I was thinking about doing something along these lines after I added pinvh, but never got to it.
Tests all pass for me; other than my one pep8 comment above, I think this is ready to merge.

Contributor

akhmerov commented Jan 1, 2013

Thanks, and my bad about PEP8.

Member

jakevdp commented Jan 1, 2013

Great - I'll let this sit for a few days in case others have comments, but I'm +1 for merge. Thanks for the contribution!

Member

jakevdp commented Jan 4, 2013

Merging, thanks!

@jakevdp jakevdp added a commit that referenced this pull request Jan 4, 2013

@jakevdp jakevdp Merge pull request #393 from akhmerov/master
Minor improvement of pinv2 and pinvh
a0c0a2a

@jakevdp jakevdp merged commit a0c0a2a into scipy:master Jan 4, 2013

Contributor

akhmerov commented Jan 9, 2013

@jakevdp Is there anything that needs to be done for this to propagate to numpy.linalg?

Member

jakevdp commented Jan 9, 2013

numpy.linalg is a separate package - you might consider copying this code and submitting a numpy PR. Note that numpy.linalg only contains one pinv function, which is implemented via SVD similar to scipy's pinv2.

Member

jakevdp commented Jan 9, 2013

I should add that I'm not sure if the numpy devs would want functionality added to np.linalg (e.g. alternative forms of pinv may not be a good fit, but you could ask) but and improved implementation of the current function will certainly be appreciated there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment