Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't adjust MoveOverhead by increment #2684

Closed

Conversation

protonspring
Copy link

This is a functional change to address a potential timing issue for slow networks. Move Overhead is unnecessarily attached to increment which doesn't allow use of small increments on slow networks (needing high Move Overhead).

If we make Move Overhead default to 10ms this should work fine for all timed games. Also, by not adjusting it we allow the user to set Move Overhead as they choose which will help users adjust time usage on slow networks.

Do I need to test other time controls?

STC, sudden death.
LLR: 2.94 (-2.94,2.94) {-1.50,0.50}
Total: 169368 W: 38023 L: 38054 D: 93291
Ptnml(0-2): 3767, 20250, 36595, 20391, 3681
https://tests.stockfishchess.org/tests/view/5ebf25efe9d85f94dc42986f

@vondele
Copy link
Member

vondele commented May 16, 2020

since our standard TCs are not negatively impacted by this, I think it is not necessary to test other TCs.

@ddugovic
Copy link

ddugovic commented May 16, 2020

There may be room to further improve this for user convenience in the future (the original patch might not have required std::max but maybe could have added the two all three terms, or omit the 10 altogether).

@protonspring
Copy link
Author

protonspring commented May 16, 2020

This is a tangential question, but is there any way for SF to determine the speed of a particular game environment. . based on timing of messages, or unexpected time left per move, or something else? That would be cool if SF could adjust move overhead automatically (i.e., based on actual measured and/or determined move overhead)?

@vondele
Copy link
Member

vondele commented May 16, 2020

@ddugovic can you clarify? Not sure I understand what you would suggest.

@protonspring I think automatic moveoverhead determination is not a good idea. For example, the average ping time to a server might be 10ms, but the move overhead might be set to deal with the once in a 100 moves ping time of 500ms.

@vondele
Copy link
Member

vondele commented May 16, 2020

@protonspring I'd be curious to know what would happen if we set move overhead to 0 default for the 10+0 TC in the context of this patch. Could you give it a try?

@protonspring
Copy link
Author

protonspring commented May 16, 2020 via email

@ddugovic
Copy link

I think that somehow the game increment could influence Move Overhead - my bot regularly accepts challenges at a variety of TCs so I'm reacting to perceived time mis-management. I recognize this function is complicated, because we're solving a complex problem...

As a chess player in zero-increment games I protect against the unlikely possibility that there are 100+ moves remaining by forcing myself to move quickly. In games with increment, I move more slowly. So I have a couple ideas:

  1. Maybe somehow limits.inc[us] could inversely correlate with mtg? Perhaps mtg /= (limits.inc[us] + 1) or something else, I don't know...
  2. moveOverhead += limits.inc[us] / 2; was my original idea but it might not make sense.

@vondele
Copy link
Member

vondele commented May 16, 2020

@ddugovic I had also considered your suggestion 2 since it seems to make sense. But we in fact now almost have this (as mentioned in the forum). We have the term:

maximumTime = std::min(0.8 * limits.time[us] - moveOverhead, max_scale * optimumTime);

so maximumTime, even with moveOverhead == 0, will always leave 0.2 * limits.time[us] time on the the clock. Since limits.time[us] >= limits.inc[us], we have an implicit moveOverhead of limits.inc[us] * 0.2

Meanwhile 10+0 finished with moveOverhead set to 0, suggesting this yields 10+ Elo improvement. , and only 2 time losses in 4000 games.

@protonspring so, one more test please, with 10+0.1 and 0 moveOverhead.

@protonspring
Copy link
Author

I also think move overhead and increment are inversely related. If the increment is small, increasing move overhead makes more sense to me than decreasing it.

@protonspring
Copy link
Author

protonspring commented May 16, 2020

With moveOverhead = 0, 10+0.1 games, the patch moves quite a bit faster than master resulting in worse performance.

We can mask the problem by raising moveOverhead to 10+, but wouldn't it be better to address it in the time calculations? All we need to do is: "If the move overhead is tiny (< 100ms), go a little faster."

LLR: -2.97 (-2.94,2.94) {-1.50,0.50}
Total: 49608 W: 9365 L: 9627 D: 30616
Ptnml(0-2): 856, 5904, 11509, 5716, 819
https://tests.stockfishchess.org/tests/view/5ec00b32e9d85f94dc429903

@vondele
Copy link
Member

vondele commented May 16, 2020

I would like to avoid adding new branches, if at all possible. So, should we try 10+0.1 using default move overhead 10ms?

@protonspring
Copy link
Author

protonspring commented May 16, 2020

LLR: 2.94 (-2.94,2.94) {-1.50,0.50}
Total: 83896 W: 16092 L: 16026 D: 51778
Ptnml(0-2): 1401, 9697, 19670, 9795, 1385
https://tests.stockfishchess.org/tests/view/5ec0239de9d85f94dc42991e

@vondele vondele added the to be merged Will be merged shortly label May 17, 2020
@vondele
Copy link
Member

vondele commented May 17, 2020

@protonspring OK, that looks good. I'll merge this to fix the regression those that play over a network might see.

@vondele vondele closed this in 83c9e59 May 17, 2020
@vondele
Copy link
Member

vondele commented May 17, 2020

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
to be merged Will be merged shortly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants