10000 DictionaryLearning should support more general constraints on the components · Issue #8994 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

DictionaryLearning should support more general constraints on the components #8994

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.

Sign up for GitHub

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

Jump to bottom
Open
dohmatob opened this issue Jun 6, 2017 · 8 comments
Open

DictionaryLearning should support more general constraints on the components #8994

dohmatob opened this issue Jun 6, 2017 · 8 comments
Labels
module:decomposition New Feature

Comments

@dohmatob
Copy link
Contributor
dohmatob commented Jun 6, 2017 •
edited
Loading

Description

In the reference paper, arbitrary convex compact contraints can be imposed on the dictionary atoms, as long as one can compute the euclidean projection (e.g l2 ball, elastic-net ball, fused-lasso ball, etc.). For example, sparsity can be a crucial constraint if we know from prior knowledge that the dictionary atoms / components should be sparse.

Proposal

The idea would be to allow the DictionaryLearning and MiniBatchDictionaryLearning estimators take strings (e.g "enet ball") or function handles (e.g compute_fused_lasso_projection(locals())), to allow it customize the way the dictionary atoms are updated.

Waiting for

PR #9036 merged.
@vene
Copy link
Member
vene commented Jun 7, 2017

This would be nice; my only concern is that it requires some new-ish API that I don't think we aren't using elsewhere, but this kind of API is really very convenient.

For reference, SAGA in lightning takes a similar API to what you suggest, taking a function handle for the proximal operator which could be user-defined. In lightning there is also a projection module that implements a bunch of such functions that can be used in this proposal.

Just to check that I'm not misunderstanding, the API you are suggesting is that the user passes a single projection function, not a chain of several, right?

@dohmatob
Copy link
Contributor Author
dohmatob commented Jun 7, 2017

@vene: Thanks for the comments.

  • Your understanding of the problem and the proposed solution is exact.

  • About the projection, I indeed mean a single function :).

  • About API, we could do something like:

def _update_dict(*usual_args, projector=None, **usual_kwargs):
...
If projector is None:
    do_default_stuff
else:
    dictionary[:, k] = projector(locals())

@vene
Copy link
Member
vene commented Jun 7, 2017

Yep. It would be even better if projection just takes a vector and returns a vector.

Any kind of arguments to the projection (e.g. the norm of the ball) can be encapsulated inside the projection function using partial application. e.g.

For instance:

def euclidean_ball_proj(x, tau):
    return x /  (tau * np.linalg.norm(x))

...

DictionaryLearning(..., projection=partial(euclidean_ball_proj, tau=5)
  1. is there some other use case which is possible with locals() but not with partial?
  2. is there anywhere else in scikit-learn where a similar api could be possible?

@dohmatob
Copy link
Contributor Author
dohmatob commented Jun 7, 2017 •
edited
Loading

locals() is indeed "nasty" but provides more extensibility (for a proximal-type update, you'll need code[k, k] too, but that's another story...). BTW, there are already a couple of calls to locals() in the dict_learning.py module (e.g in the callbacks), so we can just use it as well.

BTW, concerning the the l2 projection, you're above expression (which appears to be the current implementation in sklearn) is not correct: it's not a projection ;). For one thing, you won't want to shrink the argument x if it's already in the ball; you'd want to leave it untouched. Thus,

def _proj_l2_inplace(x, tau):
    """Computes point y in the L2 ball of radius tau, which is closest to x in the L2 sense.
   """
   norm = sqrt(np.dot(x, x))
   if norm > tau:
       x *= tau / norm
  return x

Agreed ? I'll open another issue to that effect.

@vene
Copy link
Member
vene commented Jun 7, 2017

I guess it's the question of projecting on the ball vs the sphere :P but good point indeed.

@dohmatob
Copy link
Contributor Author
dohmatob commented Jun 7, 2017 •
edited
Loading

For the record, projection onto sphere (surface of ball) is ill-defined (due to lack of convexity). E.g the origin's projection (i.e closest point) is the entire sphere ;). Just take a small nonzero vector (i.e a vector in the neighborhood of the origin), then project it onto the sphere, then let the norm of this input small vector go to zero along arbitrary directions. You obtain the entire sphere.

@dohmatob
Copy link
Contributor Author
dohmatob commented Jun 7, 2017

@vene: If you have time, I'd like to know what you think of PR #9036 too. Thx.

@dohmatob
Copy link
Contributor Author
dohmatob commented Jun 13, 2017 •
edited
Loading

At one point, I will PR this code to implement the feature (generic dict / code updates).
This will depend on how PR #9036 goes.

@jnothman jnothman added this to the 0.20 milestone Jun 14, 2017
@jnothman jnothman removed this from the 0.20 milestone Jun 14, 2017
@rth rth mentioned this issue Apr 14, 2020
Closed
5 tasks
@jeremiedbb jeremiedbb mentioned this issue Dec 16, 2021
Draft
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
module:decomposition New Feature
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@jnothman @vene @dohmatob @cmarmo
0