Use LAPACK functions for cho_solve, lu_factor, solve_triangular
#1605
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (60.86%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1605 +/- ##
==========================================
- Coverage 81.71% 81.69% -0.02%
==========================================
Files 230 230
Lines 52925 52966 +41
Branches 9402 9418 +16
==========================================
+ Hits 43249 43272 +23
- Misses 7244 7253 +9
- Partials 2432 2441 +9
🚀 New features to boost your workflow:
|
| @@ -8,6 +8,7 @@ | |||
| import scipy.linalg as scipy_linalg | |||
| from numpy.exceptions import ComplexWarning | |||
| from scipy.linalg import get_lapack_funcs | |||
| from scipy.linalg._misc import LinAlgError, LinAlgWarning | |||
There was a problem hiding this comment.
from scipy.linalg import LinAlgError, LinAlgWarning also works :)
There was a problem hiding this comment.
This is really great, thanks for taking this on!
I have one requested change (the one about shape checking) and the rest is related to what we should do if the algorithm fails. Please don't make those changes until the other devs weigh in, since it's a bit of an API break.
| if c.ndim != 2 or c.shape[0] != c.shape[1]: | ||
| raise ValueError("The factored matrix c is not square.") | ||
| if c.shape[1] != b.shape[0]: | ||
| raise ValueError(f"incompatible dimensions ({c.shape} and {b.shape})") |
There was a problem hiding this comment.
We don't need to do shape checking in perform, that is handled by make_node
There was a problem hiding this comment.
not true, shapes may not be static
There was a problem hiding this comment.
not true, shapes may not be static
FWIW; we'll deprecate in-place modifications of the shape (also dtype and strides) modifications in numpy 2.4
There was a problem hiding this comment.
Static here means we don't know the shape until runtime, as in the following graph:
import pytensor
import pytensor.tensor as pt
x = pt.vector("x", shape=(None,))
out = pt.exp(x)
fn = pytensor.function([x], out)
fn([1, 2, 3])
fn([1, 2, 3, 4]) # Allowed to call with different input lengths each time| if info != 0: | ||
| raise ValueError(f"illegal value in {-info}th argument of internal potrs") | ||
|
|
There was a problem hiding this comment.
I'd prefer if we returned a matrix of np.nan if info !=0 rather than erroring out. This is what jax does, and it makes it a lot more ergonomic to work with in iterative algorithms.
This might be out of scope for this PR; asking @ricardoV94 for a 2nd opinion
| if info < 0: | ||
| raise ValueError( | ||
| f"illegal value in {-info}th argument of internal getrf (lu_factor)" | ||
| ) | ||
| if info > 0: | ||
| warnings.warn( | ||
| f"Diagonal number {info} is exactly zero. Singular matrix.", | ||
| LinAlgWarning, | ||
| stacklevel=2, | ||
| ) |
| if info > 0: | ||
| raise LinAlgError( | ||
| f"singular matrix: resolution failed at diagonal {info-1}" | ||
| ) | ||
| elif info < 0: | ||
| raise ValueError(f"illegal value in {-info}-th argument of internal trtrs") |
Description
Directly use LAPACK functions in the
performmethod of the following classes, removing some checks fromscipy.linalg(e.g._datacopiedandasarray):Add coverage for empty case in each.
Related Issue
Checklist
Type of change
📚 Documentation preview 📚: https://pytensor--1605.org.readthedocs.build/en/1605/