Skip to content

Comments

Added 2 algorithms to solve LASSO-like problems: restarting FISTA and POGM'#7

Merged
tomMoral merged 5 commits intobenchopt:masterfrom
zaccharieramzi:pogm-fista
Jan 17, 2021
Merged

Added 2 algorithms to solve LASSO-like problems: restarting FISTA and POGM'#7
tomMoral merged 5 commits intobenchopt:masterfrom
zaccharieramzi:pogm-fista

Conversation

@zaccharieramzi
Copy link
Contributor

These 2 algorithms are implemented in ModOpt, where they were intended to solve inverse problems (this is why the notations might seem a bit funny).

The defaults are those from this paper (that I wrote at the beginning of my thesis so please don't be mean).

I saw that none of the other solvers had docs about what they were implementing (i.e. a citation) or their potential hyperparameters, so I didn't do it for those 2, but happy to do it. Otherwise, all there is to know is inside the ModOpt package.

I still need to check that the conda install method is working.

Should I post some results on boston and simulated datasets here?

@zaccharieramzi
Copy link
Contributor Author

I guess since the tests are passing, the conda install method is fine.

@agramfort
Copy link
Contributor

agramfort commented Dec 16, 2020 via email

@zaccharieramzi
Copy link
Contributor Author

Sure which ones do you have in mind?

boston_reg01

Here is for example the results on the Boston dataset against CD, Celer, L-BFGS-B and PGD, with reg=0.1 and max-runs=10.

The simulated ones are quite slow (and fista greedy diverges with the current hyperparameter setup in some cases), but I can run them if they make things clearer.

@agramfort
Copy link
Contributor

agramfort commented Dec 16, 2020 via email

@zaccharieramzi
Copy link
Contributor Author

Well FISTA greedy uses a strategy where the step size begins by being superior to the inverse of the Lipschitz constant. There is a parameter telling you by how much it should be bigger (in this case 1.3x).

Here, I didn't want to tune the hyperparameters, so I set it at the recommended value which is supposed to work well in most settings (like for MRI reconstruction, on the Boston dataset, or some simulated cases). Unfortunately for some cases of simulated data, it doesn't and the algorithm diverges.
I could try to fix it by tuning this hyperparameter but it feels like the comparison with other algorithms wouldn't be fair.

Re simulated: do you want me to run the benchmarks or is the boston one enough?

@agramfort
Copy link
Contributor

agramfort commented Dec 16, 2020 via email

Copy link
Member

@tomMoral tomMoral left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A small comment on the initialization of the method. To be consistent with other, you need to compute L in the run.

@zaccharieramzi
Copy link
Contributor Author

@tomMoral Done ! (The reason it doesn't seem very natural code-wise is because those 2 algorithms were implemented in the spirit of the inverse problem framework, where the X matrix is usually know well in advance).

If I may, I noticed that one another algorithm made a computation before run: In glmnet, a matrix-vector product is computed in the set_objective. Maybe this should also be moved some other place.

Copy link
Member

@tomMoral tomMoral left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the long delay @zaccharieramzi !!

LGTM! merging

@tomMoral tomMoral merged commit bf3548c into benchopt:master Jan 17, 2021
@tomMoral
Copy link
Member

For the matrix vector product in glmnet, this is a bit different because this our way to set the lambda parameter for this solver to the same value as for the others, not something that is necessary for the solver. So I would say this is correct as is but I am ready to discuss if you think otherwise.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants