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

Conversion from logits to probabilities happens on a batch by batch basis #2195

Open
GabrielBianconi opened this issue Oct 31, 2023 · 5 comments · May be fixed by #1676
Open

Conversion from logits to probabilities happens on a batch by batch basis #2195

GabrielBianconi opened this issue Oct 31, 2023 · 5 comments · May be fixed by #1676
Labels
bug / fix Something isn't working good first issue Good for newcomers help wanted Extra attention is needed v1.0.x

Comments

@GabrielBianconi
Copy link

🐛 Bug

I'm using BinaryAUROC and calling .update(logits, y) after each batch (validation_step), expecting the following behavior:

"If preds has values outside [0,1] range we consider the input to be logits and will auto apply sigmoid per element."

I noticed some discrepancies in the results, and after investigating realized that the rule above happens on a batch by batch basis rather than across the entire dataset (it happens in _binary_precision_recall_curve_format, which is called by update).

Therefore, if some batch has all logits in [0, 1], then the conversion doesn't happen for that batch.

My use case has batch size = 1, so this happens often.

Expected behavior

I'd expect that the conversion would happen consistently across the entire dataset (i.e., when calling .compute()). If not that, this behavior should be documented more prominently.

Environment

torchmetrics==1.0.3
@GabrielBianconi GabrielBianconi added bug / fix Something isn't working help wanted Extra attention is needed labels Oct 31, 2023
@github-actions
Copy link

Hi! thanks for your contribution!, great first issue!

@Borda Borda added the v1.0.x label Oct 31, 2023
@ocharles
Copy link

I came to report just this. As a "workaround", I always call sigmoid first, as at least that path is sound.

@ocharles
Copy link

Dupe of #1604, probably

@SkafteNicki SkafteNicki linked a pull request Dec 20, 2023 that will close this issue
27 tasks
@Borda
Copy link
Member

Borda commented Jan 16, 2024

@GabrielBianconi could you please clarify how you installed TM 1.0.3, as at the time the latest was already 1.2 🐰

@GabrielBianconi
Copy link
Author

I installed it a few months before my post. But I'm pretty sure that the bug was still on master back then.

@Borda Borda added the good first issue Good for newcomers label Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug / fix Something isn't working good first issue Good for newcomers help wanted Extra attention is needed v1.0.x
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@ocharles @GabrielBianconi @Borda and others