Skip to content
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

Add generalized ufunc linalg functions and make numpy.linalg use them #3220

Merged
merged 92 commits into from Apr 17, 2013

Conversation

pv
Copy link
Member

@pv pv commented Apr 10, 2013

This is a followup to gh-2954 (list of new commits), which replaces numpy.linalg routines with the corresponding umath_linalg implementations, while preserving backward compatibility for ndim <= 2 inputs. The new code is all under numpy/linalg.

Numpy's test suite and those of Scipy, Pandas, and Nipy pass with these changes, so there probably aren't obvious problems with backward compat.

This PR doesn't add new functions to numpy.linalg, only replaces existing ones. That, and other things still to do are summarized in issue gh-3217.

@charris
Copy link
Member

charris commented Apr 10, 2013

Looks like it needs a rebase.

ovillellas added 27 commits April 10, 2013 22:47
… code from the matrix_multiply in other gufuncs
…ue and the functions are not guaranteed to work with them).

also got cholesky working.
… functions (as is made in linalg module). Changed some code so that it uses BLAS instead of cblas (the fortran functions) also in line with what it is done on linalg. Modified the matrix multiply code, made it simpler and adapted to use blas (it was using cblas with CblasRowMajor, that is not available in the fortran function.
… and needed by out library. It seems to work now on Linux.
…ed from PDL" ufuncs). Only missing from "inspired from PDL" is matrix_multiply3.
added chosolve and poinv, they need testing.
@pv
Copy link
Member Author

pv commented Apr 10, 2013

Rebased.

@njsmith
Copy link
Member

njsmith commented Apr 10, 2013

Is there a test for the FP flags thing (63a8aef)? That seems like it could use a test.

For the double/single issue, does it work to use the ufunc dtype= argument? Theoretically this ought to let the internal ufunc machinery make any necessary promotions one-buffer-at-a-time, which is nicer than copying entire arrays.

@pv
Copy link
Member Author

pv commented Apr 10, 2013

@njsmith: test_linalg.py:test_reduced_rank fails if the flags aren't cleared (for SVD, but not all of the routines). Not sure how to exactly best test that otherwise, as the issue is floating point errors (0/0 et al) inside LAPACK. Matrices full of inf cause those, but in that case LAPACK also signals a failure... I can't think of a way to directly test this.

The dtype keyword seems buggy for gufuncs, btw (gh-3222):

>>> import numpy as np
>>> a = np.array([[1.+2j,2+3j], [3+4j,4+5j]], dtype=np.cdouble)
>>> np.linalg._umath_linalg.eigvals(a, dtype=np.complex128)
__main__:1: DeprecationWarning: Implicitly casting between incompatible kinds. In a future numpy release, this will raise an error. Use casting="unsafe" if this is intentional.
__main__:1: ComplexWarning: Casting complex values to real discards the imaginary part
array([-0.37228132+0.j,  5.37228132+0.j])
>>> np.linalg._umath_linalg.eigvals(a)
array([-0.35670389-0.26307815j,  5.35670389+7.26307815j])
>>> b = np.array([[1.,2], [3,4]], dtype=np.float64)
>>> np.linalg._umath_linalg.eigvals(b, dtype=np.float64)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: No loop matching the specified signature was found for ufunc eigvals

The signature keyword however seems to work (but is much messier to use) --- does also that do the cast per-block?

@njsmith
Copy link
Member

njsmith commented Apr 11, 2013

@pv: I think that dtype=X is just a shorthand for sig=<all X>? But I haven't checked.

pv added 3 commits April 12, 2013 18:59
…erations

This ensures that the FP invalid flag always reflects the return code
from LAPACK. Fixes a bug in 63a8aef where umath_linalg raises a
warning only if the error occurs in the last iteration of the ufunc
inner loop.
@pv
Copy link
Member Author

pv commented Apr 12, 2013

Using signature= for type selection, and do some ugly stuff to make the FP invalid flag reflect the LAPACK error status from all iterations. It would be nice to get the better error handling into ufuncs, but perhaps some ugliness can be tolerated before that.

pv added 3 commits April 13, 2013 16:14
With the new ufunc-based linalg, GIL is released in ufuncs, and needs to
be reacquired when raising errors in xerbla_.
If an exception is pending (raised from xerbla), the routines must
return NULL.
@pv
Copy link
Member Author

pv commented Apr 13, 2013

And then fixes for a GIL bug if a LAPACK error condition occurs.

@charris
Copy link
Member

charris commented Apr 14, 2013

@pv Do you feel this is in a state to go in?

@pv
Copy link
Member Author

pv commented Apr 14, 2013

I'm not aware of anything that breaks with this change (scipy, nipy, scikit-learn, pandas, statsmodels, networkx seem to suffer no regressions). Although this PR doesn't implement everything in the roadmap, it stands alone OK, and I think there's not more to add to it.

Should be advertised on the ML after (or before) merging.

@charris
Copy link
Member

charris commented Apr 15, 2013

The list post is getting much (any) response, but I'll wait a few more days.

@pv What about something like npymath, say npylapack_lite, for the combined library?

@pv
Copy link
Member Author

pv commented Apr 15, 2013

@charris: I don't see immediate use cases for npylapack_lite, as BLAS is widely available, if that's what you meant.

@pv
Copy link
Member Author

pv commented Apr 17, 2013

No further input seems to be coming, so maybe if there are no remaining concerns, we can merge and be done with it.

@charris
Copy link
Member

charris commented Apr 17, 2013

OK, in it goes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants