[go: nahoru, domu]

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

RMSLE (root mean squared log error) #20326

Merged
merged 22 commits into from
Jun 25, 2021
Merged

Conversation

helper-uttam
Copy link
Contributor
@helper-uttam helper-uttam commented Jun 22, 2021

Reference Issues/PRs

Fixes #20318

What does this implement/fix? Explain your changes.

Updating the function mean_square_log_error and includeing one more argument in it i.e., squared= which will be True by default, because in many contests or workplace there is a need of finding root_mean_square_log_error(RMSLE). And scikit-learn initially do not have such inbuilt function but it has 'mean_square_log_error' and we can get root_mean_square_log_error' by taking the square root of mean_square_log_error'.

Any other comments?

Copy link
Member
@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

I think we should also register neg_root_mean_squared_log_error in sklearn.metrics.SCORERS to make it possible to use it with the model selection tools, for instance by passing scoring="neg_root_mean_squared_log_error" to cross_validate.

sklearn/metrics/_regression.py Outdated Show resolved Hide resolved
helper-uttam and others added 3 commits June 23, 2021 01:34
fixing example
adding more examples
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
sklearn/metrics/_regression.py Outdated Show resolved Hide resolved
fixing Indentation error
fixing indentation and removing extra examples
helper-uttam and others added 2 commits June 23, 2021 15:29
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
@helper-uttam
Copy link
Contributor Author

I had approved all the proposed changes still it's not working :(

how to know that what's going wrong with ci/circle , linting ?

@ogrisel
Copy link
Member
ogrisel commented Jun 23, 2021

The message says:

sklearn/metrics/_regression.py:473:68: W291 trailing whitespace

helper-uttam and others added 2 commits June 23, 2021 22:30
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
@helper-uttam
Copy link
Contributor Author

The message says:

sklearn/metrics/_regression.py:473:68: W291 trailing whitespace

Okay If it will fail this time then I will write the code from the start,,,let's see :)

@ogrisel
Copy link
Member
ogrisel commented Jun 23, 2021

Okay If it will fail this time then I will write the code from the start,,,let's see :)

No need, it's a better strategy to read the error message to understand what's happening.

@ogrisel
Copy link
Member
ogrisel commented Jun 23, 2021

For the next step, could you please address: #20326 (review) ?

doc/whats_new/v1.0.rst Outdated Show resolved Hide resolved
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
@helper-uttam
Copy link
Contributor Author

I think we should also register neg_root_mean_squared_log_error in sklearn.metrics.SCORERS to make it possible to use it with the model selection tools, for instance by passing scoring="neg_root_mean_squared_log_error" to cross_validate.

Sure! but I need some explanation of this, to understand more about it and the implementation part (How to implement).

updating contributors docs
@helper-uttam
Copy link
Contributor Author

Hey @ogrisel I got your point and I think we should definitely do that to cross_validate !

I think we should also register neg_root_mean_squared_log_error in sklearn.metrics.SCORERS to make it possible to use it with the model selection tools, for instance by passing scoring="neg_root_mean_squared_log_error" to cross_validate.

Sure! but I need some explanation of this, to understand more about it and the implementation part (How to implement).

Hey @ogrisel , I got your point and I think we should definitely do this to 'cross_validate`

doc/whats_new/v1.0.rst Outdated Show resolved Hide resolved
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
.......................

- |Feature| :func:`~sklearn.metrics.mean_squared_log_error` now supports
`squared='False'`. :pr:`20326` by :user:`Uttam kumar <helper-uttam>`_.
Copy link
Member

Choose a reason for hiding this comment

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

The doc CI is failing because of those two warnings:

doc/whats_new/v1.0.rst:587: WARNING: Field list ends without a blank line; unexpected unindent.
doc/whats_new/v1.0.rst:589: WARNING: Mismatch: both interpreted text role prefix and reference suffix. 

The error messages are weird but I suspect this might be caused because there is already an section named sklearn.metrics upper in the same file. The module sections are ordered by alphabetical order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ops! I thought it was occurring due to some whitespaces or extra lines. :)

Copy link
Member

Choose a reason for hiding this comment

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

Please move this entry to the sklearn.metrics on line 410 in the same file to avoid redefining the sklearn.metrics section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, I didn't noticed this previously!

I had moved to 410.

doc/whats_new/v1.0.rst Outdated Show resolved Hide resolved
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
@helper-uttam
Copy link
Contributor Author

Again same warning!

@helper-uttam
Copy link
Contributor Author

@ogrisel @rth
Hey, I am very thankful to you both!
Finally we did it :)

Contributors like you are always inspiring to new contributors (like me) 💯 🥇

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.

Thanks @helper-uttam !

Copy link
Member
@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for your contribution @helper-uttam. Once this this merged, please update your main branch and feel free to open a new PR to add scorer I mentioned previously in the discussion.

@ogrisel ogrisel merged commit 509b9ff into scikit-learn:main Jun 25, 2021
sakibguy added a commit to sakibguy/scikit-learn that referenced this pull request Jun 25, 2021
RMSLE (root mean squared log error) (scikit-learn#20326)
samronsin pushed a commit to samronsin/scikit-learn that referenced this pull request Nov 30, 2021

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
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.

Root Mean Square Log Error
3 participants