-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Inconsistent pv/bestmove at low node counts #834
Comments
Perhaps worth noting is that h2h3 (Kh3) is mate in 14, while h2g1 (Kg1) is mate in 15. |
Re-run test with a fresh instance of stockfish for each run. Hash does not appear to have been cleared. Most engines will exhibit what appears to be non-deterministic behavior if hash is not cleared. Restarting engine ensures you are starting with a cleared hash. The clue hash was not cleared is that you have a different number of nodes being searched at each depth level. |
@MichaelB7 The issue I'm raising isn't about the non-determinism; it's the inconsistency between the pv and the bestmove. |
Looks like a bug to me, but you are using SF7 so the question is now whether this still happens with the latest development version. (I could not immediately reproduce it on this position. Maybe if I try harder.) |
OK, I can reproduce it. I just have to repeat the test a few more times. |
Thanks @syzygy1. Was that with Stockfish 7 or the dev version? |
With the latest version (or at least one of the latest versions). |
Reading through the UCI standard it doesn't seem to be an explicit violation of the protocol -- the protocol never explicitly says that the pv root must == the bestmove** -- but I'm not aware of other engines that exhibit this behavior. So if it is by design, then I'd say the design is questionable... ** However, the standard does say that the pv is "the best line found". |
There is (somewhat ungrammatically): "Directly before that the engine should send a final "info" command with the final search information, the the GUI has the complete statistics about the last search." But it is not clear that this must include the current pv. There certainly are other engines that output a best move that is not the first move of the last pv, but I think that is usually considered a bug. But in this particular case the fail-high/fail-low "pv" was in principle output, except that it was suppressed together with some other information during the first 3 seconds of the search. In multi-threaded mode, Stockfish does take care to output a new pv if it decides, just before outputting the best move, to play a move found by a different search thread (i.e. a thread other than the main search thread that produces the "regular" PVs that you see). So I'm sure it is considered desirable to keep the last pv consistent with the bestmove. Anyway, I'll see if I can find a simple fix if nobody else beats me too it. |
@syzygy1 The easiest solution seems to be to output the PV in any case, not only if a new best thread has been found in a multi-threaded search. This would catch such cases as described here, while in standard cases the pv is simply output twice. Which may considered to be ugly, as well! You could also raise a flag when the search has been stopped while still in the fail-high/fail-low aspiration loop, and also output a new pv in that case. |
Actually I think outputting the last pv in any case might be the right thing to do. It will also update the statistics such as nodes, nps, tbhits etc. However, it should probably be avoided if SF happened to finish the last iteration and then immediately decided time is up (or rather, no time left for the next iteration). Reading again your comment in full, I think we fully agree :-) |
The problem with outputting the PV always (unless the last iteration was completed) is that the PV may be truncated, which isn't very nice. So it seems better to keep track of the first move of the last full PV displayed and then display a possibly truncated PV if the bestmove has changed. Now I get this: Truncated PV, but with the correct (changed) first move. |
Nice. But outputting nodes and time right before the final pv in such a case looks somewhat redundant. :-) |
Yes, but the same already happens if the "best thread" changes. If the main search thread finds out at the very end that another search thread completed a higher depth, then the PV collected by that other thread is displayed after nodes and time were outputted. I wondered a bit whether tbhits should be outputted as well in that info node/time line, but the same can be asked for nps. The best way to do that is to call UCI::pv(), but then a truncated PV will be output too. It would perhaps be possible to make outputting the pv in UCI::pv() optional (depending on the arguments with which it is invoked). But how far do we want to go in making this "perfect"... |
Stockfish outputs truncated pvs all the time, in deep and shallow searches too. I wasn't aware that was considered to be a problem. I'd think it would be best to follow the communication protocol and output a consistent (if truncated) pv when the bestmove changes. |
The patch I proposed also makes sure that the last pv is consistent with the best move. But I thought it was not nice to "overwrite" the last full PV with a new truncated PV if the first move did not change anyway. However, I now think that there is no need to worry about this. So I will simplify. |
New attempt: #846 |
After #846 I can't reproduce this. Thank you! |
In some circumstances, especially at low depth or low fixed node counts, stockfish will return a pv where the first move does not match the bestmove. As far as I have observed it, this behavior is not deterministic and sometimes requires multiple engine runs before it is exhibited.
For example:
The text was updated successfully, but these errors were encountered: