-
-
Notifications
You must be signed in to change notification settings - Fork 26.5k
FIX online updates in MiniBatchDictionaryLearning #25354
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
Conversation
| >>> np.mean(X_transformed == 0) < 0.5 | ||
| True |
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.
I changed that because there was doctest failures in some jobs. The result was slightly under 0.39 in some jobs and slightly over 0.39 in others.
Since the purpose is to show that the matrix has some sparsity, I think this change is acceptable.
glemaitre
left a comment
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.
Yes it makes sense looking at the algorithm. LGTM.
Co-authored-by: Olivier Grisel <[email protected]>
Co-authored-by: Olivier Grisel <[email protected]>
Co-authored-by: Olivier Grisel <[email protected]>
Co-authored-by: Olivier Grisel <[email protected]>
The online updates of the sufficient statistics should be normalized by batch_size since it's an average over the batch. When fitting with a constant batch size it doesn't matter but when using partial_fit on batches of different sizes it has an impact.
It's described in the original paper of Mairal
Online Learning for Matrix Factorization and Sparse Coding, page 9 of the PDF, in section 3.4.3 mini-batch extension. (https://www.jmlr.org/papers/volume11/mairal10a/mairal10a.pdf)I computed the final objective function for 100 datasets randomly generated, fitted using partial_fit on batch of various sizes and the objective function is constantly lower with this fix (code below).
It's not really possible to add a test for this, but I think the results above are convincing enough.