FIX: tol in _spectral_embedding.py #21194
Conversation
Fix the typo in tol from -15 to -5
Would it be possible to add a non regression test inspired by #21147 but using a small synthetic dataset? |
There was a problem hiding this comment.
LGTM since the existing tests do not fail. It would be great to have a new non-regression test as explained above but I would not hold the merge of this PR for this if it's too complex to craft such a non-regression test (given the fact that it should also be fast to run, ideally in less than 100ms).
|
We also need an entry for the changelog targeting 1.0.1 (in |
Added info on #21194
A unit test added
moved for the proper order
wrong file edited! Back to the original
|
@ogrisel - It's difficult to make a direct unit test without using that specific matrix presented in the original file, since the example needs to reproduce the logic where the default solver, |
|
+1 for new issues to analyse the bugs you found while trying to work on a new test. |
SpectralClustering breaks on small-size low-rank data #21274 |
Fix the typo in tol from -15 to -5, making it consistent with other calls of lobpcg here.
Reference Issues/PRs
Fixes #21147
What does this implement/fix? Explain your changes.
Excessively small tol in lobpcg not only makes it unnecessary slower, increasing the number of iterations for no good reason, but can also make it fail due to numerical instabilities, as found in #21147
Any other comments?