-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
ENH: estimate condition number for sparse matrices #21620
base: main
Are you sure you want to change the base?
Conversation
Implemented in the C layer, calls the SuperLU gscon function.
Calls scipy.sparse.linalg.splu and then calls the rinvnorm function of the resulting LU decomposition.
using scipy.sparse.linalg.rinvnormest and ...onenormest
instead of the condition number itself
>>> np.linalg.cond(A.toarray(), p=1) | ||
45.0 | ||
""" | ||
return onenormest(A)/rinvnormest(A) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When A
is singular, we currently get a RuntimeError: Factor is exactly singular
. In contrast, np.linalg.cond
gives a numpy.linalg.LinAlgError: Singular matrix
. Shall we catch the RuntimeError
and raise a LinAlgError
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the error type would require a deprecation cycle as it breaks backwards compatibility. I would leave it for a follow up PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be more precise, I meant to catch the LinAlgError raised by the rinvnormest call in cond1est and to replace it by a LinAlgError. This would not break backwards compatibility as cond1est is a new function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, it's a new function, then we can choose what we want. What I am not sure about is if we should mix error messages from dense and sparse linalg. There is a MatrixRankWarning
from sparse
for example. Let me loop in our sparse experts @dschult and @perimosocordiae.
See #18969 for previous (and not yet completed) discussions on the API. Also, otvam proposed another implementation on the Scientific Python Discourse forum that does not use SuperLU |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @maxaehle , thanks for the PR. Out of curiosity: could you provide a performance comparison between the new function and np.linalg.cond
for the same matrix in sparse and dense form?
The refguide and lint failures need to be fixed, but that should not be too difficult. Besides that, I have one remark about required tests (see below).
I cannot review the C code in depth but I think that a few comments would be useful for long term maintenance and review. SuperLU is very dense code, any pointers will help.
if (!CHECK_SLU_TYPE(self->type)) { | ||
PyErr_SetString(PyExc_ValueError, "unsupported data type"); | ||
return NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a test that this error is indeed raised. Same for the other ValueError below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of the errors might be unreachable because the SuperLU decomposition would fail before the rinvnorm function is called, but I've added a check for the norm
argument in d43b143
I think the terminology is going a bit different here. The condition number is defined as So they are inverse of one another. I'm not sure the docstrings are also correct in that regard. scipy/scipy/linalg/_matfuncs_expm.pyx.in Lines 495 to 628 in 867638e
|
I've added non-zero imaginary part to the complex64 and complex128 testcases in 7e33770 and since then the complex testcases fail. I need to investigate this further. |
I'm not sure which changes would be required. The new cond1est computes |
Here's a performance plot for 500x500 np.float64 matrices, produced with performance.py.txt on a 4-core system: For more dense matrices (200x200 np.float64), the performance advantage is not as big: |
What I mean is you can get cond1est by the reciprocal of the output of Here is the docstring of
So where does the inverse come in? |
|
Yes, you are right. And that's why typically, you slap a call to |
Interesting, I wasn't aware of
or are you ok with the current version of the API? |
Yes the story is a bit convoluted. The one norm estimation originally started from the need of matrix exponential. When the original expm was being written it is written for both sparse and the dense versions. In that algorithm, you need to investigate the one-norm of an array repeatedly hence for very large arrays 1-norm estimation (instead of calculating exactly) becomes very important. Then we rewrote the dense version independently and broke that relationship between sparse and dense Since you are wrapping SuperLU within C, I think lacon2 way is more ergonomic for your current way of doing it, that is to say just call scipy/scipy/linalg/_matfuncs_expm.c Lines 258 to 287 in e7c89a7
But I don't know if you have access to matvec/rmatvec methods at C level. If that is not a problem indeed number 2 is much simpler in my opinion. For number one, I think onenormest is good enough for folks historically speaking. But as you rightly pointed out passing the norm value back to C-level seems a bit awkward in terms of public API. So I agree that's a bit limping. |
Reference issue
Closes gh-18969
What does this implement/fix?
Add
scipy.sparse.linalg.SuperLU.rinvnormest
to compute reciprocal norm of inverse matrix given an LU decomposition,scipy.sparse.linalg.rinvnormest
to compute LU decomposition and call the function mentioned previously,scipy.sparse.linalg.cond1est
to compute the 1-norm condition number, calling the function mentioned previously andscipy.sparse.linalg.onenormest