ENH - fit intercept in ProxNewton solver
#77
Conversation
|
here are the benchmark results and the link to repo to reproduce. I have one concern: I cannot explain why we don't have the same initial objective value with Blitz in the case of any thoughts? |
|
I'd test the output (intercept and coef_) outside of benchopt for these two solvers on the problematic data. Check that both get 0 (but I don't think they should, cf our discussion on alpha_max with intercept) |
The reason behind this is: Blitz performs an intercept update before entering the So nothing surprising in such behavior. |
| past_grads = np.zeros(len(ws)) | ||
| X_delta_w_ws = np.zeros(X.shape[0]) | ||
| w_ws = w_epoch[ws] | ||
| ws_intercept = np.append(ws, -1) if fit_intercept else ws |
There was a problem hiding this comment.
This line of code if duplicated 4 times in the file.
It feels like it should be factorized, but I do not have a simple solution to do it right now.
Co-authored-by: PAB <pierreantoine.bannier@gmail.com>
| @@ -24,6 +24,9 @@ class ProxNewton(BaseSolver): | |||
| tol : float, default 1e-4 | |||
| Tolerance for convergence. | |||
|
|
|||
| fit_intercept : bool, default True | |||
There was a problem hiding this comment.
My experience with sklearn is that it's quite confusing to fit an intercept by default. Should it be set to False here ?
There was a problem hiding this comment.
Yes agree! But what about #77 (comment)?
There was a problem hiding this comment.
Nitpick (this can be discussed later IMO):
This means that it would be set to False by default in ProxNewton, and set to True by default in SparseLogisticRegression. This might lead to errors in the future
| @@ -24,6 +24,9 @@ class ProxNewton(BaseSolver): | |||
| tol : float, default 1e-4 | |||
| Tolerance for convergence. | |||
|
|
|||
| fit_intercept : bool, default True | |||
There was a problem hiding this comment.
Nitpick (this can be discussed later IMO):
This means that it would be set to False by default in ProxNewton, and set to True by default in SparseLogisticRegression. This might lead to errors in the future
|
Thanks @Badr-MOUFAD ! |
This adds unpenalized intercept support to
ProxNewtonsolver and closes #68.It proceeds as follows
View benchmark results
Link to benchmark repository