-
Couldn't load subscription status.
- Fork 52
feat: add eig and eigvals to the linalg extension
#978
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
base: main
Are you sure you want to change the base?
Conversation
eig and eigvals to the linalg extensioneig and eigvals to the linalg extension
|
Could we not handle the NumPy return type discrepancy in the compatibility layer? I.e., we'd standardize always returning a complex array and we'd smooth that over in the compat layer for NumPy? |
|
If push comes to shove, yes it's an option. A larger issue is what is the compat layer: is it a thin temporary layer to smooth over small incompatibilities for until array libraries match the standard, or is this something permanent where we correct the behavior of array libraries? If we say it's OK to do an allocation for numpy's |
|
As discussed in the Oct 16 consortium meeting, the preference is to require eigenvalues to be always of a complex dtype. The last commit updates the text accordingly, and fixes the dtype of eigenvectors (wihch is always complex, too). |
Grumpy old man hat, but: If the NumPy issue isn't resoelved (and it possibly cannot, at least not for 2 years even if I agree), then I don't like this. I am not opposed at all to just codifying this, but we should clarify the scope somewhere in this repo more clearly. To me that is easy to resolve: Accept that exactly matching API in the main namespace is aspirational at best and actually document that clearly somewhere so I can point at it from the NumPy repo. Otherwise, I don't know what to do, because it seems to me I would have to veto adding this until the NumPy issue is resolved. |
|
I'm fine with accepting this PR as long as NumPy has made a decision to made the change (whether through flipping a switch in a future release, or through a deprecation process). That the standard and NumPy are temporarily misaligned is okay, and |
Following up on an RFC at #935, add non-symmetric eigenvalue solvers,
eigandeigvals, to thelinalgextension.The routines are available in
numpy,torch,jax.numpyandcupy(as of to-be-released cupy 1.14, cf cupy/cupy#8980). The main point of contention in gh-935 was the behavior for all eigenvalues on the real line. NumPy returns a real array and all other libraries return a complex array (with zero imaginary parts if they happen to be zero).Based on the discussion in numpy/numpy#29000, it might it be difficult to change in numpy itself. Therefore, the suggestion is to make the precise dtype implementation-defined.
I believe the benefit of having
eigin the standard outweighs this wart.One other minor point of inconsistency is
jax-ml/jax#32558:already fixed.jaxreturns a standard-compliant namedtuple foreighbut a bare tuple foreig. I believe this is a minor issue though.