Skip to content

TFocalMetric negative values fix #2386

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

Conversation

diditforlulz273
Copy link

@diditforlulz273 diditforlulz273 commented May 14, 2023

Before submitting a pull request, please do the following steps:

  1. Read instructions for contributors.
  2. Run ya make in catboost folder to make sure the code builds.
  3. Add tests that test your change.
  4. Run tests using ya make -t -A command.
  5. If you haven't already, complete the CLA.

1 - +
2 - cmake via python setup.py bdist_wheel --no-widget builds it successfully.
3 - tests were not written by me, this is a fix for what's covered already - focal loss that I also wrote
4 - +
5 - I Did it already on the initial focal loss commit, but anyways I agree to it here.

The actual case was that metric in some cases becomes negative, which is not possible for focal loss itself by design.
The mistake is that I copied loss code part to metric code part - which means it calculated as the metric the sum of the first and second derivatives of the loss, not the loss itself.

Surely sum of derivatives could be negative, and while this does not affect the actual model(the loss is correct), it confused users by showing although diminishing, loss value going negative.

@diditforlulz273 diditforlulz273 changed the title fix TFocalMetric negative values fix May 14, 2023
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.

1 participant