Skip to content
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

minor cosmetic changes to follow the work of Mairal, 2009 #4718

Closed
wants to merge 2 commits into from

Conversation

laurentperrinet
Copy link

In the paper from Mairal et al referenced in the script, the set of possible dictionary vectors is not the ball but the convex set of dictionaries whose norm is inferior or equal to one.
This is simply implemented in the normalization step in the function updating the script + corresponding changes in the documentation.

@amueller
Copy link
Member

This is a behavior change, so it is not really cosmetic, right?

@agramfort
Copy link
Member

this will have no effect in practice as at the optimum all atoms will be of norm 1.

@laurentperrinet
Copy link
Author

right, it is not only cosmetic due to the modified normalization step (see my comment on the commit). I was able to test the behavior on the http://scikit-learn.org/stable/auto_examples/decomposition/plot_image_denoising.html example with qualitatively similar results.

@laurentperrinet
Copy link
Author

this will have no effect in practice as at the optimum all atoms will be of norm 1.

right, but is seems important during learning.

@agramfort
Copy link
Member

agramfort commented May 13, 2015 via email

@laurentperrinet
Copy link
Author

I have compared with and without and the results are very similar (I based myself on ?? examples/decomposition/plot_image_denoising_mod.py). I am kind of surprised since the original paper is insisting on that point.
Since the code is already quite complicated, I'll close this PR - if anybody really wants to re-open I'll be glad to assist.

@GaelVaroquaux
Copy link
Member

GaelVaroquaux commented May 22, 2015 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants