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

A small simplification in 'bool doEasyMove'. #673

Closed
wants to merge 2 commits into from

Conversation

joergoster
Copy link
Contributor

Passed STC non-regression test:
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 70238 W: 12855 L: 12819 D: 44564

Bench: 8716243

@ElbertoOne
Copy link

Given that the tm patch has passed: http://tests.stockfishchess.org/tests/view/573a5cdb0ebc59301a354ea9 with a change on the same line of code I think it is best to close this pull request.

@joergoster
Copy link
Contributor Author

@ElbertoOne Why? 5/42 is almost as ridiculous as 25/204. Question is, if the test at STC needs to be rerun.
OTOH, I first want to try a more general simplification. If the maintainers don't mind I will keep this pr open.

@ElbertoOne
Copy link

ElbertoOne commented May 18, 2016

@joergoster I agree that the changes in the division value for easy move are minimal, but I also think that improvement patches (like the tm patch) have preference over simplifications. So in order to continue with your simplification, I think a new test run needs to be done against the passed tm patch.

@joergoster joergoster closed this May 18, 2016
@mstembera
Copy link
Contributor

@lp--
Regarding the 1.25 factor, I believe you are referring to this patch?
#523
I just looked it over and noticed that the new TimeManagement::available() returns a larger number than before because the internal scale went from 0.76 to 1.016. This means that 1/10 should have gone to something like 1/13 and NOT 1/8 which is in the WRONG direction. Now I understand why the easy moves don't looks as easy as I remember when I first wrote it :( The idea is to save as much of the time on an easy move as possible so other moves can use it for an elo gain. However the divisor parameter isn't very sensitive because whether you save 80% or 90% of the moves time isn't very different. This is why the patch passed regression anyway. I bet you could simplify it away completely 10% at a time but the end total would be a regression of course. This is also why I don't think it lends itself to automatic tuning and doesn't justify any high precision fractions. I hope you may consider rectifying it with your next time management patch. You do great work and congrats on your latest successful time patch!

@lp--
Copy link

lp-- commented May 19, 2016

Yes, this patch. It also has ponder problem fix. Factor 1.25 from ponder went downward to both places including easy move. 0.76 changed to 1.016 because of other changes. Everything was done in one patch because it removed max function, so it became possible to move factor up and down.

So everything is correct with rescaling. The only thing is that from that time a lot of time has been saved and it was distributed proportionally to easy and other moves. This is why they don't look easy to you -- they take longer because of saved time elsewhere.

After that I remember there was another change and tuning but factor did not to drift much fluctuating between 204 and 210.

If you think they take too long you can try to change that. At least in the last tuning run there was constant drift to make easy moves faster. I have this parameter in new tuning, so we can also see where it is going.

@mstembera
Copy link
Contributor

@lp--
I can't seem to find the 1.25 factor. Can you please point to specific line(s) in the patch? Thanks.

@lp--
Copy link

lp-- commented May 19, 2016

There was somewhere discussion of the ponder bug which was fixed in that patch. I don't remember where -- maybe in issues, maybe in forum . Actually the problem was with cutechess-cli, as it did not send uci 'ponder off'. But in stockfish default before that patch was set to ponder on. This meant that in fishtest stockfish at the end of timeman.cpp was increasing optimum time by 1.25. Other guis of course sent 'ponder off', so with them stockfish was using less time than it was tuned for.

To fix that, ponder was set by default to 'ponder off', so additional factor 1.25 was needed. It was propagated downward to search.spp

@lp--
Copy link

lp-- commented May 19, 2016

This is code which was active with previous default:
if (Options["Ponder"])
optimumTime += optimumTime / 4;

@mstembera
Copy link
Contributor

@lp--
Ok so optimumTime(which was fixed) affects Time.available() which is compared to Time.elapsed() to decide whether to stop. In the normal non easy move case (line 514 master, line 516 patch) the ratio of these two was NOT adjusted by 25%. In the easy move case the comparison happens between the same exact quantities except Time.available() is scaled by 1/10 (line 517 master, line 519 patch). Since no 25% adjustment was made in the normal case, I don't understand why one was made for the easy move case? Can you please explain?

@lp--
Copy link

lp-- commented May 19, 2016

It was adjusted in normal case also. Denominator was 800 before that

@mstembera
Copy link
Contributor

@lp--
I do not see 800 anywhere. What line are you talking about?

@lp--
Copy link

lp-- commented May 19, 2016

It is in original tuning tests. It was tuned with denominators 800 for normal and 10 for easy move.
there was several tune_nfl runs. In final tests factor 1.25 was propagates to down to change 800, 10 to 640, 8

@mstembera
Copy link
Contributor

@lp--
Ok it finally all makes sense :) Thanks!

@mstembera
Copy link
Contributor

mstembera commented May 21, 2016

Just FYI, this
http://tests.stockfishchess.org/tests/view/574051440ebc59301a354fb1
is my attempt to make the normal stop time and easy move stop time use the same logic and just be a fixed ratio of each other again. Unfortunately, it's most likely elo neutral and thus unlikely to pass [0,5].

@lp--
Copy link

lp-- commented May 22, 2016

@mstembera
I remember at the time I was testing soft stops on fail low I was thinking about such option -- if to use for easy move stop time Time.available directly or Time.avialable with fail low modifications. I decided against modifications. In cases of easy moves fail lows most likely will not happen so easy move time will be decreased around twice. So now you changing this factor from 5/42 to around from 1/15 to 1/23. Do you think that it can be so low?

Maybe it is more logical to save what was thinking time on previous move and modify easy move time depending on that? If SF was thinking a lot on previous move then it can make easy move faster. if SF was thinking only little before, than maybe it needs longer easy move time now.

@mstembera
Copy link
Contributor

Thanks for the interesting comments. My intuition tempts me to speculate a lot but I have learned better. Let's see how the test ends. I do predict a yellow result.

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.

4 participants