Skip to content

Refactor linear model coordinate descent code to work as LARS for cv #2186

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
Closed
wants to merge 3 commits into from
Closed

Refactor linear model coordinate descent code to work as LARS for cv #2186

wants to merge 3 commits into from

Conversation

agramfort
Copy link
Member

there is a million dollar bug cf. plot_model_lasso_model_selection.py

tests pass but the example output is wrong...

@agramfort agramfort mentioned this pull request Jul 22, 2013
Closed
@jaquesgrobler
Copy link
Member

i'll look at this when i'm home a bit too.

2013/7/22, Alexandre Gramfort notifications@github.com:

there is a million dollar bug cf. plot_model_lasso_model_selection.py

tests pass but the example output is wrong...
You can merge this Pull Request by running:

git pull https://github.com/agramfort/scikit-learn refactor_cd_cv

Or you can view, comment on it, or merge it online at:

#2186

-- Commit Summary --

  • make new classes for lasso_path/enet_path and deprecate old
  • lasso_path output changed/depr warnings temporarily removed
  • lasso_path output changed/depr warnings temporarily removed
  • old_return added and tests adjusted
  • slight cleanup in tests
  • squashed:example cleanup, warning change, Alex`s suggestions
  • fix broken test
  • cosmetic: same line parameters
  • remove ugly divider from example
  • clarify test description and remove duplicate warning
  • update what`s new
  • fix merge artifact - log10s readded
  • fix merge artifact
  • comments and pep8 changes
  • remove unneccasary if
  • misc
  • API : deprecating fit_intercept and normalize in lasso_path and
    enet_path
  • ENH : more cleanup on linear models path refactoring
  • API : massive refactoring of CV models in coordinate descent. Now the
    algo core is in path functions
  • attempt to fix plot_lasso_model_selection

-- File Changes --

M doc/whats_new.rst (6)
M examples/linear_model/plot_lasso_coordinate_descent_path.py (47)
M examples/linear_model/plot_lasso_model_selection.py (2)
M sklearn/linear_model/base.py (7)
M sklearn/linear_model/coordinate_descent.py (1261)
M sklearn/linear_model/tests/test_coordinate_descent.py (42)
M sklearn/linear_model/tests/test_least_angle.py (16)
M sklearn/linear_model/tests/test_sparse_coordinate_descent.py (5)

-- Patch Links --

https://github.com/scikit-learn/scikit-learn/pull/2186.patch
https://github.com/scikit-learn/scikit-learn/pull/2186.diff

@agramfort
Copy link
Member Author

@fabianp spotted a bug with precompute.

@agramfort agramfort mentioned this pull request Jul 22, 2013
Closed
2 tasks
vene
vene reviewed Jul 23, 2013
View reviewed changes
sklearn/linear_model/coordinate_descent.py
than ``tol``.
normalize : boolean, optional, default False
If ``True``, the regressors X will be normalized before regression.
WARNING : will be deprecated in 0.15
Copy link
Member

Choose a reason for hiding this comment

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

Could you point me to where the decision to deprecate has been made? Can we also deprecate normalize in orthogonal matching pursuit? The code is on the point of unmaintainability because of branching in case of normalization and copy_X, copy_Gram, copy_Xy interaction.

Copy link
Member Author

Choose a reason for hiding this comment

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

Could you point me to where the decision to deprecate has been made?

search for :

  • if normalize is not None:
  •    warnings.warn("normalize param will be removed in 0.15."

  •                  " Intercept fitting and feature normalization will be"

  •                  " done in estimators.",

  •                  DeprecationWarning, stacklevel=2)

Can
we also deprecate normalize in orthogonal matching pursuit? The code is on
the point of unmaintainability because of branching in case of normalization
and copy_X, copy_Gram, copy_Xy interaction.

Yes I think it's the most reasonable option. Indeed all these options make
it a nightmare to maintain.

@agramfort
Copy link
Member Author

cleanup, rebased, ready for review !

warning the diff is horrible as many functions have moved. It's a big refactoring.

@agramfort
Copy link
Member Author

travis is happy !

@fabianp
Copy link
Member

fabianp commented Jul 23, 2013

Merged by rebase

@fabianp fabianp closed this Jul 23, 2013
@arjoly arjoly mentioned this pull request Jul 26, 2013
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@agramfort @jaquesgrobler @fabianp @vene