Skip to content

fix: correct off-by-one and tie handling in od p-value computation - #959

Open
shivamlalakiya wants to merge 1 commit into
SeldonIO:masterfrom
shivamlalakiya:fix/od-base-p-value-off-by-one
Open

fix: correct off-by-one and tie handling in od p-value computation#959
shivamlalakiya wants to merge 1 commit into
SeldonIO:masterfrom
shivamlalakiya:fix/od-base-p-value-off-by-one

Conversation

@shivamlalakiya

Copy link
Copy Markdown

Fixes #958

What

od/pytorch/base.py:199 and od/sklearn/base.py:148 compute the outlier
p-value as

return (1 + (scores[:, None] < self.val_scores).sum(-1))/len(self.val_scores) \
    if self.threshold_inferred else None

od/pytorch/ensemble.py:114, same author, same subpackage, computes the
same "add one to the count the test score beats" idea but divides by
len(self.val_scores) + 1:

p_vals = (1 + less_than_val_scores.sum(1))/(len(self.val_scores) + 1)

The base.py denominator can push the value above 1. Reproduced through
the public API (0.13.0 wheel, Python 3.11, numpy 1.26.4):

>>> d = GMM(n_components=1, backend='sklearn')
>>> d.fit(X_fit)          # X_fit as in the linked issue
>>> d.infer_threshold(X_cal, fpr=0.5)   # len(val_scores) == 4
>>> d.predict(np.array([[3.5]], dtype=np.float32))['data']['p_value']
array([1.25])

The same line's strict < separately collapses tied scores toward 1/n
instead of 1. Full numbers, and the exact-rational tie-support sweep,
are in #958.

Fix

One line changed per file: < becomes <=, and the denominator becomes
len(self.val_scores) + 1, matching ensemble.py.

return (1 + (scores[:, None] <= self.val_scores).sum(-1))/(len(self.val_scores) + 1) \
    if self.threshold_inferred else None

Verified against the patched files:

>>> s.val_scores = np.full(9, 5.0); s._p_vals(np.array([5.0]))
array([1.])
>>> s.val_scores = np.array([1.0,2.0,3.0,4.0]); s._p_vals(np.array([0.0]))
array([1.])

I searched od/pytorch/tests and od/sklearn/tests for coverage of this
expression and found none, so this is a pure two-line change with nothing
else to update. ensemble.py:114 is untouched; its denominator is already
correct and its strict comparator is a separate, package-wide question
I've left out of this PR (noted in #958).

od/pytorch/base.py and od/sklearn/base.py compute the outlier p-value as

    (1 + (scores[:, None] < self.val_scores).sum(-1)) / len(self.val_scores)

The sibling computation in od/pytorch/ensemble.py:114 does the same count
but divides by len(self.val_scores) + 1, which is what keeps that value
a valid p-value in [0, 1]. With the base.py denominator, a test score
below every validation score returns (n+1)/n, e.g. 1.25 at n=4, in the
documented p_value output field. The same expression's strict '<' also
collapses to 1/n instead of 1 when the test score ties every validation
score.

This changes both files to '<=' over (len(self.val_scores) + 1),
matching ensemble.py's denominator and closing the tie gap. No test in
od/pytorch/tests or od/sklearn/tests exercises this expression, so
nothing else needs to change.
@CLAassistant

CLAassistant commented Aug 18, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

od p-value computation disagrees with od/pytorch/ensemble.py's own denominator

2 participants