Skip to content

Implement RMSE (root-mean-square error) metric and scorer #13467

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

Merged
merged 13 commits into from
Aug 7, 2019

Conversation

urvang96
Copy link
Contributor

Reference Issues/PRs
Fixes #12895
Implement RMSE (root-mean-square error) metric and scorer

What does this implement/fix? Explain your changes.
Added a boolean parameter in MSE implementation to return RMSE value, if set to true. Also added an RMSE scorer.

Any other comments?
One test.

@urvang96
Copy link
Contributor Author

As per my understanding, I have added the scorer. Let me know if I missed something.
Thank you in advance.

@qinhanmin2014
Copy link
Member

there're still some pep8 errors, please refer to https://circleci.com/gh/scikit-learn/scikit-learn/52068

@urvang96 urvang96 changed the title [WIP]Implement RMSE (root-mean-square error) metric and scorer Implement RMSE (root-mean-square error) metric and scorer Mar 22, 2019
@qinhanmin2014
Copy link
Member

qinhanmin2014 commented Mar 30, 2019

@qinhanmin2014
Copy link
Member

I think we need more tests, e.g., test_regression_multioutput_array and test_regression_metrics_at_limits.

@xinyuliu12
Copy link
Contributor

May I try continuing work on this?

@urvang96
Copy link
Contributor Author

urvang96 commented Apr 20, 2019

@xinyuliu12 Actually I am waiting for a review on this. Also, I am not sure about the doc failures in the commit so if you want you can help with that.

@qinhanmin2014 Could you please review and let me know if any changes required.

@thomasjpfan
Copy link
Member

@urvang96 Merge with master should fix the doc tests

Copy link
Member

@qinhanmin2014 qinhanmin2014 left a comment

Choose a reason for hiding this comment

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

Please add an entry to the change log at doc/whats_new/v*.rst. Like the other entries there, please reference this pull request with :pr: and credit yourself (and other contributors if applicable) with :user:.

@urvang96
Copy link
Contributor Author

urvang96 commented Aug 5, 2019

@qinhanmin2014 Added the changes in whats_new. Thank you.

@@ -253,7 +260,11 @@ def mean_squared_error(y_true, y_pred,
# pass None as weights to np.average: uniform mean
multioutput = None

return np.average(output_errors, weights=multioutput)
mse = np.average(output_errors, weights=multioutput)
if not squared:
Copy link
Member

Choose a reason for hiding this comment

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

Nit:

return mse if squared else np.sqrt(mse)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

@rth rth left a comment

Choose a reason for hiding this comment

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

LGTM as well, thanks @urvang96 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement RMSE (root-mean-square error) metric and scorer
6 participants