BUG: Always return a real dtype from linalg.cond (gh-18304) #29333
Conversation
Addresses numpygh-18304. The condition number of a matrix is the product of two norms, which are always non-negative and real-valued, so the condition number itself should be non-negative and real-valued. This commit returns the proper real dtype from `linalg.cond`, and includes tests for the condition number of a complex matrix in various norms.
numpy/linalg/_linalg.py
Outdated
| return r | ||
| # Return the absolute value, since the condition number is | ||
| # always a non-negative real number. | ||
| return abs(r) |
There was a problem hiding this comment.
Thanls, I am sure it's correct, but I do feel it should probably be more targeted to where the complex number comes from here.
This commit addresses a reviewer comment on the blanket application of `abs(r)`. It specifically ensures the return type of complex-valued matrices will be the corresponding real type.
| @@ -2011,6 +2011,7 @@ def cond(x, p=None): | |||
| # contain nans in the entries where inversion failed. | |||
| _assert_stacked_square(x) | |||
| t, result_t = _commonType(x) | |||
| result_t = _realType(result_t) # condition number is always real | |||
| signature = 'D->D' if isComplexType(t) else 'd->d' | |||
| with errstate(all='ignore'): | |||
| invx = _umath_linalg.inv(x, signature=signature) | |||
There was a problem hiding this comment.
So, I think this is fine. Your other comment (deleted now) is not wrong, in that it would be fine to cast invx to the original result_t to achieve the right thing.
(Norm might in some paths end up casting back and forth then, but it would work too.)
|
I agree that a release note is probably not needed. There is functionally no difference, since the imaginary part was always 0 anyway. There are no member functions that differ between |
Addresses gh-18304.
The condition number of a matrix is the product of two norms, which are
always non-negative and real-valued, so the condition number itself
should be non-negative and real-valued. This commit returns the proper
real dtype from
linalg.cond, and includes tests for the conditionnumber of a complex matrix in various norms.
The existing docstring for
linalg.condalready states that the return value iseither a float or inf, so the function behavior is now consistent with the
docstring.