Increase optimumTime by 10%.

[xoto10]

STC 10+0.1 :
LLR: 2.94 (-2.94,2.94) <-0.50,2.50>
Total: 47032 W: 12078 L: 11841 D: 23113
Ptnml(0-2): 159, 5098, 12746, 5373, 140
https://tests.stockfishchess.org/tests/view/613f9df1f29dda16fcca8731

LTC 60+0.6 :
LLR: 2.95 (-2.94,2.94) <0.50,3.50>
Total: 66248 W: 16631 L: 16301 D: 33316
Ptnml(0-2): 44, 6560, 19578, 6906, 36
https://tests.stockfishchess.org/tests/view/6140603d7315e7c73204a4c1

Non-regression tests with other time control styles:

Moves/Time 40/10+0 :
LLR: 2.93 (-2.94,2.94) <-2.50,0.50>
Total: 51640 W: 13350 L: 13254 D: 25036
Ptnml(0-2): 183, 5770, 13797, 5908, 162
https://tests.stockfishchess.org/tests/view/6141592b7315e7c73204a599

TCEC Style 10+0.01 :
LLR: 2.94 (-2.94,2.94) <-2.50,0.50>
Total: 20592 W: 5300 L: 5157 D: 10135
Ptnml(0-2): 81, 2240, 5544, 2317, 114
https://tests.stockfishchess.org/tests/view/61425bb27315e7c73204a6a2

Sudden death 15+0 :
LLR: 2.94 (-2.94,2.94) <-2.50,0.50>
Total: 127104 W: 32728 L: 32741 D: 61635
Ptnml(0-2): 735, 13973, 34149, 13960, 735
https://tests.stockfishchess.org/tests/view/614256a77315e7c73204a699

The first 3 tests were run with an initial version of the code, which was then modified to make the amount of extra time dependent on the size of increment. No increment gives no extra time, and the extra time given increases until an increment of 1% or more of remaining time gives 10% extra thinking time.

Further work: the ideal increase in time use may differ between stc and ltc (and beyond), so it may be worth investigating this.

Bench 6658747

[xoto10]

This change should probably alter the existing constants rather than add an extra * 1.1 operation, see master...xoto10:c5198e7
I can run tests with that code if required.

I made a comment that this would alter other time control styles (i.e. x moves in y seconds) implying this is bad, but I now think this is probably ok. If the driver for this is the improved play of stockfish then this is likely to apply there as well. I have started non-regression tests to check this.

[vondele]

can you indeed update the pull request so that the constants are changed instead of multiplied later by 1.1?
No need to retest, but please add the result of the other tests
CI fails, but I think it is unrelated, rather some issue with the github action.

[DamasClasicas]

What if this change gets worse in those games without increment?

Wouldn't be good to do a check of this first?

[xoto10]

@DamasClasicas
I requested stc non-regression tests for 40/10+0, 10+0.01 and 15+0 and It has failed the last one.

@vondele
Do we want to scrap the PR because of this? or work around it somehow if we can, or ignore it since we don't care much about sudden death? (or some other response?)

[crossbr]

Is it desirable to make optimumTime contingent on the game's time's control, so that it remains as in when the TC is no increment, but adds 10% when playing with increment?

[xoto10]

@crossbr
Yes, I had that thought. One way is https://github.com/xoto10/stockfish-xoto10/compare/fd5e779..09e6d44

[vondele]

@xoto10 I suggest we wait a bit before merging this with the failed non-regression test at sudden death. I wonder if there is something you can do, for example give more time by adding more time via the increment (like add a similar amount of time using the movestogo variable).

[locutus2]

@xoto10
Perhaps the factor 1.1 could be scaled somehow with the ratio of increment and timeleft. For sudden death this is zero and then no upscaling should occur. So if less relative increment then less upscaling of time is done.

[xoto10]

A list of the initial tests run:

STC 10+0.1 :
LLR: 2.94 (-2.94,2.94) <-0.50,2.50>
Total: 47032 W: 12078 L: 11841 D: 23113
Ptnml(0-2): 159, 5098, 12746, 5373, 140
https://tests.stockfishchess.org/tests/view/613f9df1f29dda16fcca8731

LTC 60+0.6 :
LLR: 2.95 (-2.94,2.94) <0.50,3.50>
Total: 66248 W: 16631 L: 16301 D: 33316
Ptnml(0-2): 44, 6560, 19578, 6906, 36
https://tests.stockfishchess.org/tests/view/6140603d7315e7c73204a4c1

TCEC-style 10+0.01 :
LLR: 2.94 (-2.94,2.94) <-2.50,0.50>
Total: 19784 W: 5201 L: 5056 D: 9527
Ptnml(0-2): 92, 2160, 5238, 2315, 87
https://tests.stockfishchess.org/tests/view/614159967315e7c73204a59b

Moves/Time 40/10+0 :
LLR: 2.93 (-2.94,2.94) <-2.50,0.50>
Total: 51640 W: 13350 L: 13254 D: 25036
Ptnml(0-2): 183, 5770, 13797, 5908, 162
https://tests.stockfishchess.org/tests/view/6141592b7315e7c73204a599

Sudden Death 15+0 :
LLR: -2.96 (-2.94,2.94) <-2.50,0.50>
Total: 51760 W: 13280 L: 13536 D: 24944
Ptnml(0-2): 365, 5812, 13669, 5782, 252

[dsmsgms]

Seems to work with "almost" no increment, will it disabled for no increment then?

[xoto10]

Seems to work with "almost" no increment, will it disabled for no increment then?

I have a version which disables the extra time for no increment, I have a number of different tests to run ...

[locutus2]

see my comment here. xoto10@c1dc4ce#commitcomment-56534868

[locutus2]

I doesn't know if it possible that time[us] becomes zero. But for safety i would cap it at the lower end by 1 like its also done for timeLeft to avoid division by zero errors:
double optExtra = std::clamp(1.0 + 10.0 * limits.inc[us] / std::max(limits.time[us], TimePoint(1)), 1.0, 1.1);

[xoto10]

I did that at one point, but noticed that testing without the max() didn't seem to trigger any problems for me. I did some reading and it seems that / 0.0 (float/double) is well defined in c++ and returns +/-inf, which we then clamp, so in theory it shouldn't be a problem. Of course if people think it might be a problem with some compiler somewhere we can code it more defensively with the max() ... whatever people think is best.

[locutus2]

@xoto10
I have checked that in the case of division by zero for floats/doubles we get Inf and that is correctly clamped to 1.1. Only if we would have NAN we had also NAN as clamping result. So i think it is no problem we have to care of.