8000 [MRG] Bayesian regression model dtype consistency by Henley13 · Pull Request #9087 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

[MRG] Bayesian regression model dtype consistency #9087

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
Closed
wants to merge 4 commits into from
Closed

[MRG] Bayesian regression model dtype consistency #9087

wants to merge 4 commits into from

Conversation

Henley13
Copy link
Contributor
@Henley13 Henley13 commented Jun 9, 2017 •
edited by amueller
Loading

Reference Issue

Works on #11000

What does this implement/fix? Explain your changes.

Avoids Bayesian Regression to aggressively cast the data to np.float64 when np.float32 is supplied.

Any other comments?

@Henley13 Henley13 changed the title Bayesian regression model dtype consistency [WIP] Bayesian regression model dtype consistency Jun 9, 2017
@Henley13 Henley13 mentioned this pull request Jun 9, 2017
Merged
@GaelVaroquaux
Copy link
Member

The tests are failing.

@Henley13 Henley13 changed the title [WIP] Bayesian regression model dtype consistency [MRG] Bayesian regression model dtype consistency Jun 23, 2017
@Henley13
Copy link
Contributor Author

@GaelVaroquaux Ping

raghavrv
raghavrv approved these changes Jun 28, 2017
View reviewed changes
Copy link
Member
@raghavrv raghavrv left a comment

Choose a reason for hiding this comment

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

Thanks! This looks good to me, pending minor comments. :)

sklearn/linear_model/tests/test_bayes.py
assert X.dtype == dtype
assert model.coef_.dtype == X.dtype
assert y_mean.dtype == X.dtype
assert y_std.dtype == X.dtype
Copy link
Member

Choose a reason for hiding this comment

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

could you also add a line testing dtype of predict?

sklearn/linear_model/tests/test_bayes.py
ard_64.fit(X_64, y_64)

assert_array_almost_equal(br_32.coef_, br_64.coef_, decimal=5)
assert_array_almost_equal(ard_32.coef_, ard_64.coef_, decimal=5)
Copy link
Member

Choose a reason for hiding this comment

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

same here too. could you test it for the output of predict too?

massich
massich approved these changes Jun 28, 2017
View reviewed changes
Copy link
Contributor
@massich massich left a comment

Choose a reason for hiding this comment

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

LGTM

@amueller
Copy link
Member

Can you please address the comments and fix the merge conflicts? Thanks!

@massich massich mentioned this pull request Jun 1, 2018
Closed
Base automatically changed from master to main January 22, 2021 10:49
@Micky774 Micky774 mentioned this pull request Feb 17, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@massich massich massich approved these changes

@raghavrv raghavrv raghavrv approved these changes

Assignees
No one assigned
Labels
help wanted module:linear_model Stalled
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Henley13 @GaelVaroquaux @amueller @massich @raghavrv @cmarmo
0