Improve LLR algorithm

[87flowers]

Normalized elo only for commentry.

Happy to add logistic elo support if required.

[Disservin]

i wouldnt want to have it in a dead state, so id prefer if this is also implemented

[Disservin]

cc @gahtan-syarif

[87flowers]

Implemented.

[gahtan-syarif]

Nice, this uses the same MLE formula as fishtest right?

[87flowers]

@gahtan-syarif Yes, it does.

[87flowers]

Not sure why macOS is crashing. We do however often do an intentional floating point divide by zero when calculating bounds for θ, which should not usually be an issue.

[robertnurnberg]

I assume a review by @vdbergh would be good.

[robertnurnberg]

Not sure why macOS is crashing. We do however often do an intentional floating point divide by zero when calculating bounds for θ, which should not usually be an issue.

To debug this further, you could create a new branch that outputs a lot more debug information. Then open a PR from that branch to the master branch of the fork in your repo. Github should then run the CI on your fork, where you can study the debug output in more detail.

[Disservin]

later today i check if i can reproduce this on my mac

[Disservin]

it hit your own assert lol

Assertion failed: (f_a < 0 && 0 < f_b), function itp, file sprt.cpp, line 163.
Process 15950 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = hit program assert
    frame #4: 0x00000001000db9f8 fastchess-tests`double fastchess::SPRT::getLLR_normalized<3ul>(double, std::__1::array<double, 3ul>, std::__1::array<double, 3ul>, double, double) const::'lambda'(double, double)::operator()(double, double) const (.cold.1) [inlined] double fastchess::itp<double fastchess::SPRT::getLLR_normalized<3ul>(double, std::__1::array<double, 3ul>, std::__1::array<double, 3ul>, double, double) const::'lambda'(double, double)::operator()(double, double) const::'lambda'(double)>(f=<unavailable>, a=<unavailable>, b=<unavailable>, k_1=<unavailable>, k_2=<unavailable>, n_0=<unavailable>, epsilon=<unavailable>) at sprt.cpp:163:5 [opt]
   160          std::swap(a, b);
   161          std::swap(f_a, f_b);
   162      }
-> 163      assert(f_a < 0 && 0 < f_b);
   164
   165      double n_half = std::ceil(std::log2(std::abs(b - a) / (2.0 * epsilon)));
   166      double n_max  = n_half + n_0;
Target 0: (fastchess-tests) stopped.
warning: fastchess-tests was compiled with optimization - stepping may behave oddly; variables may not be available.

[Disservin]

values just before the assert

(double) a = 1.9878485028081294
(double) b = -2.0121656491861892
(double) k_1 = 0.10000000000000001
(double) k_2 = 2
(double) n_0 = 0.98999999999999999
(double) epsilon = 9.9999999999999995E-8
(double) f_a = 4275402293727788.5
(double) f_b = 1990145231113661
(double) n_half = 3.0493289413280831E-314
(double) n_max = 3.0493289452806083E-314

[Disservin]

fyi, macos passes with -ffp-contract=off

[87flowers]

thanks, i'll look at this this weekend

[87flowers]

tl;dr: fix in ff9276f

Root cause: Allowing fp contraction results in improper results arising where we would normally expect +/- inf to be generated. Compare:

Normal run:

phi[0] = -0.500000
phi[1] = 0.000000
phi[2] = 0.500000
theta bounds = -2.000000 2.000000
f(theta) bounds = inf -inf
theta = 0.011206
p[0] = 0.256674
p[1] = 0.486653
p[2] = 0.256674
---
phi[0] = -0.500000
phi[1] = 0.000000
phi[2] = 0.500000
theta bounds = -2.000000 2.000000
f(theta) bounds = inf -inf
theta = 0.011206
p[0] = 0.256674
p[1] = 0.486653
p[2] = 0.256674
---
phi[0] = -0.502938
phi[1] = -0.001175
phi[2] = 0.497062
theta bounds = -2.011820 1.988318
f(theta) bounds = 1155602826786613.250000 -inf
theta = -0.005000
p[0] = 0.254595
p[1] = 0.486650
p[2] = 0.258755
---
phi[0] = -0.503056
phi[1] = -0.001031
phi[2] = 0.496977
theta bounds = -2.012166 1.987849
f(theta) bounds = inf -inf
theta = -0.004862
p[0] = 0.254613
p[1] = 0.486650
p[2] = 0.258737
---

With contraction:

phi[0] = -0.500000
phi[1] = 0.000000
phi[2] = 0.500000
theta bounds = -2.000000 2.000000
f(theta) bounds = inf -inf
theta = 0.011206
p[0] = 0.256674
p[1] = 0.486653
p[2] = 0.256674
---
phi[0] = -0.500000
phi[1] = 0.000000
phi[2] = 0.500000
theta bounds = -2.000000 2.000000
f(theta) bounds = inf -inf
theta = 0.011206
p[0] = 0.256674
p[1] = 0.486653
p[2] = 0.256674
---
phi[0] = -0.502938
phi[1] = -0.001175
phi[2] = 0.497062
theta bounds = -2.011820 1.988318
f(theta) bounds = 1224224144331749.250000 -39094986504801456.000000
theta = -0.005000
p[0] = 0.254595
p[1] = 0.486650
p[2] = 0.258755
---
phi[0] = -0.503056
phi[1] = -0.001031
phi[2] = 0.496977
theta bounds = -2.012166 1.987849
f(theta) bounds = 1990145231113661.000000 4275402293727788.500000

This occurs because the values of the bounds of theta are the negative inverses of elements for phi, thus generation of +/- infinity is practically guaranteed here. FP contraction messes with the precision of the divide by zero.

The solution to this is to avoid computation of f at extremal values in itp, by passing in our pre-known values for f(a) and f(b) instead of computing them.