Skip to content

Prevent NaN values for LLR - #881

Closed
robertnurnberg wants to merge 1 commit into
Disservin:masterfrom
robertnurnberg:sprt-fix
Closed

Prevent NaN values for LLR#881
robertnurnberg wants to merge 1 commit into
Disservin:masterfrom
robertnurnberg:sprt-fix

Conversation

@robertnurnberg

Copy link
Copy Markdown
Contributor

This patch attempts to avoid NaN return values for the LLR computations.

I am not sure whether mathematically this is the most sound approach. So a review by @87flowers or @vdbergh would be great.

Fixes #879.

@robertnurnberg

Copy link
Copy Markdown
Contributor Author

Some example outputs for very one-sided matches:

Results of sf0 vs sf1 (1, 1t, 16MB, UHO_Lichess_4852_v1.epd):
Elo: -517.92 +/- 38.60, nElo: -856.15 +/- 19.08
LOS: 0.00 %, DrawRatio: 5.18 %, PairsRatio: 0.00
Games: 1274, Wins: 32, Losses: 1183, Draws: 59, Points: 61.5 (4.83 %)
Ptnml(0-2): [549, 54, 33, 1, 0], WL/DD Ratio: 15.50
LLR: 2.97 (100.8%) (-2.94, 2.94) [500.00, 1500.00]
--------------------------------------------------
SPRT ([500.00, 1500.00]) completed - H1 was accepted
Results of sf0 vs sf1 (1+0.01, 1t, 16MB, UHO_Lichess_4852_v1.epd):
Elo: -inf +/- -nan, nElo: -inf +/- -nan
LOS: 0.00 %, DrawRatio: 0.00 %, PairsRatio: 0.00
Games: 2, Wins: 0, Losses: 2, Draws: 0, Points: 0.0 (0.00 %)
Ptnml(0-2): [1, 0, 0, 0, 0], WL/DD Ratio: -nan
LLR: -111.03 (-3770.7%) (-2.94, 2.94) [-1500.00, -500.00]
--------------------------------------------------
SPRT ([-1500.00, -500.00]) completed - H0 was accepted

@robertnurnberg

Copy link
Copy Markdown
Contributor Author

As pointed out by @vondele, the acceptance of H0 in the above example is probably wrong. I also tried

-        lpr[i] = std::log(regularize(p1[i])) - std::log(regularize(p0[i]));
+        lpr[i] = std::abs(p0[i]) > std::abs(p1[i]) ? std::log(regularize(p1[i]/p0[i])) : -std::log(regularize(p0[i]/p1[i]));

but that did not improve the situation much. The problem is that in practice these p_i can actually be (very) negative. So a more sophisticated fix within the mle function is probably needed. Or an adhoc LLR return value for very one-sided match statistics.

Putting this to draft for now.

@robertnurnberg
robertnurnberg marked this pull request as draft July 3, 2025 19:21
@87flowers

87flowers commented Jul 3, 2025

Copy link
Copy Markdown
Contributor

The ability for us to find a solution for the Lagrangian multiplier θ depends on a good value for the initial values for p, since we iterate.

In this extremal case a uniform prior doesn't work.

Would have to think how best to handle this.

The lazy solution would be to bail out and to fall back to an approximation, but that feels inelegant.

Edit: Going to continue discussion on #879.

@robertnurnberg

Copy link
Copy Markdown
Contributor Author

Closing in favour of #882.

@robertnurnberg
robertnurnberg deleted the sprt-fix branch March 18, 2026 15:12
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.

SPRT numerics issue

2 participants