-
Notifications
You must be signed in to change notification settings - Fork 214
[MRG] estimated step size for SAG* objects #42
Conversation
def get_max_squared_sum(X): | ||
n = X.shape[0] | ||
return np.max(X.dot(X.T)[range(n), range(n)]) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would use np.sum(X ** 2, axis=1).max()
in the dense case and csr_row_norms(X).max()
in the CSR case (from sklearn.utils.sparsefuncs).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that function is available in scikit-learn 0.14, it looks like it was added in 0.15
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used your formula for the dense case at least.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice! Thanks! |
I've changed the backport of get_max_squared_sum to use csr_row_norms as suggested by @mblondel . I've also updated the README to reflect this (the import is anyway lazy so lightning should at least import in older versions of sklearn). |
And bump required version to 0.15
Merging, thank you very much! |
[MRG] estimated step size for SAG* objects
This estimates the step size from the data instead of relying on the user having to specify a step size. The result is that it converges for many cases for which the default step size will not converge, such as multiplying the 20news dataset by 3.
Some remarks: