Skip to content

Update xgboost_regressor.py #9058

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

Closed
wants to merge 2 commits into from

Conversation

rohan472000
Copy link
Contributor

@rohan472000 rohan472000 commented Sep 13, 2023

Describe your change:

The mean_squared_error() function has been updated in the latest version of scikit-learn. The previous version of the function used a different formula for calculating the mean squared error, which resulted in a different value. This PR updates the code (changed the doctest results) to use the new formula.

The new version of the function uses the following formula:

      mean_squared_error(y_true, y_predicted) = np.average((y_true - y_predicted)**2)

The previous version of the function used the following formula:

     mean_squared_error(y_true, y_predicted) = np.sum((y_true - y_predicted)**2) / len(y_true)
  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms include at least one URL that points to Wikipedia or another similar explanation.
  • If this pull request resolves one or more open issues then the description above includes the issue number(s) with a closing keyword: "Fixes #ISSUE-NUMBER".

@algorithms-keeper algorithms-keeper bot added the tests are failing Do not merge until tests pass label Sep 13, 2023
@algorithms-keeper algorithms-keeper bot added awaiting reviews This PR is ready to be reviewed and removed tests are failing Do not merge until tests pass labels Sep 13, 2023
@rohan472000
Copy link
Contributor Author

@cclauss kindly review this PR.

@rohan472000 rohan472000 mentioned this pull request Sep 14, 2023
2 tasks
@tianyizheng02
Copy link
Contributor

@rohan472000 Can you provide a source that documents this formula change?

@tianyizheng02
Copy link
Contributor

https://github.com/scikit-learn/scikit-learn/blob/7f9bad99d/sklearn/metrics/_regression.py#L404

This is just a link to the mean_squared_error in scikit-learn's source code. How does this show that the formula has changed? If you check the blame for the file, you'll see that the code for the mean_squared_error function hasn't changed in years:

screenshot

https://scikit-learn.org/stable/whats_new/v1.0.html

This is a changelog for scikit-learn 1.0, but the newest stable version is 1.3.0. You said the formula changed in the latest version, so why not link to the changelog for version 1.3.0? Either way, I wasn't able to find any info on either changelog stating that the formula for mean_squared_error has changed.

@rohan472000 I don't doubt that your PR fixes the broken doctests, but you haven't explained why the outputs have changed. Do you have any sources to back up your claim that the formula actually changed? I want to make sure that your new doctest outputs are actually the correct expected outputs and not simply due to an error in the implementation.

@rohan472000
Copy link
Contributor Author

rohan472000 commented Sep 19, 2023

Yes ..you are correct as I read an article a month ago regarding metrics error especially mse and rmse, but it is my mistake i concluded that wrong..I also checked but no change in formula I found... closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting reviews This PR is ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants