-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Decouple confidence normalization from ranking_length
parameter
#9642
Comments
@TyDunn Just bringing this issue to your notice. This coupling between |
We have that:
So how about we ...
wdyt? |
Oh, similarly, we could parameterize sklearn intent classifier - that one will always report confidences for top 10 intents (if there are that many) |
Probably a mistake, should be fixed.
I agree with everything but lets keep the behaviour universal for all 3 components, i.e.:
If people want to get the old behaviour they can set |
👍 wrote 10 above because
This isn't possible for
Wasn't sure if it's ok to change that one, I'm fine with breaking things to unify the behaviour :D |
Ahh right, forgot about it. So, in the setting-to-0 approach, the confidences that have been set to 0 don't contribute to renormalization (if renormalization was turned on), right?
This is exactly why this needs to be done in 3.0 . Renormalization is completely wrong IMO but if it floats the boat for some users, we should support it, but just not the default behaviour 😅 |
I'm not sure what you mean by they don't contribute to renormalisation. We normalise by the sum of the confidences. If we remove them from the list (in DIET), they don't contribute to the sum, just like they don't contribute to the sum if we set them to 0 (in TED). Btw, there's also a tiny bug in the normalisation
👍 |
Totally my bad, don't know what I was thinking |
No worries, happens to me all the time ^^. Btw, actually there's no reason to mask the probabilities in |
What problem are you trying to solve?
Currently, all ML components (
DIETClassifier
,ResponseSelector
,TEDPolicy
) have a parameter calledranking_length
which is supposed to be used when the user does not want to see the confidences of all possible candidate labels (intents / actions) and instead want to see a smaller number of labels.However, what happens by default is that model's predicted confidences for the labels inside the ranking length are renormalized to sum up to 1. This is wrong because this can artificially boost the confidences of labels and hides the original confidences that were predicted by the model itself. Renormalization step should be decoupled from this parameter.
Removing the renormalization step is also not possible because if a user has a lot of intents, say 200, all the predicted confidences tend to be very low which is problematic for fallback. So, it should be done only when needed, i.e. the user sees a benefit of it. Anecdotal evidence of this happening with customers
What's your suggested solution?
Have a separate parameter, something like -
renormalize_confidences
which if set toTrue
applies the renormalization.Note that the re-normalization step is currently only applied when
loss_type
is set tocross_entropy
and that condition should still stay.Examples (if relevant)
No response
Is anything blocking this from being implemented? (if relevant)
No response
Definition of Done
ranking_length
parameter.The text was updated successfully, but these errors were encountered: