Added handling for LinalgError and fill_value for failed pixels#674
Conversation
| Sa_surface = Sa[fm.idx_surface, :][:, fm.idx_surface] | ||
| Sa_inv = svd_inv_sqrt(Sa_surface, hash_table, hash_size)[0] | ||
|
|
||
| except np.linalg.LinAlgError: |
There was a problem hiding this comment.
Since it was requested in the tagup meeting how to avoid duplicating code, there's two ways you can merge this together:
- Use a dict to retrieve the error code for the respective exception
errorCodes = {
np.linalg.LinAlgError: -15
ValueError: -11
}
try:
...
except Exception as e:
C_rcond = []
trajectory[n + 1, :] = [fill_value for i in x]
EXIT_CODE = errorCodes[e]- Use
isinstance
try:
...
except Exception as e:
C_rcond = []
trajectory[n + 1, :] = [fill_value for i in x]
if isinstance(e, np.linalg.LinAlgError):
EXIT_CODE = -15
elif isinstance(e, ValueError):
EXIT_CODE = -11There was a problem hiding this comment.
Additionally, if you'd rather catch specifically those exceptions and no others so others could raise naturally, you can just change to:
except (np.linalg.LinAlgError, ValueError) as e:| # On invertible matrix error, save NaN array on step and update exit code | ||
| C_rcond = [] | ||
| trajectory[n + 1, :] = x | ||
| trajectory[n + 1, :] = [fill_value for i in x] |
There was a problem hiding this comment.
Super minor, and I'm not certain the shape of your x necessarily or how impactful this could be, but you could potentially save likely a marginal amount of time:
fill_value = 7
x = np.arange(1_000)
a = [fill_value] * len(x)
b = [fill_value for i in x]
assert a == b
%timeit [fill_value] * len(x)
2.24 µs ± 21.8 ns per loop (mean ± std. dev. of 7 runs, 100,000 loops each)
%timeit [fill_value for i in x]
29.3 µs ± 123 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)There was a problem hiding this comment.
It's silly I didn't consider np.full, which is even faster
%timeit np.full(len(x), fill_value)
819 ns ± 8.17 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)(sorry, this isn't really necessary just fun to consider)
|
Sweet! Thanks so much for the comments @jammont I made the change to a shared exception block (Linalg and ValueError exclusive). Changing to the faster list fill is also a no brainer. |
We hit a strange error case within the invert_analytical routine. Certain pixels were failing within the Sa matrix inversion block, which were accurately caught as failed pixels. We looked into why they were failing, and it turned out to be more complicated that we initially thought.
Rather than the matrix containing any NaNs, the matrix was correctly formed, but the scipy.linalg.eigh function failed with error:
LinAlgError: Internal Error. There is some similarity and a chance this error follows: scipy/scipy#13220. Theeigheven succeeds on the transpose of the matrix when the only difference is precision.For now the work around is to explicitly handle this error within the analytical line.
This PR also includes an addition of a fill value for failed pixels within the analytical line. Rather than returning the result of invert_algebraic, failed pixels will now return fill (-9999 by default).