Skip to content

[MRG + 1] FIX for LassoLarsCV on with readonly folds#4684

Merged
amueller merged 1 commit intoscikit-learn:masterfrom
ogrisel:readonly-lars-path
May 20, 2015
Merged

[MRG + 1] FIX for LassoLarsCV on with readonly folds#4684
amueller merged 1 commit intoscikit-learn:masterfrom
ogrisel:readonly-lars-path

Conversation

@ogrisel
Copy link
Copy Markdown
Member

@ogrisel ogrisel commented May 7, 2015

FIX #4597

@ogrisel ogrisel added the Bug label May 7, 2015
@ogrisel ogrisel added this to the 0.16.2 milestone May 7, 2015
@amueller
Copy link
Copy Markdown
Member

amueller commented May 7, 2015

do all four arrays need to be writable?

@amueller
Copy link
Copy Markdown
Member

amueller commented May 7, 2015

The answer is yes. LGTM.

@amueller amueller changed the title [MRG] FIX for LassoLarsCV on with readonly folds [MRG + 1] FIX for LassoLarsCV on with readonly folds May 7, 2015
amueller
amueller reviewed May 7, 2015
View reviewed changes
sklearn/linear_model/tests/test_least_angle.py
Copy link
Copy Markdown
Member

@amueller amueller May 7, 2015

Choose a reason for hiding this comment

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

you could probably do a more light-weight test by using stringIO and np.memmap? I don't like IO in tests.... but seems good enough.

Copy link
Copy Markdown
Member Author

@ogrisel ogrisel May 20, 2015

Choose a reason for hiding this comment

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

mmap is a system call, you cannot do it on a Python StringIO instance.

@amueller
Copy link
Copy Markdown
Member

amueller commented May 7, 2015

It might be interesting to add a flag to joblib to force it to memmap in parallel for debugging and testing purposes. Maybe even to memmap with n_jobs=1?

@ogrisel
Copy link
Copy Markdown
Member Author

ogrisel commented May 20, 2015

It might be interesting to add a flag to joblib to force it to memmap in parallel for debugging and testing purposes. Maybe even to memmap with n_jobs=1?

Automatic memmaping is done internally by the MemmapingPool. The whole multiprocessing backend is skipped when n_jobs=1. I don't think we want to change that.

@ogrisel
Copy link
Copy Markdown
Member Author

ogrisel commented May 20, 2015

@agramfort I would appreciate a second review on this. I met today a real user that was impacted by this issue.

@agramfort
Copy link
Copy Markdown
Member

agramfort commented May 20, 2015

LGTM

does it fix the pb of this user?

@amueller
Copy link
Copy Markdown
Member

amueller commented May 20, 2015

it should.

amueller added a commit that referenced this pull request May 20, 2015
@amueller
Merge pull request #4684 from ogrisel/readonly-lars-path
6873c9f 
[MRG + 1] FIX for LassoLarsCV on with readonly folds
@amueller amueller merged commit 6873c9f into scikit-learn:master May 20, 2015
@ogrisel ogrisel mentioned this pull request Jun 24, 2015
Merged
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

Bug

Projects

None yet

Milestone

0.16.2

Development

Successfully merging this pull request may close these issues.

LassoLarsCV error if n_jobs > 1

3 participants

@ogrisel @amueller @agramfort