Skip to content

Remove sigmoid in BinaryPrecisionRecallCurve #3182

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

PussyCat0700
Copy link

@PussyCat0700 PussyCat0700 commented Jul 15, 2025

What does this PR do?

This PR addresses #3179 by making the use of sigmoid normalization optional in BinaryPrecisionRecallCurve. Specifically, it introduces a normalization parameter to _binary_precision_recall_curve_format and disables automatic sigmoid application in BinaryPrecisionRecallCurve.

Changes

  • Adds a normalization: Optional[str] = "sigmoid" argument to _binary_precision_recall_curve_format, allowing users to override the default behavior.

  • Passes normalization=None in BinaryPrecisionRecallCurve to preserve raw logits and prevent batch-wise distortion during .update() calls.

Notes

  • This fix only applies to BinaryPrecisionRecallCurve, which is the class I directly encountered issues with.

  • I’ve noticed that other metric classes may also apply sigmoid implicitly in ways that could cause similar inconsistencies across batch-wise updates, but since I have not tested those cases, I am leaving them unchanged to avoid unintended side effects.


📚 Documentation preview 📚: https://torchmetrics--3182.org.readthedocs.build/en/3182/

@PussyCat0700
Copy link
Author

python -m pytest -v --color=no tests/unittests/classification/test_roc.py::TestBinaryROC::test_binary_roc>
Seems that a few unit tests weren't passed because the test cases have only considered the results after sigmoid to be correct. So it is impossible to pass the unittests currently, I suppose?

Copy link
Member

@Borda Borda left a comment

Choose a reason for hiding this comment

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

pls add entry to changelog

@PussyCat0700
Copy link
Author

pls add entry to changelog

Done

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.

2 participants