Apply KL-divergence epsilon floor symmetrically to p and q#184
Merged
Apply KL-divergence epsilon floor symmetrically to p and q#184
Conversation
Previously kl_divergence clipped only q_receiver by epsilon=1e-10:
q_receiver = np.maximum(q_receiver, epsilon)
return np.sum(rel_entr(p_donor, q_receiver))
This was asymmetric: a category present in p but absent in q
contributed p * log(p / eps) (a large finite value that depends
arbitrarily on epsilon), while a category present in q but absent in p
contributed rel_entr(0, q) = 0 (a free pass). The sign of the
divergence bias depended entirely on which side had the missing mass.
Now apply the epsilon floor to BOTH p_donor and q_receiver, then
renormalise so they are still valid probability vectors. KL behaves
consistently regardless of which side has a missing category.
Tests
- test_kl_divergence_symmetric_epsilon_handling: a category absent
from the donor but present in the receiver contributes positively
to KL and the result is finite.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
MaxGhenis
commented
Apr 17, 2026
Contributor
Author
MaxGhenis
left a comment
MaxGhenis
left a comment
There was a problem hiding this comment.
KL symmetric epsilon verified:
- Both
p_donorandq_receiverare floored withepsilon=1e-10and renormalised back to probability vectors. test_kl_divergence_symmetric_epsilon_handlingasserts strictly positive and finite KL when donor lacks a category the receiver has (previously this was a free pass:rel_entr(0, q) = 0).
Trivial, correct. CI all green. Mergeable. LGTM.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes finding #12.
kl_divergencepreviously clipped onlyq_receiverbyepsilon=1e-10, so a category present inpbut absent inqcontributedp * log(p/eps)(a large finite value that depends arbitrarily onepsilon), while a category present inqbut absent inpcontributedrel_entr(0, q) = 0— a free pass. The sign of the divergence bias depended entirely on which side had the missing mass.Now both
p_donorandq_receiverget the same epsilon floor and are renormalised back to probability vectors.Test plan
test_kl_divergence_symmetric_epsilon_handling: a category absent from the donor but present in the receiver contributes positively to KL and the result is finite.tests/test_metrics.pypasses (53/53)