-
Notifications
You must be signed in to change notification settings - Fork 12
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
Hybrid ce loss changes #364
Conversation
regression_input = torch.inner(input, self.brackets.to(input.device)) | ||
# regression loss needs normalized logits to probability as input to do inner product with self.brackets | ||
# we apply softmax on the raw logits first | ||
softmax_input = self.softmax(input) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we discussed that the softmax
should go as the last_activation
since we want the classification metrics to work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mentioned about this on slack about the auroc and average precision metrics. It looks like they are happy with both probabilities or logits. In that case we don't need a custom CE loss? Or do you think there would be other classification metrics which only accept probabilities as the input?
` Accepts the following input tensors:
- ``preds`` (float tensor): ``(N, C, ...)``. Preds should be a tensor containing probabilities or logits for each
observation. If preds has values outside [0,1] range we consider the input to be logits and will auto apply
softmax per sample.`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, just saw your Slack message!
I checked also Accuracy and f1. I think this is consistent across metrics 🎉
Changelogs
Checklist:
feature
,fix
ortest
(or ask a maintainer to do it for you).discussion related to that PR
Fix to hybridceloss, but it is not actually helping the hybridceloss for val and test using the debug config.