WIP sparse matrix support in randomized logistic regression #1133

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
@amueller
Member

amueller commented Sep 9, 2012

His adds sparse matrix support to randomized logistic regression.
Needs a bit more testing and I should probably also do the lasso for completeness.

@mblondel

This comment has been minimized.

Show comment
Hide comment
@mblondel

mblondel Sep 13, 2012

Member

Looks like an easy merge for the people sprinting in Paris :) CC @agramfort @GaelVaroquaux

Member

mblondel commented Sep 13, 2012

Looks like an easy merge for the people sprinting in Paris :) CC @agramfort @GaelVaroquaux

@amueller

This comment has been minimized.

Show comment
Hide comment
@amueller

amueller Sep 13, 2012

Member

I can have a look tonight if I have commits in my working version which should go in here, but I think they are already in the right branch.

Member

amueller commented Sep 13, 2012

I can have a look tonight if I have commits in my working version which should go in here, but I think they are already in the right branch.

@amueller

This comment has been minimized.

Show comment
Hide comment
@amueller

amueller Sep 13, 2012

Member

If any one wants to work on this, they need to cherry pick this commit: 12a0f0e from my branch "working".

Member

amueller commented Sep 13, 2012

If any one wants to work on this, they need to cherry pick this commit: 12a0f0e from my branch "working".

@amueller

This comment has been minimized.

Show comment
Hide comment
@amueller

amueller Sep 20, 2012

Member

So I did the cherry picking. Unfortunately it doesn't work for the lasso yet, as the data is centered there. I'm not sure it makes sense to skip that for sparse data as no intercept is fit.

Member

amueller commented Sep 20, 2012

So I did the cherry picking. Unfortunately it doesn't work for the lasso yet, as the data is centered there. I'm not sure it makes sense to skip that for sparse data as no intercept is fit.

@larsmans

This comment has been minimized.

Show comment
Hide comment
@larsmans

larsmans Oct 11, 2012

This is not being used.

@larsmans

This comment has been minimized.

Show comment
Hide comment
@larsmans

larsmans Oct 11, 2012

I don't understand this comment. Shouldn't the randomized LR estimator do centering?

I don't understand this comment. Shouldn't the randomized LR estimator do centering?

This comment has been minimized.

Show comment
Hide comment
@amueller

amueller Oct 11, 2012

Owner

will clarify. the LR does not do centering on sparse data (which is sensible), so I do this here to make sparse and dense results comparable.

Owner

amueller replied Oct 11, 2012

will clarify. the LR does not do centering on sparse data (which is sensible), so I do this here to make sparse and dense results comparable.

@larsmans

This comment has been minimized.

Show comment
Hide comment
@larsmans

larsmans Oct 16, 2012

Member

Dropped my issue with the comment and merged by rebase.

Member

larsmans commented Oct 16, 2012

Dropped my issue with the comment and merged by rebase.

@larsmans larsmans closed this Oct 16, 2012

@amueller

This comment has been minimized.

Show comment
Hide comment
@amueller

amueller Oct 16, 2012

Member

Thanks.

Member

amueller commented Oct 16, 2012

Thanks.

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