✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

_{Generated for commit: 4c8d6e9. Link to the linter CI: here}

Just by curiosity, are NumPy and SciPy using different implementation of LAPACK? Is there a way to choose the right driver in SciPy that would provide the same behaviour than the one of NumPy or the issue is elsewhere?

Just by curiosity, are NumPy and SciPy using different implementation of LAPACK? Is there a way to choose the right driver in SciPy that would provide the same behaviour than the one of NumPy or the issue is elsewhere?

In scipy the test unfortunately fails for all lapack_driver options.

So looking a bit more, it seems that NumPy use the GELSD driver.

https://github.com/numpy/numpy/blob/v2.0.0/numpy/linalg/umath_linalg.cpp#L4037

Note that before merging such a change we would need to verify whether there is any speed impact on a few synthetic datasets with various shapes and sizes. But first we need to make all the tests pass consistently.

Some of the tests fail because of a future warning for the rcond parameter that will change its default value. Let's set it explicitly to the future default value to silence the warning and make the code behave consistently across NumPy (or SciPy) versions, e.g. rcond=max(X.shape) * np.finfo(X.dtype.resolution) or rcond=max(X.shape) * np.finfo(X.dtype.eps).

Note that scipy.linalg.lstq also exposes a cond parameter with a seemingly different meaning, but it's not clear what the default cond=None means and how it interacts with the choice of the lapack driver. Here are the links to the doc:

• https://numpy.org/doc/stable/reference/generated/numpy.linalg.lstsq.html
• https://scipy.github.io/devdocs/reference/generated/scipy.linalg.lstsq.html

So maybe scipy is not buggy, but it's just our way of calling it with the default cond=None that is a bug. See also my previous attempt at using cond=1e-15 (which is the same as cond=np.finfo(X.dtype).resolution with float64 input data): #26164 (comment)

Once settled on a choice of setting scipy cond or numpy rcond, please push a commit with the following commit message to run the tests with all admissible random seed values on each of the CI worker (different version of numpy/scipy, CPU architectures / operating systems)...:

Trigger CI for [all random seeds]
test_linear_regression_sample_weight_consistency

As explained in #28959.

EDIT: fixed a typo in the commit message used to trigger all random seeds tests.

Indeed @ogrisel the cut-off ratio for the singular values cond/rcond seems to be the real culprit !
On my machine, both numpy.linalg.lstsq and scipy.linalg.lstsq fail with cond=eps (old numpy default value).
However they both pass with cond=max(X.shape)*eps (new numpy default value) and whatever the lapack_driver used for scipy.

I guess we need the benchmark to decide between numpy and the various options for scipy.

The remaining failures are only for csr matrices (one seed and just below tolerance), I think I made the test a bit too stringent by increasing n_features.

Could you then please open a similar but concurrent draft PR that attempts to set the cond parameter for scipy.linalg.lstsq instead, and try to make the test pass for this with all random seeds?

I revert to the old test (with n_samples, n_features = 10, 5) as it was already able to detect the failure, especially if we test for all random seeds. But maybe we could refactor this test once #29818 is merged, for instance do a sparse version of
check_sample_weight_equivalence ?

@ogrisel was also speaking of a more extensive test suite (beyond just the sample weight) for LinearRegression and Ridge (sparse/dense, fit_intercept=True/False, wide/long dataset, very ill-conditioned or not, etc..)

The more extensive test suite should better be tackled by reviving the stalled #25948 PR.

@antoinebaker could you please add a changelog entry targeting scikit-learn 1.6?

Please also open an alternative PR that does the minimal fix to make the test pass with scipy.linalg.lstq instead of numpy.linalg.lstq.

I suspect that scipy.linalg.lstq(..., cond=np.finfo(X.dtype).resolution) might work, but I would rather check with an [all random seeds] CI run for that option as well.

Oh sorry @ogrisel I forgot to link to the concurrent PR #30040. All tests are passing with the scipy solver, with cond set to the numpy default or just the resolution.

Doing some naive benchmarks, it seems that scipy with lapack_driver='gelsy' seems to be a bit faster in agreement with the documentation, but it really depends on the data shape.

Closing in favor of #30040.