Implement L1 solver for GLM #9430
Conversation
|
don't blacken the module. I will not merge it. A pull request should only have the relevant changes, with at most a few small changes in the code that surrounds the main change. |
|
Any chance you could unblack the fine? Really hard to find genuine changes. |
|
We should black the entire repo, but that is a bigger discussion. |
|
|
||
| -loglike/n + alpha*(1-L1_wt)*|params|_2^2/2 | ||
| loglike_kwds = {} if loglike_kwds is None else loglike_kwds |
Check notice
Code scanning / CodeQL
Unused local variable
|
|
||
| -loglike/n + alpha*(1-L1_wt)*|params|_2^2/2 | ||
| loglike_kwds = {} if loglike_kwds is None else loglike_kwds | ||
| score_kwds = {} if score_kwds is None else score_kwds |
Check notice
Code scanning / CodeQL
Unused local variable
| -loglike/n + alpha*(1-L1_wt)*|params|_2^2/2 | ||
| loglike_kwds = {} if loglike_kwds is None else loglike_kwds | ||
| score_kwds = {} if score_kwds is None else score_kwds | ||
| hess_kwds = {} if hess_kwds is None else hess_kwds |
Check notice
Code scanning / CodeQL
Unused local variable
| def fit_regularized( | ||
| self, | ||
| method="elastic_net", | ||
| alpha=0.0, | ||
| start_params=None, | ||
| refit=False, | ||
| opt_method="bfgs", | ||
| **kwargs, | ||
| ): |
Check notice
Code scanning / CodeQL
Mismatch between signature and use of an overridden method
| ) | ||
|
|
||
| if cov_type == "nonrobust": | ||
| self.cov_type = "nonrobust" |
Check warning
Code scanning / CodeQL
Overwriting attribute in super-class or sub-class
| self.cov_kwds = { | ||
| "description": "Standard Errors assume that the" | ||
| + " covariance matrix of the errors is correctly " | ||
| + "specified." | ||
| } |
Check warning
Code scanning / CodeQL
Overwriting attribute in super-class or sub-class
| def score_test( | ||
| self, | ||
| exog_extra=None, | ||
| params_constrained=None, | ||
| hypothesis="joint", | ||
| cov_type=None, | ||
| cov_kwds=None, | ||
| k_constraints=None, | ||
| observed=True, | ||
| ): |
Check notice
Code scanning / CodeQL
Returning tuples with varying lengths
| data = longley.load() | ||
| # data.exog = add_constant(data.exog) | ||
| GLMmod = GLM(data.endog, data.exog).fit() | ||
| GLMT = GLMmod.summary(returns='tables') | ||
| GLMT = GLMmod.summary(returns="tables") |
Check notice
Code scanning / CodeQL
Unused global variable
| # GLMT[0].extend_right(GLMT[1]) | ||
| # print(GLMT[0]) | ||
| # print(GLMT[2]) | ||
| GLMTp = GLMmod.summary(title='Test GLM') | ||
| GLMTp = GLMmod.summary(title="Test GLM") |
Check notice
Code scanning / CodeQL
Unused global variable
|
Hello @louisabraham! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
|
Just did it and repushed/ |
|
Hello! did you have time to look at this? |
| @@ -1380,8 +1380,8 @@ def fit_regularized(self, method="elastic_net", alpha=0., | |||
|
|
|||
| Parameters | |||
| ---------- | |||
| method : {'elastic_net'} | |||
| Only the `elastic_net` approach is currently implemented. | |||
| method : {'elastic_net', 'l1'} | |||
There was a problem hiding this comment.
better to keep the full name for l1_slsqp here
Needs comment that is an interior point method (in contrast to coordinate descent of elastic net)
|
Overall this looks good. I have only looked at the overall structure and not yet at the details. It's a useful extension and can be merged after a proper review and addition of unit tests. It should be possible to add the unit test using the corresponding unit test of discrete models, logit, poisson, ... |
NumPy's guide.
Can you tell me how and where to write a test? I could try to match the results against the code in #9428