-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
Fix seed sensitivity of test_fastica_eigh_low_rank_warning #29612
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
Fix seed sensitivity of test_fastica_eigh_low_rank_warning #29612
Conversation
…eeds] test_fastica_eigh_low_rank_warning
Apparently this fixed the problem on Linux with MKL but not on macos with MKL... EDIT: not sure in retrospect. See the edited description of the PR. |
test_fastica_eigh_low_rank_warning
…in test [azure parallel] [all random seeds] test_fastica_simple test_fit_transform test_inverse_transform test_fastica_simple_different_solvers test_fastica_eigh_low_rank_warning
fc42de6
to
8cf7261
Compare
8cf7261
to
ae6c18b
Compare
Base on the CI report in 4981cbf, this fixed the sensitivity problem in the test. Note that to fix this test failure, I actually had to change the code in |
Self review fixes.
/cc @lesteve |
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.
LGTM
As originally reported in #26802 and more recently in #29609.
I cannot reproduce on my local setup, so let's try via the CI triggered by this draft PR.
EDIT: the
ConvergenceWarning
happens later (in the fast ica solver itself) while the expectedUserWarning
should be raised earlier, in the whitening step, irrespective of whetherFastICA
converges or not. I therefore think that the cause of the problem is that our eps-based detection of rank deficient case is too low to be platform agnostic.Fixes #26802.