Skip to content

CI Fixes test_rank_deficient_design #21345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Oct 18, 2021
Merged

CI Fixes test_rank_deficient_design #21345

merged 3 commits into from
Oct 18, 2021

Conversation

thomasjpfan
Copy link
Member

@thomasjpfan thomasjpfan commented Oct 15, 2021 •
edited
Loading

Reference Issues/PRs

Closes #21340

What does this implement/fix? Explain your changes.

This issue came down to:

scikit-learn/sklearn/linear_model/_least_angle.py

Line 722 in 47a49c5

corr_eq_dir = np.dot(X.T[n_active:], eq_dir)

where the np.dot had a slightly different result between openblas 0.3.18 and 0.3.17:

import numpy as np

X_T_active = np.array([
    [6.666666666666666, -3.3333333333333335, -3.3333333333333335],
    [-0.3333333333333333, -0.3333333333333333, 0.6666666666666667]
], dtype=np.float64)

eq_dir = np.array([0.816496580927726, -0.4082482904638631, -0.4082482904638631], dtype=np.float64)

[i for i in np.dot(X_T_active, eq_dir)]

On openblas=0.3.18, the above result was: [8.16496580927726, -0.4082482904638631]
On openblas=0.3.17, the above result was: [8.164965809277259, -0.4082482904638631]

This slight difference worked its way down to generating a slightly different Cov, which snowballed from there.

@thomasjpfan thomasjpfan changed the title TST Fixes test_rank_deficient_design CI Fixes test_rank_deficient_design Oct 15, 2021
@adrinjalali
Copy link
Member

@ogrisel seems like we still have the scientific printing issue here. Could we please move to conda-forge?

thomasjpfan
thomasjpfan commented Oct 16, 2021
View reviewed changes
sklearn/linear_model/_least_angle.py
@@ -726,6 +727,9 @@ def _lars_path_solver(
# orthogonal (QR) decomposition of X
corr_eq_dir = np.dot(Gram[:n_active, n_active:].T, least_squares)

# Workaround for inconsistent rounding in openblas
np.around(corr_eq_dir, decimals=cov_precision, out=corr_eq_dir)
Copy link
Member Author

@thomasjpfan thomasjpfan Oct 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels more like symmetry breaking. Without this fix and on openblas 0.3.18, Cov gets updated to:

Cov = [2.3809523809523814, 2.3809523809523814]

When g1 is computed (with min_pos), it returns FLT_MAX because C - Cov is zero.

On openblas 0.3.17, Cov is:

Cov = [2.380952380952385, 2.3809523809523814]

which is not symmetric, leading to a non FLT_MAX value for g1.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this what @lesteve and @glemaitre discovered just before leaving for the WE ;)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it was the case. Then, the wrong dimension will be picked and everything goes sideways.

ogrisel
ogrisel approved these changes Oct 16, 2021
View reviewed changes
Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Maybe we could add a quick changelog entry for this fix.

sklearn/linear_model/_least_angle.py
@@ -726,6 +727,9 @@ def _lars_path_solver(
# orthogonal (QR) decomposition of X
corr_eq_dir = np.dot(Gram[:n_active, n_active:].T, least_squares)

# Workaround for inconsistent rounding in openblas
np.around(corr_eq_dir, decimals=cov_precision, out=corr_eq_dir)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this what @lesteve and @glemaitre discovered just before leaving for the WE ;)

@ogrisel
Copy link
Member

ogrisel commented Oct 16, 2021 •
edited
Loading

@ogrisel seems like we still have the scientific printing issue here. Could we please move to conda-forge?

Yes let's do it since it is happening again. I reopened #21324 and reported the problem upstream: ContinuumIO/anaconda-issues#12667

Edit: The switch to conda-forge is happening here: #21337.

@ogrisel ogrisel mentioned this pull request Oct 16, 2021
Merged
thomasjpfan and others added 2 commits October 16, 2021 13:06
@thomasjpfan @ogrisel
Update sklearn/linear_model/_least_angle.py
91f4b03 
Co-authored-by: Olivier Grisel <olivier.grisel@gmail.com>
@thomasjpfan
DOC Adds whats new
dee0742
@glemaitre glemaitre merged commit 5aecf20 into scikit-learn:main Oct 18, 2021
@glemaitre
Copy link
Member

LGTM Thanks @thomasjpfan

@glemaitre glemaitre mentioned this pull request Oct 23, 2021
Merged
10 tasks
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Oct 23, 2021
@thomasjpfan @ogrisel @glemaitre
CI Fixes test_rank_deficient_design (scikit-learn#21345)
7fb6882 
Co-authored-by: Olivier Grisel <olivier.grisel@gmail.com>
glemaitre pushed a commit that referenced this pull request Oct 25, 2021
@thomasjpfan @ogrisel @glemaitre
CI Fixes test_rank_deficient_design (#21345)
810098b 
Co-authored-by: Olivier Grisel <olivier.grisel@gmail.com>
samronsin pushed a commit to samronsin/scikit-learn that referenced this pull request Nov 30, 2021
@thomasjpfan @ogrisel @samronsin
CI Fixes test_rank_deficient_design (scikit-learn#21345)
e9f8efd 
Co-authored-by: Olivier Grisel <olivier.grisel@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glemaitre glemaitre glemaitre left review comments

@ogrisel ogrisel ogrisel approved these changes

Assignees
No one assigned
Labels
Build / CI module:linear_model
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CI failure in sklearn/linear_model/tests/test_least_angle.py:test_rank_deficient_design
4 participants
@thomasjpfan @adrinjalali @ogrisel @glemaitre