Do not raise in linalg Ops #1834
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #1078
Instead of adding
on_errorto every Op, this PR choses the simpler approach, just catch and return nan by default.This also allows us to simplify the numba implementations quite a lot and get rid of a couple of low-level dispatch for condition number and the like. Perhaps it even speeds up uncached compilation.
Only one Op had this special behavior: Cholesky. For helping with transitioning,
on_error="raise"is now implemented symbolically with a FutureWarning. The default switched toon_error="nan", which is the "default" in all lingalg Ops (at least the ones I covered).This is analogous to us returning 1/0->nan without any error/warning. If it's undefined/can't be computed we return nan, and let the users deal with it. Most of the times the error makes it hader to work with, not easier, as there is no symbolic
try/except, while there is symbolicisnan(x).any().I also removed the
check_finiteargument internally. Users can define it without it having any effect for API compat. The stated reason for this in Scipy is that some implementation of BLAS/Lapack can hang / crash the system with non-finite values. I'm taking a chance here, as those issues may never show up in the versions our users install / modern implementations. We can revisit this later if it proves to be still needed.I'm marking this as a major breaking change.
Finally, I've removed some lapack overloads that were never used anywhere, and inlined others that were used only in one place. I believe it makes code more readable and perhaps uncached numba compilation faster