Add tol to LinearRegression#30521
Conversation
ogrisel
left a comment
There was a problem hiding this comment.
Thanks for the PR.
Please add an entry such as:
LinearRegression: {"check_sample_weight_equivalence_on_dense_data": dict(tol=1e-12)},in the PER_ESTIMATOR_CHECK_PARAMS dict defined in sklearn/utils/_test_common/instance_generator.py and then remove the matching entry from the PER_ESTIMATOR_XFAIL_CHECKS dict in the same file. This should help check that we can use this to actually fix #30131.
You can run the common tests locally using:
pytest -vlxk "check_sample_weight_equivalence_on_sparse_data and LinearRegression" sklearn/tests/test_common.py
|
Actually, we also need to remove the xfail marker in the parametrization of This should involve setting a low enough value for |
|
Also, there are failing tests on the CI because the EDIT: more precisely: "tol": [Interval(Real, 0, None, closed="left")]as defined in the |
|
We will also need a dedicated changelog entry under |
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
|
The merge of #30535 into the You are welcome to use the "Resolve conflicts" button to use the online GitHub editor to do so, but don't forget to pull the result into your local branch before doing any subsequent commits locally. |
ogrisel
left a comment
There was a problem hiding this comment.
LGTM, thanks very much @SuccessMoses!
doc/whats_new/upcoming_changes/sklearn.linear_model/30521.fix.rst
Outdated
Show resolved
Hide resolved
OmarManzoor
left a comment
There was a problem hiding this comment.
LGTM. Thanks @SuccessMoses
|
This also closes #24601. |
Reference Issues/PRs
What does this implement/fix? Explain your changes.
Fixes #30131. I added
toltoLinearRegression.__init__this value is passed to thescipy.sparse.linalg.lsqr.Any other comments?
No.