[BUG] Guard default RidgeClassifierCV against LAPACK SVD overflow - #3755
[BUG] Guard default RidgeClassifierCV against LAPACK SVD overflow#3755aashoday wants to merge 7 commits into
Conversation
Several classifiers (RDSTClassifier, RocketClassifier, MultiRocketHydraClassifier) fit a default RidgeClassifierCV on transformed feature matrices that can exceed the 32-bit LAPACK indexing limit for SVD, raising an unhelpful scipy error on large datasets. - Add check_lapack_svd_safe() to aeon.utils.validation, which raises an informative ValueError before fitting when the default estimator is used and the matrix would overflow. - Wire this check into RDSTClassifier and RocketClassifier ahead of their default RidgeClassifierCV fit. - Expose an estimator parameter on MultiRocketHydraClassifier and apply the same check, matching the pattern already used by RDSTClassifier and RocketClassifier. - The check is skipped entirely when a custom estimator is supplied, so behaviour for existing custom-estimator users is unchanged.
- test_check_lapack_svd_safe.py: boundary tests for check_lapack_svd_safe (under/at/over the 32-bit limit) and a check that the error message includes the computed element count. - test_rdst.py, test_rocket.py, test_mr_hydra.py: for each classifier, confirm check_lapack_svd_safe is called once when the default estimator is used, and never called when a custom estimator is supplied.
Thank you for contributing to
|
| def check_lapack_svd_safe(n_samples: int, n_features: int, estimator_name: str) -> None: | ||
| """Raise an informative error if a matrix is too large for LAPACK SVD. | ||
|
|
||
| Matrices with more than ``2**31 - 1`` elements may overflow 32-bit integer |
There was a problem hiding this comment.
Not sure how relevant this is in this context but it is not necessary that LAPACK will overflow for more than 2**31 - 1 elements, it is only the case in the default LP64 scipy build. However, now SciPy supports ILP64 builds which can accomodate far large matrices (upto 64 bit integer indices), see these release notes, if we add these checks it will mostly work okay in the default SciPy but will restrict user who specifically use ILP64 SciPy builds.
Therefore, the more robust way would be to query scipy:
import scipy
try:
config = scipy.show_config(mode='dicts')
is_ilp64 = config['Build Dependencies']['blas']['cython_blas_ilp64']
print(f"Is SciPy ILP64? {is_ilp64}")
except KeyError:
print("Is SciPy ILP64? False (Default LP64 profile active)")
Reference Issues/PRs
Fixes #3737.
What does this implement/fix? Explain your changes.
Several classifiers (
RDSTClassifier,RocketClassifier,MultiRocketHydraClassifier)fit a default
RidgeClassifierCVon transformed feature matrices that can exceed the32-bit LAPACK indexing limit for SVD, causing an unhelpful scipy
ValueErroron largedatasets.
This PR:
check_lapack_svd_safe()toaeon.utils.validation, which raises aninformative
ValueErrorbefore fitting when the default estimator is used andthe transformed matrix would overflow 32-bit LAPACK's SVD indexing.
RDSTClassifierandRocketClassifier, ahead of theirdefault
RidgeClassifierCVfit.estimatorparameter onMultiRocketHydraClassifier(previouslyhardcoded to
RidgeClassifierCV), matching the pattern already used byRDSTClassifierRocketClassifier, and applies the same check.existing custom-estimator users is unchanged.
RocketClassifier's internal pipeline was split intotransform/scale/fit steps (it previously used
make_pipeline), since thecheck needs the transformed matrix shape before fitting.
Does your contribution introduce a new dependency? If yes, which one?
No.
Any other comments?
This PR targets only the LAPACK indexing bug not the separate 128GB OOM reported for
MultiRocketHydraClassifieronAustraliaRainfall_disc.AI was used to help understand parts of the codebase and LAPACK, scipy internals
while working through this fix; all changes were written and reviewed by me.
PR checklist
For all contributions
For new estimators and functions
__maintainer__at the top of relevant files and want to be contacted regarding its maintenance. Unmaintained files may be removed. This is for the full file, and you should not add yourself if you are just making minor changes or do not want to help maintain its contents.