fairy-stockfish / Fairy-Stockfish

chess variant engine supporting Xiangqi, Shogi, Janggi, Makruk, S-Chess, Crazyhouse, Bughouse, and many more
https://fairy-stockfish.github.io/
GNU General Public License v3.0
589 stars 186 forks source link

Support pondering in CECP protocol #103

Closed ianfab closed 3 years ago

ianfab commented 4 years ago

Ponder option is set on easy and hard commands already, but actually running pondering is not supported yet.

HGMuller commented 3 years ago

This should be almost trivial to implement, and I urge you to give it some priority, as people are pestering me that pondering doesn't work when running under WinBoard.

After printing the move, when pondering is on, you should start a ponder search on the ponder move, and remember the latter. When a usermove comes in during a ponder search, compare it to the remembered ponder move to decide whether you abort the search (UCI 'stop') or turn it into a timed search (UCI 'ponderhit', after clearing the remembered ponder move). When a ponder search (as recognized by a ponder move being remembered) aborts, ignore the result and clear the remembered ponder move.

That is really all.

ianfab commented 3 years ago

Yes, conceptually pondering might be trivial, but since Stockfish is designed with the UCI protocol in mind, it makes implementing pondering in CECP very difficult. Searches are run asynchroneously from the thread that handles stdin, which blocks until receiving new input, so letting the finished search trigger an event without receiving any new input is working against the design. After all, the CECP implementation in Fairy-SF more or less is just a very small wrapper around the UCI logic, it does not really properly implement the CECP state machine.

HGMuller commented 3 years ago

Well, I don't know how exactly the input thread would communicate to other threads that a search should start. (launching new threads, waking up existing ones...) But it is hard for me to imagingine that whatever it does, the thread responsible for printing the move could not do exactly the same.

HGMuller commented 3 years ago

One way to solve this would be to create a separate thread for processing CECP commands: on reception of 'xboard' the input thread would launch the CECP interpreter as a separate thread, instead of jumping to it, and then jump to a loop where it writes everything it gets from the GUI into an (internal) pipe it created. The CECP interpreter would read from that pipe, instead of from stdin. At the point where the search thread would have printed the move for the GUI, it could then inject a command of your own making into that pipe, which would make the CECP interpreter launch a ponder search.

HGMuller commented 3 years ago

Another method that might work is to forcefully release the input thread from the blocking when the main search thread prints its move. In Linux this can be done by sending a signal. I am not sure what the corresponding Windows method would be. The resulting error code (EINTR) from the blocking input call could be interpreted as the command needed to start the ponder search.

[Edit] It seems that Windows has a CancelSynchronousIo function, which can be called by the main search thread with the handle of the input thread to release it from its blocking. This would make the blocked read operation return 0, and a following call to GetLastError would return ERROR_OPERATION_ABORTED. This can then be handled as if the command "ponder" was read.

[Edit2] If all this is too difficult to make platform independent, how about this: just print a new CECP feature ponderhelp=1. If it is accepted, let the main search thread send "ponder " to the GUI directly after it sent the move. XBoard will then send that back unmodified, so your input thread is woken up and knows what to ponder on.

HGMuller commented 3 years ago

Strange. Yesterday I posted an elaborate description of an idea here, and now I don't see it anymore. Let me quickly repeat it:

A quite simple solution to the ponder problem exist: just let the engine use the GUI to relay the ponder move, like would be done in UCI. One can make a CECP-compliant GUI echo a message to the engin, by wrapping that message in a feature command:

feature XXX=0

from the engine must be replied to with

rejected XXX

if XXX is not an existing feature name. As none of the possible chess moves is (or will ever be) in use as a feature name, you can make XXX the ponder move (or the ponder move prefixed with some keyword to facilitate recognizing it). So the following changes would do it:

ghost commented 3 years ago

Hello @HGMuller . I know you are a Winboard developer or someone who contributed to it. I have a question about the Winboard feature. In the mode section of Winboard, there is a machine/analysis mode. Do you know what's the difference between machine / analysis mode with Ponder function? At first glance, the analysis mode seems to include the Ponder function, but it is different from the general Ponder function. Can you tell me what is Winboard's analysis mode / machine mode? And, can you tell us what's the difference between the analysis mode / machine mode where the Ponder function of Winboard is available?

HGMuller commented 3 years ago

This is rather off-topic here; this is the Stockfish development site. Please ask general questions about computer chess on forums like talkchess.com or open-aurec.com/wbforum . This time I will answer:

In Machine White or Machine black mode the engine will always automatically play a move for the mentioned color, when it has been thinking long enough, and you cannot enter moves (with mouse or by typing) when the mentioned color is on move. Taking back moves is not easy. You typically use that when you want to play an entire game against the engine. In Analyze mode it will never play a move, no matter how long you let it think. And you will be allowed to play moves (or take them back, with the < button) for both sides. It is typically used for finding the answer to questions like "what does the engine think is best if I play this or that".

ianfab commented 3 years ago

Only through my confusion from the last comment I got that there is a parallel discussion and Fairy-SF development going on: http://www.open-aurec.com/wbforum/viewtopic.php?f=2&t=54747.

Fixing Stockfish ourselves is a lot less work than trying to convince its developers to do it...

The main reason why it takes time until change requests are appearing in the master branch (besides that implementation simply takes time) is that having a stable version on master is very important to me and therefore I only merge changes after they have been thoroughly tested and I consider them stable. And even more so if changes are backwards-incompatible, such as changes to protocols, because otherwise it causes confusion on the user side and additional work for me. If you check other branches you will see that I am working on most of the requested changes, but if waiting for a stable solution is not an option, then writing your own implementation is the only option. Although it of course it goes without saying that this in my opinion is a waste of time, because joint discussion and development is more efficient than doing double work.

I did not reply here for a while because I first needed to look into possible solutions, since I do not really want to discuss and explore workarounds/hacks in too much detail before at least having a deeper look whether a proper solution is possible, which could render the discussion obselete. Besides that there are a lot of parallel discussions going on in this repository, so I might sometimes be slow to respond, especially if a topic requires more thought first.

ghost commented 3 years ago

When entering https://www.youtube.com/watch?v=pzUyrUBuFns, Winboard developer @HGMuller implemented CECP Ponder function. What this means is that Fairy Stockfish can implement CECP Ponder functionality.

ianfab commented 3 years ago

Yes, but does developing a workaround without even sharing the code contribute anything to the progress of this project? Nope, because it will remain a separate fork and quickly get out of date, and cause confusion in the maintenance of the project, because users might use inofficial versions and report bugs based on them, but I of course can not give any support for inofficial versions. Of course it is great for you if you are happy with the functionality you get using that version, but that does not mean that it is of any benefit for the project itself (and I hope that is our goal, because that in the end benefits all users including yourself).

It is obvious that pondering can be implemented, the question is how to do it in a way that it is relatively clean and robust enough to be included in a stable release of the engine. I am working on that, but it simply takes time. Hacking something together that somehow works is easy, and can of course be an appropriate solution if only a few people need it, the hard part is making it maintainable and in this way making it accessible to all users via the main branch/version of the project.

HGMuller commented 3 years ago

@Ianfab - Note that I in no way want to criticize your attitude; on the contrary, it makes perfect sense, and I would probably do exactly the same myself. Fairy-Stockfish is a very big project, (which I greatly applaud), so it is understandable that you have to set priorities, and cannot work on everything at the same time. And the CECP ponder issue can in general be easily worked around by running in UCI mode. Janggi is just one of the few variants for which this work-around does not work. But OTOH an entire country suffers from that for its national variant. This is probably the largest 'customer base' Fairy-Stockfish will ever have.

So if I can find anything to make ir possible to make WinBoard + SF fully operational, even if it is through a dirty hack that can only be considered a temporary solution, I consider it worth the effort. In due time you will fix the problem in a fundamentally correct way, and this can then silently replace the hack. I am in no way suggesting you should incorporate my hack into the main branch. But that I have been fooling around with it a bit could be helpful with some aspects of the ultimate fix. That is the blessing of open-source software.

As to the question of bas-del that triggered this: I don't have the executable you ask for. Stockfish doesn't compile on the compiler I use (gcc 3.4.4 under Cygwin), and all I did is to suggest a few patches to the user that works on this ('janggi-korea' on WB forum; I think he posted also here, but under a different name). He applied them, compiled the modified SF, and tested them out.

The problem of inter-thread communication was solved by a hack, (using the GUI as relay), and this should obviouly get a better solution. But for human-engine games it appears to work flawlessly, and that was the goal. The remaining part of the implementation is pretty much as I imagine a thorough solution would do it as well:

That is all I did, and to my dismay it caused a mysterious problem: when you abort an ongoing ponder game with New Game, it crashes the engine. From the debug log it seems everything works as expected; the 'force' command terminates the ponder search, a few more CECP commands are processed (ping, memory, cores), and on reception of the 'new' command the engine dies. As the 'new' command does nothing that I would consider suspect or a possible source of trouble, my messing with 'pos' must have created some inconsistent state in SF that only causes a crash when handling 'new', but not during handling a ponder hit or miss.

As I am basically working here on a program I have never seen before, written in a programming language I am not familiar with, I am dependent on "monkey-see-monkey-do" programmings techniques, and this problem seems beyond me. Fortunately there is a work-around: when the engine specifies feature reuse=0 it will never get a 'new' command after pondering, because a new instance will be started on New Game. Nevertheless it bothers me to use a program that seems to be primed for crashing, even when you quit it before the actual crash happens.

ianfab commented 3 years ago

Thanks for the clarification. Independently providing temporary workarounds while I am working on final solutions is welcome, the only change I would ask for is in communication, because the above mentioned thread sounds very different, but I might have caused this in the first place by my temporary inactivity on this thread. I seem to have underestimated how important pondering is to some users, because I basically never use it myself, since I either do analysis or engine vs. engine games (where pondering can be detrimental). And even in the rare case I play against the engine myself I do not need pondering at all, since the engine anyway far surpasses my level. Therefore, I did not expect that this is such a pressing issue.

The steps you outlined describe quite well what I implemented, although I unfortunately did not find any way around creating a temporary thread that triggers the ponder search, since the main search thread would otherwise deadlock. I have not implemented a solution to the highlight problem yet, but I thought I might just cache the list of legal moves in the protocol state when pondering in order to avoid having to modify or copy the position object. I also have not investigated the potential problem with starting a new game while pondering yet, but I am in general worried about stability in the XBoard protocol implementation after this change, therefore I am trying to refactor it to make possible states more transparent and avoid illegal transfers between states that might cause crashes.

ghost commented 3 years ago

Hello. @HGMuller I sent the email back to you. Do you have this files, can you send it to me?

ghost commented 3 years ago

Hello. @ianfab @HGMuller

CONFIRMATION OF REGISTRATION at http://www.open-aurec.com/wbforum/ucp.php?mode=register

How many pieces are left on the chess board after the moves 1.e2-e4 e7-e5?: This question is a means of preventing automated form submissions by spambots.

Do you know what the answer is? If I enter 1, 2, 3... it turns out that it is not correct.

HGMuller commented 3 years ago

32

HGMuller commented 3 years ago

Another point that came up: 'force' is not the only command that should terminate a ponder search. XBoard sometimes sends 'result' to end a game without a preceding 'force', and the CECP v2 specs do not forbid that. (A v3 proposal that would make most commands illegal when not in force mode was never officially accepted.) And it stands to reason that after 'result' indicates a game finished, the engine should revert to force mode, and no longer think or ponder about new moves.

So for an engine that doesn't do any learning, 'result' should be synonymous to 'force', and not a no-op.

HGMuller commented 3 years ago

I have a question:

To make a less messy CECP implementation it would be beneficial if the main search thread would be able to terminate a ponder search in silence. As it already does in the case of an analysis search. (Which it can detect through Options["UCI_AnalyseMode"].) Would it me possible to make this dependent on Threads.main()->ponder in the main search thread?

And if there are cases where I want to abort thinking without getting a move (e.g. through 'force'), would it then be possible to revert it to a ponder search by using Threads.main()->ponder = true from xboard.cpp's process_command()? I see the opposite is done in case of a ponder hit, but I wonder if this could also be used in reverse, to shut up an ongoing timed search.

cjssh1002 commented 3 years ago

Human janggi pros understand the weaknesses of stockfish, so stockfish needs a Ponder for supplement. The human pro has already win the stockfish using the kakao janggi rule. https://youtu.be/s3W-VeZjHMU Ponder issue had already been requested several months ago. I testing with Muller every day for a month. And the information is open, and we can refer to it. How to contribute code to the GitHub project is not yet familiar for me, so if I learn to code a little more from Muller and learn how, I can contribute code to GitHub. cute-janggi supports Ponder in UCI, but it is still bug and limited in functionality. To date, Korean users are familiar with Winboard. And many features and improved in compatibility. That is why Muller's efforts are very much appreciated.

And Ianfab is also grateful. There have been janggi engine many updates lately, and many improvements have been made. I always appreciate it.

Bas-del,

As it is still an unofficial version, it will not be shared.

from. janggi-korea

ianfab commented 3 years ago

@HGMuller Sure, ponder search should not print a move in the end, which can e.g. be done like in https://github.com/ianfab/Fairy-Stockfish/commit/17500e677030ae7a70550820d94c78e2c3432905#diff-da923b7afa45cab7add143c4705b54142e46b2afe9a2627d5fa3b3474bdc8aecR304.

If you want to really abort a search, i.e., avoid printing a move as well as stopping the search, you can use a similar logic as is used for bughouse holding commands, where the search is aborted and restarted after updating the position: https://github.com/ianfab/Fairy-Stockfish/blob/7ce7b18d74539ca1e61e94b5924ff76d783b2203/src/xboard.cpp#L98-L100 The exchange is for an atomic get and set of the value so that we can be sure that either none or both of printing and applying the move is done. The need for an atomic operation will be gone when the search thread applies the move itself.

ianfab commented 3 years ago

I now have a first implementation that I consider a candidate to merge to master: https://github.com/ianfab/Fairy-Stockfish/commit/fc3fb5cf5e8eea24a7b40574613411612ea812e7. It also contains a somewhat bigger refactoring of the CECP handling in general that should hopefully improve the behavior in several scenarios. Since I found that solving the lift/highlight problem is somewhat annoying, instead of working around it I decided to try to take the lift command as an advantage by randomly updating the guess of the ponder move if the lift is from a different square than the current ponder move, because in that case the ponder move is most likely wrong anyway. This can of course lead to awkward ponder moves at times, but a random move with the correct piece should not be a worse guess than the best move with the wrong piece.

Edit: I changed it so that in case the piece of the ponder move is lifted, it replies the cached highlight command without restarting search.