@tnakae Thanks for working on this! I'll try to review.

Could you please add a test that checks that it converge to the same solution with positive=True and positive=False when the ground truth data generative process is positive?

Something like:

>>> import numpy as np
>>> rng = np.random.RandomState(42)
>>> X = rng.randn(300, 100)
>>> true_coef = rng.uniform(0.1, 1, size=X.shape[1])
>>> y = X @ true_coef + 1 + rng.normal(X.shape[0]) / 10
>>> from sklearn.linear_model import Ridge
>>> Ridge().fit(X, y).coef_
array([0.8425452 , 0.88417954, 0.48863294, 0.43436882, 0.65071889,
       0.1547429 , 0.72064224, 0.80411708, 0.45595553, 0.25229772,
       ...
       0.29870553, 0.10603551, 0.69988191, 0.82548552, 0.33983739])

@ogrisel Added test based on your comment!

Thanks. I have another test idea: try to fit such a Ridge model both with positive=True and positive=False on a medium size (few hundreds samples and features) random dataset generated by make_regression and then check that the penalized objective is lower for positive=False but than any small random positive perturbation of the solution found by Ridge(positive=True) has a higher objective value than objective value of the solution without the random perturbation.

This last point is similar to the idea I suggested in #19615 to use the check_min utility to improve the ridge solver tests.

Out of curiosity could you also please benchmark the solver on a medium size dataset and compare to the speed of the solvers used for positive=False? Although they are not solving for the same objective, I would be curious about the performance profile and scalability of this solver. Maybe try with various synthetic datasets generated with make_regression with various n_samples / n_features chosen such that Ridge(positive=True) lasts between 5 and 10 seconds for instance.

It would also be interesting to see a screenshot of the output of a profiling run with https://pypi.org/project/viztracer/ just to make sure that there is no unexpected performance problem.

@lorentzenchr

One last thing is to test _solve_lbfgs directly against another solver for positive=False.

Added test function test_lbfgs_solver_consistency.

@tnakae Thanks again. This was a pleasant review experience on my side. Looking forward for the next one:smirk:

great work @tnakae !