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

making dictionary learning closer to the SparseNet algorithm #4693

Open
wants to merge 28 commits into
from

Conversation

Projects
None yet
5 participants
@laurentperrinet

The dictionary learning algorithm was assuming that the norm of the filters was equal to one. By using a heuristic to control for the norm of the filters, we allow for a more equilibrated learning. The implementation is a simplification of the one used in the original paper from Olshausen.

The dictionary learning is tested in:
http://blog.invibe.net/posts/2015-05-05-reproducing-olshausens-classical-sparsenet.html
and this PR is tested in:
http://blog.invibe.net/posts/2015-05-06-reproducing-olshausens-classical-sparsenet-part-2.html

laurentperrinet added some commits May 8, 2015

making dictionary learning closer to the SparseNet algorithm \n The …
…dictionary learning algorithm was assuming that the norm of the filters was equal to one. By using a heuristic to control for the norm of the filters, we allow for a more equilibrated learning. The implementation is a simplification of the one used in the original paper from Olshausen.
@GaelVaroquaux

This comment has been minimized.

Show comment
Hide comment
@GaelVaroquaux

GaelVaroquaux May 8, 2015

Member

Thanks for the contribution.

The algorithm and problem formulation that we use is a fairly standard one (albeit different from the historical one of Olshausen). What is the practical benefit of this formulation?

Also, shouldn't the documentation be changed? The meaning of the gain parameter as well as its impact on the results should be explained in the narrative documentation.

Finally, isn't this a change in behavior? If it's the case, we need to have an option that maintains the old behavior, and a didactic example that shows the difference between the settings.

Member

GaelVaroquaux commented May 8, 2015

Thanks for the contribution.

The algorithm and problem formulation that we use is a fairly standard one (albeit different from the historical one of Olshausen). What is the practical benefit of this formulation?

Also, shouldn't the documentation be changed? The meaning of the gain parameter as well as its impact on the results should be explained in the narrative documentation.

Finally, isn't this a change in behavior? If it's the case, we need to have an option that maintains the old behavior, and a didactic example that shows the difference between the settings.

@laurentperrinet

This comment has been minimized.

Show comment
Hide comment
@laurentperrinet

laurentperrinet May 8, 2015

Thanks for the comments!

First off, I made sure that by default there is no change to the results if the gain_rate setting is not used or set to zero. As such, there is no change in the behavior of the examples that I took from the sklearn website.

Now, you are right that this gain vector certainly needs some clarification. I will try to update the _update_gain function to explicit that.

Thanks for the comments!

First off, I made sure that by default there is no change to the results if the gain_rate setting is not used or set to zero. As such, there is no change in the behavior of the examples that I took from the sklearn website.

Now, you are right that this gain vector certainly needs some clarification. I will try to update the _update_gain function to explicit that.

@GaelVaroquaux

This comment has been minimized.

Show comment
Hide comment
@GaelVaroquaux

GaelVaroquaux May 8, 2015

Member
Member

GaelVaroquaux commented May 8, 2015

@laurentperrinet

This comment has been minimized.

Show comment
Hide comment
@laurentperrinet

laurentperrinet May 8, 2015

OK, thanks for the answer. I will try to make a minimal example of how it works.

OK, thanks for the answer. I will try to make a minimal example of how it works.

laurentperrinet added some commits May 13, 2015

making dictionary learning closer to the SparseNet algorithm : docum…
…enting the _update_gain function / Mairal 2009 uses norm inferior or equal to 1
making dictionary learning closer to the SparseNet algorithm : clean…
…ly separating gradient learning (not yet implemented) and Dictionary learning from Mairal 2009
making dictionary learning closer to the SparseNet algorithm : clean…
…ly separating gradient learning (implementing...) and Dictionary learning from Mairal 2009
making dictionary learning closer to the SparseNet algorithm : clean…
…ly separating gradient learning (implemented and testing) and Dictionary learning from Mairal 2009
Merge branch 'mairal' of https://github.com/meduz/scikit-learn into s…
…parsenet

* 'mairal' of https://github.com/meduz/scikit-learn:
  minor cosmetic changes to follow the work of Mairal, 2009
@laurentperrinet

This comment has been minimized.

Show comment
Hide comment
@laurentperrinet

laurentperrinet May 13, 2015

Owner

this is the only line which is a behavior change. should improve convergence according to Mairal et al, 2009

this is the only line which is a behavior change. should improve convergence according to Mairal et al, 2009

laurentperrinet added some commits May 20, 2015

Merge branch 'master' into sparsenet
* master: (845 commits)
  MAINT: use xrange from six
  Update my website on whats_new.rst
  colorbar doesn't handle label parameter
  DOC Add description for UndefinedMetricWarning
  fontsize legend parameter is not available for matplotlib < 1.3
  FIX: gaussian_process will be deprecated in 0.19, not 0.18
  doc: fix typo
  FIX/ENH Make the moved class resilient to the deprecation patching
  MAINT move deprecated into deprecation.py
  MAINT Deprecated the UndefinedMetricWarning at sklearn.metrics.base
  DOC/SPHINX Add template for classes which don't have init (Exception classes)
  MAINT move custom error/warning classes into sklearn.exceptions
  ENH add pomegranate to related
  DOC: added entry for new GP implementation
  TST test_lml_precomputed() checks only for equality in first 7 digits
  ADD Mixins for normalized and stationary kernels
  FIX Enforcing y to be numeric in GPR and fixing python3 issue with maps
  ENH GPR.log_marginal_likelihood() returns the current log-likelihood if no theta vector is provided
  FIX String comparison via equality and not identity in Hyperparameter.__init__
  MISC Changing default for copy_X_train to True in GaussianProcessClassifier
  ...
@laurentperrinet

This comment has been minimized.

Show comment
Hide comment
@laurentperrinet

laurentperrinet Oct 21, 2015

I have cleaned-up the changes to just focus on the replication of Olshausen's SparseNet algorithm. An example script is available @ https://github.com/meduz/SHL_scripts/blob/master/src/shl_scripts.py and the result may be seen @ https://github.com/meduz/SHL_scripts/blob/67a3286ba8b62a4390dd976b4fa305cadb9228d2/index.ipynb

I have cleaned-up the changes to just focus on the replication of Olshausen's SparseNet algorithm. An example script is available @ https://github.com/meduz/SHL_scripts/blob/master/src/shl_scripts.py and the result may be seen @ https://github.com/meduz/SHL_scripts/blob/67a3286ba8b62a4390dd976b4fa305cadb9228d2/index.ipynb

@amueller

This comment has been minimized.

Show comment
Hide comment
@amueller

amueller Dec 10, 2015

Member

why did you add another class? And could you add an example (to the examples folder) showing the difference to the current version?

Member

amueller commented Dec 10, 2015

why did you add another class? And could you add an example (to the examples folder) showing the difference to the current version?

laurentperrinet added some commits Dec 11, 2015

Merge branch 'master' of https://github.com/scikit-learn/scikit-learn
…into sparsenet

* 'master' of https://github.com/scikit-learn/scikit-learn: (283 commits)
  update file header in affinity_propagation.py
  DOC: Fix spelling
  adding 0.17 support link
  adding missing 0.16 doc link and removing broken support links
  Fixed unit test in sklearn/tree/tests/test_tree.py.
  fix link to contributing in website drop-down
  Fix Kernel PCA docstring to reflect how remove_zero_eig defaults to false
  removed the doc line 'Only applies if analyzer == 'word'' from CountVectorizer analyzer'
  Parallelize embarrassingly parallel loop in RFECV.fit
  Changed orders of docstring descriptions for polynomial_kernel
  Added more gamma info for sigmoid, polynomial kernels
  Added default gamma info for laplacian_kernel and rbf_kernel
  Add link to multiisotonic
  ENH add multinomial SAG solver for LogisticRegression
  change comment, printed comment, and code behavior to all reflect computation of mean squared error
  residual sum of squares should be computed by summing the squared differences instead of taking the mean.
  repaired dead link
  It's better to fix a comment
  Fix coveralls report sending
  edited docstring for clarity regarding the naming of the pipeline components.
  ...
@laurentperrinet

This comment has been minimized.

Show comment
Hide comment
@laurentperrinet

laurentperrinet Dec 11, 2015

Thanks for your answer, this helps improving the PR.

  • I have added a class to follow what was being done in the dict_learning.py file without interfering with existing methods.
  • I have now added an example script to show the 2 different set of filters with the existing method and that of Olshausen. To load an image I used imageio, I hope this fits with guidelines within sklearn.

Cheers,
Laurent

Thanks for your answer, this helps improving the PR.

  • I have added a class to follow what was being done in the dict_learning.py file without interfering with existing methods.
  • I have now added an example script to show the 2 different set of filters with the existing method and that of Olshausen. To load an image I used imageio, I hope this fits with guidelines within sklearn.

Cheers,
Laurent

@amueller

This comment has been minimized.

Show comment
Hide comment
@amueller

amueller Dec 11, 2015

Member

can you please paste the image here?
Unfortunately we can't use any non-standard libraries except the dependencies (numpy, scipy, matplotlib in the examples).

Member

amueller commented Dec 11, 2015

can you please paste the image here?
Unfortunately we can't use any non-standard libraries except the dependencies (numpy, scipy, matplotlib in the examples).

@laurentperrinet

This comment has been minimized.

Show comment
Hide comment
@laurentperrinet

laurentperrinet Dec 15, 2015

I transformed the script to only use the standard library - should be better now.

here are the results without and with the homeostasis used by Olshausen compared to the one used so far in sklearn:

png image 516 x 567 pixels
homeo png image 516 x 567 pixels

I commented this in http://blog.invibe.net/posts/2015-12-11-reproducing-olshausens-classical-sparsenet-part-4.html

I transformed the script to only use the standard library - should be better now.

here are the results without and with the homeostasis used by Olshausen compared to the one used so far in sklearn:

png image 516 x 567 pixels
homeo png image 516 x 567 pixels

I commented this in http://blog.invibe.net/posts/2015-12-11-reproducing-olshausens-classical-sparsenet-part-4.html

ii = iter_offset - 1
for ii, this_X in zip(range(iter_offset, iter_offset + n_iter), batches):
dt = (time.time() - t0)

This comment has been minimized.

@podshumok

podshumok Dec 16, 2015

Contributor

maybe you meant timeit.default_timer()?

@podshumok

podshumok Dec 16, 2015

Contributor

maybe you meant timeit.default_timer()?

This comment has been minimized.

@laurentperrinet

laurentperrinet Dec 16, 2015

this bit of code is not related to this PR and is repeated for instance in dict_learning, dict_learning_online and dict_learning_grad

@laurentperrinet

laurentperrinet Dec 16, 2015

this bit of code is not related to this PR and is repeated for instance in dict_learning, dict_learning_online and dict_learning_grad

@laurentperrinet

This comment has been minimized.

Show comment
Hide comment
@laurentperrinet

laurentperrinet Nov 4, 2016

Any news on this PR? Thanks!

Any news on this PR? Thanks!

@jnothman

I hope to look at more of this soon, though it is outside of my expertise.

Show outdated Hide outdated examples/decomposition/plot_sparsenet.py
This example compares the set of filters obtained by sparse coding
algorithm described by Olshausen and Field in Nature, vol. 381,
pp. 607-609 with that obtained using :ref:`DictionaryLearning`. The

This comment has been minimized.

@jnothman

jnothman Dec 8, 2016

Member

I think you want :class:sklearn.decomposition.DictionaryLearning

@jnothman

jnothman Dec 8, 2016

Member

I think you want :class:sklearn.decomposition.DictionaryLearning

This comment has been minimized.

@laurentperrinet

laurentperrinet Jan 11, 2017

I based my writing on the plot_image_denoising.py script which is quite similar.

@laurentperrinet

laurentperrinet Jan 11, 2017

I based my writing on the plot_image_denoising.py script which is quite similar.

Show outdated Hide outdated examples/decomposition/plot_sparsenet.py
without homeostasis leads to the emergence of edge-like filters but that a
number of filters are not learned: They still are closer to noise and are
unlikely to be selected by the sparse coding algorithm without homeostasy.
The heuristics used here follows the assumption that during learning,

This comment has been minimized.

@jnothman

jnothman Dec 8, 2016

Member

follows -> follow; or heuristics -> heuristic

@jnothman

jnothman Dec 8, 2016

Member

follows -> follow; or heuristics -> heuristic

This comment has been minimized.

laurentperrinet added some commits Jan 3, 2017

Merge pull request #1 from scikit-learn/master
merging the master branch of sklearn into my branch
@jnothman

I've not looked at the code, but there is certainly an appreciable difference between the images in your plot above.

Please mention this to the narrative doc in doc/modules/decomposition.rst. Also add the new public API to doc/modules/classes.rst

Show outdated Hide outdated .gitignore
@@ -62,3 +62,13 @@ benchmarks/bench_covertype_data/
!*/src/*.cpp
*.sln
*.pyproj
sklearn/sklearn.egg-info/top_level.txt

This comment has been minimized.

@jnothman

jnothman Dec 12, 2017

Member

No one should be building within the sklearn directory. Please revert these changes.

@jnothman

jnothman Dec 12, 2017

Member

No one should be building within the sklearn directory. Please revert these changes.

This comment has been minimized.

@laurentperrinet

laurentperrinet Jan 1, 2018

Ok, I have reverted this change to the .gitignore file

@laurentperrinet

laurentperrinet Jan 1, 2018

Ok, I have reverted this change to the .gitignore file

@jnothman

This comment has been minimized.

Show comment
Hide comment
@jnothman

jnothman Dec 12, 2017

Member

You have a logic failure coming out in the tests when fitting:

Traceback (most recent call last):
  File "/home/travis/miniconda/envs/testenv/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/travis/build/scikit-learn/scikit-learn/sklearn/utils/testing.py", line 741, in __call__
    return self.check(*args, **kwargs)
  File "/home/travis/build/scikit-learn/scikit-learn/sklearn/utils/testing.py", line 292, in wrapper
    return fn(*args, **kwargs)
  File "/home/travis/build/scikit-learn/scikit-learn/sklearn/utils/estimator_checks.py", line 460, in check_dict_unchanged
    estimator.fit(X, y)
  File "/home/travis/build/scikit-learn/scikit-learn/sklearn/decomposition/dict_learning.py", line 1482, in fit
    return_n_iter=True
  File "/home/travis/build/scikit-learn/scikit-learn/sklearn/decomposition/dict_learning.py", line 739, in dict_learning_grad
    residual = this_X - np.dot(this_code.T, dictionary.T)
ValueError: matrices are not aligned
Member

jnothman commented Dec 12, 2017

You have a logic failure coming out in the tests when fitting:

Traceback (most recent call last):
  File "/home/travis/miniconda/envs/testenv/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/travis/build/scikit-learn/scikit-learn/sklearn/utils/testing.py", line 741, in __call__
    return self.check(*args, **kwargs)
  File "/home/travis/build/scikit-learn/scikit-learn/sklearn/utils/testing.py", line 292, in wrapper
    return fn(*args, **kwargs)
  File "/home/travis/build/scikit-learn/scikit-learn/sklearn/utils/estimator_checks.py", line 460, in check_dict_unchanged
    estimator.fit(X, y)
  File "/home/travis/build/scikit-learn/scikit-learn/sklearn/decomposition/dict_learning.py", line 1482, in fit
    return_n_iter=True
  File "/home/travis/build/scikit-learn/scikit-learn/sklearn/decomposition/dict_learning.py", line 739, in dict_learning_grad
    residual = this_X - np.dot(this_code.T, dictionary.T)
ValueError: matrices are not aligned
Show outdated Hide outdated sklearn/decomposition/dict_learning.py
Returns the instance itself.
"""
random_state = check_random_state(self.random_state)
X = check_array(X)

This comment has been minimized.

@jnothman

jnothman Dec 12, 2017

Member

you should perhaps use ensure_min_samples here, as the code is currently failing for a single-sample input.

@jnothman

jnothman Dec 12, 2017

Member

you should perhaps use ensure_min_samples here, as the code is currently failing for a single-sample input.

laurentperrinet added some commits Jan 1, 2018

using at least 2 samples
adding an option in ``check_array`` to have a minimum number of samples
@jnothman

This comment has been minimized.

Show comment
Hide comment
@jnothman

jnothman Jan 1, 2018

Member

This pull request introduces 1 alert - view on lgtm.com

new alerts:

  • 1 for Variable defined multiple times

Comment posted by lgtm.com

Member

jnothman commented Jan 1, 2018

This pull request introduces 1 alert - view on lgtm.com

new alerts:

  • 1 for Variable defined multiple times

Comment posted by lgtm.com

laurentperrinet added some commits Jan 2, 2018

Fixed alert and documentation
Changed the example script to avoid alert and fixed reference to the
class.
Fixed alert and documentation
Fixed reference to the class.
Minimal documentation
Mentioning this to the narrative doc in doc/modules/decomposition.rst. Also adding to the new
public API in doc/modules/classes.rst
@jnothman

A few things here need testing (and there are some lines longer than 79 chars)

random_state = check_random_state(random_state)
if n_jobs == -1:
n_jobs = cpu_count()

This comment has been minimized.

@jnothman

jnothman Feb 1, 2018

Member

I don't think you need to do this as it will be validated downstream

@jnothman

jnothman Feb 1, 2018

Member

I don't think you need to do this as it will be validated downstream

norm = np.sqrt(np.sum(dictionary**2, axis=1)).T
if verbose == 1:
print('[dict_learning]', end=' ')

This comment has been minimized.

@jnothman

jnothman Feb 1, 2018

Member

This has no test coverage

@jnothman

jnothman Feb 1, 2018

Member

This has no test coverage

X_train = X.copy()
random_state.shuffle(X_train)
else:
X_train = X

This comment has been minimized.

@jnothman

jnothman Feb 1, 2018

Member

This has no test coverage

@jnothman

jnothman Feb 1, 2018

Member

This has no test coverage

# Maybe we need a stopping criteria based on the amount of
# modification in the dictionary
if callback is not None:
callback(locals())

This comment has been minimized.

@jnothman

jnothman Feb 1, 2018

Member

This has no test coverage.

@jnothman

jnothman Feb 1, 2018

Member

This has no test coverage.

print('Learning code...', end=' ')
elif verbose == 1:
print('|', end=' ')
code = sparse_encode(X, dictionary.T, algorithm=method, alpha=alpha,

This comment has been minimized.

@jnothman

jnothman Feb 1, 2018

Member

This has no test coverage

@jnothman

jnothman Feb 1, 2018

Member

This has no test coverage

if return_n_iter:
return dictionary.T, ii - iter_offset + 1
else:
return dictionary.T

This comment has been minimized.

@jnothman

jnothman Feb 1, 2018

Member

This has no test coverage

@jnothman

jnothman Feb 1, 2018

Member

This has no test coverage

@jnothman

A few things here need testing (and there are some lines longer than 79 chars)

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