Skip to content
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

ENH Fix averaged RMSE #17309

Merged
merged 5 commits into from
May 24, 2020
Merged

Conversation

swierh
Copy link
Contributor

@swierh swierh commented May 22, 2020

Reference Issues/PRs

Fixes #16960

What does this implement/fix? Explain your changes.

Fixed the mean_squared_error metric for the RMSE values that are averaged over multiple outputs. Previously, the behaviour was sqrt(average(MSE)). This has been changed to average(sqrt(MSE)) to reflect the documented functionality.

Documented functionality for the multioutput argument is "Defines aggregating of multiple output values."

An example of the usage and output has been included in the documentation, and two tests that expected erroneous output have been changed.

swierh added 4 commits May 22, 2020 17:27
implement code as suggested by @Paul-Aime in scikit-learn#16960 and add a code example to the documentation.
The average of the root of the MSE values is ~0.454. The previous value was equal to the root of the average of MSE values.

Note that 0.645 is equal to the root of the expected output (`(1. / 3 + 2. / 3 + 2. / 3) / 4.`) for the MSE test 3 lines above.
The correct value is 0.59.

Note that the previous value of 0.62 is equal to the root of the expected output (0.39) in the MSE test.
@swierh swierh changed the title Fix averaged rmse Fix averaged RMSE May 22, 2020
@swierh swierh marked this pull request as ready for review May 22, 2020 16:40
@swierh swierh changed the title Fix averaged RMSE [MRG] Fix averaged RMSE May 22, 2020
Copy link
Member

@jnothman jnothman left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM

doc/whats_new/v0.24.rst Outdated Show resolved Hide resolved
Co-authored-by: Joel Nothman <joel.nothman@gmail.com>
Copy link
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

LGTM

@thomasjpfan thomasjpfan changed the title [MRG] Fix averaged RMSE ENH Fix averaged RMSE May 24, 2020
@thomasjpfan thomasjpfan merged commit 9368545 into scikit-learn:master May 24, 2020
@thomasjpfan
Copy link
Member

Thank you @swierh !

viclafargue pushed a commit to viclafargue/scikit-learn that referenced this pull request Jun 26, 2020
@ogrisel ogrisel added this to the 0.23.2 milestone Aug 3, 2020
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Aug 3, 2020
jayzed82 pushed a commit to jayzed82/scikit-learn that referenced this pull request Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mean_square_error multioutput signification
4 participants