FEA Add Nonnegative LinearRegression #17578

cmarmo · 2020-06-12T16:48:02Z

Reference Issues/PRs

Fixes #8191. Resolves #11076, resolve #8428

What does this implement/fix? Explain your changes.

Adds parameters (positive and ~~maxiter~~) to
LinearRegression consistent with Lasso and ElasticNet.

Any other comments?

#11076 was already approved by @TomDLT , I think I have addressed @rth comments.

Still missing

User Guide entry
Example
what's new

…ugh `positive` parameter. Similar to PR scikit-learn#8428, but adds parameters (`positive` and `max_iter`) to `LinearRegression` consistent with `Lasso` and `ElasticNet`. Uses `scipy.optimize.nnls` under the hood. **note** `scipy.optimize.nnls` cannot accept a sparse matrix for `X` or `y`. Passing sparse `X` throws error through `check_X_y`.

Remove `max_iter` parameter since `scipy.optimize.nnls` does not return `n_iter`.

Remove `_residues` attribute and unnecessary `np.ravel()`.

TomDLT

As per my previous comments:

This needs to appear in the user guide. Maybe also add an example, as here?
You will also need to add a mention in doc/whats_new/v0.24.rst.

sklearn/linear_model/_base.py

sklearn/linear_model/tests/test_base.py

sklearn/linear_model/_base.py

NicolasHug

thanks @cmarmo , mostly looks good

I think we could also have a test showing that we get different results when positive is True vs False

sklearn/linear_model/_base.py

sklearn/linear_model/tests/test_base.py

doc/modules/linear_model.rst

examples/linear_model/plot_nnls.py

sklearn/linear_model/_base.py

Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>

sklearn/linear_model/_base.py

doc/modules/linear_model.rst

examples/linear_model/plot_nnls.py

sklearn/linear_model/tests/test_base.py

NicolasHug

Thanks @cmarmo , made another pass but mostly looks good

examples/linear_model/plot_nnls.py

doc/modules/linear_model.rst

jnothman

I wouldn't mind a test showing positive=False/True makes no difference if positive=False already gets all non-negative coefficients.

But LGTM.

cmarmo · 2020-08-03T09:08:27Z

@jnothman, I have added the test.
Perhaps, @TomDLT or @NicolasHug have some time to check again? Thanks!

sklearn/linear_model/tests/test_base.py

NicolasHug

Thanks @cmarmo , minor comments but LGTM

examples/linear_model/plot_nnls.py

sklearn/linear_model/tests/test_base.py

Co-authored-by: Nicolas Hug <contact@nicolas-hug.com> Co-authored-by: Joseph Knox <jknox13@uw.edu> Co-authored-by: Joseph Knox <joseph.edward.knox@gmail.com>

jknox13 and others added 9 commits May 7, 2018 17:12

Update base.py

a778ed6

Remove `max_iter` parameter since `scipy.optimize.nnls` does not return `n_iter`.

doctest fix

bd2614b

Implememnts edits suggested by @jnothman.

2ef0b9c

Fix comments raised by @agramfort.

436e23f

Address @agramfort review.

39ff0d1

Remove `_residues` attribute and unnecessary `np.ravel()`.

Merge branch 'master' into LinearRegression_nnls_feature

cda0263

Sync with upstream.

fff6b4d

Address rth comments.

0c1ae9b

github-actions bot added the module:linear_model label Jun 12, 2020

Fix docstrings.

f37fffb

TomDLT reviewed Jun 12, 2020

View reviewed changes

sklearn/linear_model/_base.py Outdated Show resolved Hide resolved

sklearn/linear_model/_base.py Outdated Show resolved Hide resolved

sklearn/linear_model/tests/test_base.py Show resolved Hide resolved

Put new parameter at the end.

51df7dc

jnothman requested changes Jun 13, 2020

View reviewed changes

sklearn/linear_model/_base.py Show resolved Hide resolved

cmarmo added 3 commits June 14, 2020 15:59

Store residues in single target case.

8b9ba07

Add optimize.nnls maxiter as input parameter.

6a40d90

Add test on positivity of coefficients.

3d829e9

cmarmo changed the title ~~[MRG] Nonnegative LinearRegression through positive parameter.~~ [WIP] Nonnegative LinearRegression through positive parameter. Jun 14, 2020

cmarmo added 6 commits June 14, 2020 17:11

Rename max_iter to maxiter.

da961d9

Remove maxiter keywords in nnls().

174de83

Add example from NelleV PR scikit-learn#8428.

4e31258

Update cell separator in the example.

6a55fea

Revert maxiter parameter.

3342da1

Update what's new and User Guide.

86a1d47

cmarmo changed the title ~~[WIP] Nonnegative LinearRegression through positive parameter.~~ [MRG] Nonnegative LinearRegression through positive parameter. Jun 15, 2020

NicolasHug reviewed Jun 15, 2020

View reviewed changes

cmarmo and others added 4 commits June 15, 2020 13:52

Update examples/linear_model/plot_nnls.py

95dd390

Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>

Update doc/modules/linear_model.rst

6bca08c

Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>

Update doc/modules/linear_model.rst

ca05ce6

Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>

Remove description in versionadded.

beafc76

cmarmo added 5 commits June 24, 2020 14:47

Sync with upstream.

5cc4f48

Sync with upstream.

8f6f59d

Merge branch 'master' into LinearRegression_nnls_feature

353caab

Add test for different results for positive=True and posiive=False.

7414b28

Use y instead of Y in non multioutput tests.

837e756

NicolasHug reviewed Jul 5, 2020

View reviewed changes

examples/linear_model/plot_nnls.py Outdated Show resolved Hide resolved

doc/modules/linear_model.rst Outdated Show resolved Hide resolved

cmarmo added 7 commits July 6, 2020 10:13

Merge branch 'master' into LinearRegression_nnls_feature

2e0c3cd

Address comments.

03093bc

Resolve conflicts.

6d61e82

Remove unused internal reference.

8627b9f

Merge branch 'master' into LinearRegression_nnls_feature

6ec81fb

Fix conflicts.

a75e090

Merge branch 'master' into LinearRegression_nnls_feature

2c11edf

cmarmo requested a review from jnothman August 1, 2020 13:21

jnothman approved these changes Aug 1, 2020

View reviewed changes

cmarmo added 2 commits August 3, 2020 10:12

Merge branch 'master' into LinearRegression_nnls_feature

a5c110b

Add test for non-negative problems.

fecc67a

jnothman approved these changes Aug 3, 2020

View reviewed changes

sklearn/linear_model/tests/test_base.py Show resolved Hide resolved

NicolasHug approved these changes Aug 3, 2020

View reviewed changes

examples/linear_model/plot_nnls.py Show resolved Hide resolved

sklearn/linear_model/tests/test_base.py Outdated Show resolved Hide resolved

sklearn/linear_model/tests/test_base.py Outdated Show resolved Hide resolved

cmarmo and others added 5 commits August 3, 2020 18:37

Merge branch 'master' into LinearRegression_nnls_feature

4764f7e

Address comments.

d9ff696

Fix comment about sparsity in test.

a33a918

Merge branch 'master' into LinearRegression_nnls_feature

9f8e12d

empty commit to trigger CI

485d21f

NicolasHug changed the title ~~[MRG] Nonnegative LinearRegression through positive parameter.~~ FEA Add Nonnegative LinearRegression Aug 6, 2020

NicolasHug merged commit 075d424 into scikit-learn:master Aug 6, 2020
7 checks passed

cmarmo deleted the LinearRegression_nnls_feature branch August 6, 2020 13:53

FEA Add Nonnegative LinearRegression #17578

FEA Add Nonnegative LinearRegression #17578

cmarmo commented Jun 12, 2020 •

edited

TomDLT left a comment

NicolasHug left a comment

NicolasHug left a comment

jnothman left a comment

cmarmo commented Aug 3, 2020

NicolasHug left a comment

FEA Add Nonnegative LinearRegression #17578

FEA Add Nonnegative LinearRegression #17578

Conversation

cmarmo commented Jun 12, 2020 • edited

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

Still missing

TomDLT left a comment

Choose a reason for hiding this comment

NicolasHug left a comment

Choose a reason for hiding this comment

NicolasHug left a comment

Choose a reason for hiding this comment

jnothman left a comment

Choose a reason for hiding this comment

cmarmo commented Aug 3, 2020

NicolasHug left a comment

Choose a reason for hiding this comment

cmarmo commented Jun 12, 2020 •

edited