[ENH] Add `LassoCV` to `nilearn.decoding.DecoderRegressor` #3781

michellewang · 2023-06-27T16:33:16Z

Closes #3769.

Changes proposed in this pull request:

Added 'lasso' and 'lasso_regressor' as supported estimators (sklearn.linear_model.LassoCV())
- For the default hyperparameter values, I used n_alphas=100 instead of directly specifying a list of alphas. This is the default for LassoCV and makes it so that the alphas depend on the data values, so I think this is more desirable than fixing alphas to always be the same, but let me know if you have other thoughts.

I noticed that even though we have Decoder for classification and DecoderRegressor for regression, there is no check for whether the user provides an appropriate estimator (as far as I can tell). As a result, it is technically possible to pass something like 'svc' to DecoderRegressor, which doesn't make sense since (for example) DecoderRegressor uses r2 scoring.

github-actions · 2023-06-27T16:33:32Z

👋 @michellewang Thanks for creating a PR!

Until this PR is ready for review, you can include the [WIP] tag in its title, or leave it as a github draft.

Please make sure it is compliant with our contributing guidelines. In particular, be sure it checks the boxes listed below.

PR has an interpretable title.
PR links to Github issue with mention Closes #XXXX (see our documentation on PR structure)
Code is PEP8-compliant (see our documentation on coding style)
Changelog or what's new entry in doc/changes/latest.rst (see our documentation on PR structure)

For new features:

There is at least one unit test per new function / class (see our documentation on testing)
The new feature is demoed in at least one relevant example.

For bug fixes:

There is at least one test that would fail under the original bug conditions.

We will review it as quick as possible, feel free to ping us with questions if needed.

codecov · 2023-06-27T16:47:12Z

Codecov Report

Merging #3781 (d9f2c37) into main (48aecfa) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #3781   +/-   ##
=======================================
  Coverage   91.74%   91.74%           
=======================================
  Files         134      134           
  Lines       15733    15737    +4     
  Branches     3273     3275    +2     
=======================================
+ Hits        14434    14438    +4     
  Misses        757      757           
  Partials      542      542

Flag	Coverage Δ
macos-latest_3.10	`91.66% <100.00%> (+<0.01%)`	⬆️
macos-latest_3.11	`91.66% <100.00%> (+<0.01%)`	⬆️
macos-latest_3.8	`91.63% <100.00%> (+<0.01%)`	⬆️
macos-latest_3.9	`91.63% <100.00%> (+<0.01%)`	⬆️
ubuntu-latest_3.10	`91.66% <100.00%> (+<0.01%)`	⬆️
ubuntu-latest_3.11	`91.66% <100.00%> (+<0.01%)`	⬆️
ubuntu-latest_3.8	`91.63% <100.00%> (+<0.01%)`	⬆️
ubuntu-latest_3.9	`91.63% <100.00%> (+<0.01%)`	⬆️
windows-latest_3.10	`91.61% <100.00%> (+<0.01%)`	⬆️
windows-latest_3.11	`91.61% <100.00%> (?)`
windows-latest_3.8	`91.57% <100.00%> (+<0.01%)`	⬆️
windows-latest_3.9	`91.57% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
nilearn/_utils/docs.py	`91.86% <ø> (ø)`
nilearn/decoding/decoder.py	`96.92% <100.00%> (+0.03%)`	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

bthirion

Thx, looks good so far.
Maybe we could showcase it in an example ?

bthirion · 2023-06-27T20:27:21Z

nilearn/decoding/decoder.py

+        # the default is to generate 100 alphas based on the data
+        # (alpha values can also be set with the 'alphas' parameter, in which
+        # case 'n_alphas' is ignored)
+        param_grid["n_alphas"] = [100]


100 is a lot, and Lasso fit is expensive. I'd change the default to 30 ---at most.

jeromedockes · 2023-07-21T14:19:16Z

Thanks a lot @michellewang ! LGTM apart from reducing the number of fits as @bthirion requested.

For the default hyperparameter values, I used n_alphas=100 instead of directly specifying a list of alphas.

yes I think that's the best thing to do!

there is no check for whether the user provides an appropriate estimator

that's a good point but let's deal with it in another pr

ymzayek

@michellewang LGTM, other than addressing @bthirion 's comment is there anything else left to do on this one?

michellewang · 2023-08-02T15:08:24Z

@ymzayek yes I am trying to think if it would make sense to add something to an example. This one is about cross-validation but it does classification (not regression) and there is a section showing a param_grid for the CV. Do you think it's worth adding a new example just to show the Decoder with LassoCV regressor? It would be similar to other examples with regressors I think

bthirion

LGTM. Just wondering: is there any example that works better when Lasso is used instead of Ridge ?

ymzayek · 2023-08-03T08:26:13Z

I think we only showcase regression using svr in https://nilearn.github.io/stable/auto_examples/02_decoding/plot_oasis_vbm.html#sphx-glr-auto-examples-02-decoding-plot-oasis-vbm-py. Instead of adding an example I suggest to add information about Lasso in https://nilearn.github.io/stable/decoding/estimator_choice.html#different-linear-models

bthirion · 2023-08-04T07:41:08Z

I simply suggest to try once replacing the SVR with a LassoCV to see the difference in accuracy and computation time. If it not enough or too prohibitive, let's keep the example as-is. Otherwise we can switch here.

michellewang · 2023-08-08T03:55:14Z

Ok, I modified the example to show both the SVR and Lasso results. Let me know if this is what you had in mind

ymzayek · 2023-08-08T08:31:09Z

@michellewang thanks, the example looks good! Can you run it with and without lasso and let us know the difference in computation time? If it's big, I think we should stick to showcasing one of the 2 regressors, and if not we can keep it like this.

Also, can you still add lasso to the list here: https://nilearn.github.io/stable/decoding/estimator_choice.html#different-linear-models

michellewang · 2023-08-08T15:33:42Z

@ymzayek on my laptop it takes 1min 8s without lasso and 1min 30s with Lasso, so adding the Lasso added about 22 seconds. Let me know if that's okay or if we should only keep one of them. The Lasso has higher CV error than the SVR (MAE is ~12 instead of ~10).

ymzayek · 2023-08-09T08:38:28Z

I would say to keep the example as the original (without lasso). However, maybe for users this is useful to see so I will defer to what @bthirion and others think.

bthirion · 2023-08-10T14:50:25Z

If it performs worse and takes more time, we should certainly not include it. Sorry for the useless request.

michellewang · 2023-08-11T15:28:06Z

I reverted the example, no worries @bthirion

ymzayek

Thx @michellewang ! I think this one is good to go. Maybe another review from someone who has already had a look @bthirion or @jeromedockes

jeromedockes

LGTM, thanks a lot @michellewang !

ymzayek · 2023-08-17T09:44:16Z

Thx merging

michellewang added 4 commits June 26, 2023 14:12

add LassoCV to DecoderRegressor

43352a7

update decoder tests

645dca0

add 'lasso'/'lasso_regressor' to regressor options

7dc9939

Merge branch 'main' into decoding/lassocv

8068ac2

update changelog entry

e7881b7

michellewang changed the title ~~Add LassoCV to nilearn.decoding.DecoderRegressor~~ [ENH] Add LassoCV to nilearn.decoding.DecoderRegressor Jun 27, 2023

bthirion reviewed Jun 27, 2023

View reviewed changes

michellewang added 2 commits August 1, 2023 17:36

Merge remote-tracking branch 'upstream/main' into decoding/lassocv

db7da81

use n_alphas=30 as default instead of 100

e5efb34

ymzayek reviewed Aug 2, 2023

View reviewed changes

bthirion reviewed Aug 3, 2023

View reviewed changes

michellewang added 3 commits August 7, 2023 23:49

add lasso regression decoding to example

5604c26

Merge branch 'main' into decoding/lassocv

dc04709

fix formatting in example

31884f2

add lasso(_regressor) to list of estimators in docs

ac3bf4c

michellewang added 2 commits August 11, 2023 11:25

revert example

7589fbb

fix formatting

78c9a3e

Merge remote-tracking branch 'upstream/main' into decoding/lassocv

d9f2c37

ymzayek approved these changes Aug 17, 2023

View reviewed changes

jeromedockes approved these changes Aug 17, 2023

View reviewed changes

ymzayek merged commit b7a7adf into nilearn:main Aug 17, 2023
28 of 29 checks passed

michellewang deleted the decoding/lassocv branch August 17, 2023 12:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ENH] Add `LassoCV` to `nilearn.decoding.DecoderRegressor` #3781

[ENH] Add `LassoCV` to `nilearn.decoding.DecoderRegressor` #3781

michellewang commented Jun 27, 2023

github-actions bot commented Jun 27, 2023

codecov bot commented Jun 27, 2023 •

edited

bthirion left a comment

bthirion Jun 27, 2023

jeromedockes commented Jul 21, 2023

ymzayek left a comment

michellewang commented Aug 2, 2023

bthirion left a comment

ymzayek commented Aug 3, 2023

bthirion commented Aug 4, 2023

michellewang commented Aug 8, 2023

ymzayek commented Aug 8, 2023

michellewang commented Aug 8, 2023

ymzayek commented Aug 9, 2023

bthirion commented Aug 10, 2023

michellewang commented Aug 11, 2023

ymzayek left a comment

jeromedockes left a comment

ymzayek commented Aug 17, 2023

[ENH] Add LassoCV to nilearn.decoding.DecoderRegressor #3781

[ENH] Add LassoCV to nilearn.decoding.DecoderRegressor #3781

Conversation

michellewang commented Jun 27, 2023

github-actions bot commented Jun 27, 2023

codecov bot commented Jun 27, 2023 • edited

Codecov Report

bthirion left a comment

Choose a reason for hiding this comment

bthirion Jun 27, 2023

Choose a reason for hiding this comment

jeromedockes commented Jul 21, 2023

ymzayek left a comment

Choose a reason for hiding this comment

michellewang commented Aug 2, 2023

bthirion left a comment

Choose a reason for hiding this comment

ymzayek commented Aug 3, 2023

bthirion commented Aug 4, 2023

michellewang commented Aug 8, 2023

ymzayek commented Aug 8, 2023

michellewang commented Aug 8, 2023

ymzayek commented Aug 9, 2023

bthirion commented Aug 10, 2023

michellewang commented Aug 11, 2023

ymzayek left a comment

Choose a reason for hiding this comment

jeromedockes left a comment

Choose a reason for hiding this comment

ymzayek commented Aug 17, 2023

[ENH] Add `LassoCV` to `nilearn.decoding.DecoderRegressor` #3781

[ENH] Add `LassoCV` to `nilearn.decoding.DecoderRegressor` #3781

codecov bot commented Jun 27, 2023 •

edited