-
Notifications
You must be signed in to change notification settings - Fork 182
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.
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
Logistic Regression CI fix #2013
Logistic Regression CI fix #2013
Conversation
/intelci: run |
@@ -129,5 +129,5 @@ def test_csr(queue, dtype, dims): | |||
|
|||
assert_allclose(pred, pred_sp) | |||
assert_allclose(prob, prob_sp) | |||
assert_allclose(model.coef_, model_sp.coef_) | |||
assert_allclose(model.intercept_, model_sp.intercept_) | |||
assert_allclose(model.coef_, model_sp.coef_, atol=1e-6) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can increase tolerance up to 1e-5 to avoid sporadic failures
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, or even 1e-4. I see in the nightly that max relative difference was 7.3e-5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe rtol is a better metric since its independent of data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
/intelci: run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1e-4 relative tolerance is a bit eye opening (or ~7e-5). I am going to approve it with the understanding that a follow up ticket for an investigation is made.
@Mergifyio copy rls/2024.7.0-rls |
✅ Pull request copies have been created
|
* increase tolerance * increase tolerance * increase tolerance (cherry picked from commit d7a2f92)
* increase tolerance * increase tolerance * increase tolerance
Description
test_csr fails for logistic regression in Nightly, increasing tolerance.
Checklist to comply with before moving PR from draft:
PR completeness and readability
Testing
Performance