Progress on Loki

[Sven]

Another way could be to do the reverse: the main thread processes all (UCI) input and starts one ore more search threads when a search has been requested, but continues to process input while a search is running and signals the search thread(s) to stop if that has been requested via (UCI) protocol. Checking for timeout, though, should be checked by the search thread(s) (for instance every N nodes). So a search thread either terminates on its own due to timeout or finishing its work (in case of fixed-depth search or similar), or on request by the main thread.

Looks more natural to me ... I wrote (UCI) in brackets since it might as well be the WB protocol. And my WB engine Jumbo uses the thread model described above. And I think many other engines do it in a similar way.

[niel5946]

Another way could be to do the reverse: the main thread processes all (UCI) input and starts one ore more search threads when a search has been requested, but continues to process input while a search is running and signals the search thread(s) to stop if that has been requested via (UCI) protocol. Checking for timeout, though, should be checked by the search thread(s) (for instance every N nodes). So a search thread either terminates on its own due to timeout or finishing its work (in case of fixed-depth search or similar), or on request by the main thread.

Looks more natural to me ... I wrote (UCI) in brackets since it might as well be the WB protocol. And my WB engine Jumbo uses the thread model described above. And I think many other engines do it in a similar way.

I hadn't actually given it any thought to do it the other way around... I see your point about it being more intuitive/natural to have the main thread handle all UCI-related issues. I just think it will take a lot of work to implement it this way in Loki. The reason being that I have already used a "program-structure" where the search is responsible for checking for input.
Thankfully, my UCI rework is in another branch from main, so I can experiment with it all I want without affecting the real Loki...

I am thinking of doing it this way:

1. When getting a "go" command, set the isStop = false, and start up a new thread that runs the search function.
2. While that thread is searching, look for input. Only handle this input if we recieve "stop" or "quit".
3. If the search times out by it self, it will set this isStop = true which will then signal to the main thread, that it can stop listening for search-related input.
4. Otherwise, if the search should stop due to request from the GUI, we'll set the isStop = true, end the listening loop, and wait for the search thread to join.

Am I understanding this correctly, or have I missed something?

Thanks for the help btw

[Sven]

I suggest to do it as simple as possible, so I would not care about "broken" cases where a GUI sends another "go" while a search is running.

And regarding branches and the amount of work: why not starting another branch from main, possibly cherry-picking some useful changes from your current "rework" branch?

[niel5946]

That makes sense. That is why I am thinking that I'll just look for a "stop" or a "quit", and if there are none, just reset the input and wait for a new one.

It is a good idea to cherry-pick the good parts of my current UCI-rework, considering that all UCI stuff except for the search-listening part is done.

[niel5946]

UCI rework loses elo???
Yes, you heard it right. Something as simple - or at least i thought - as a communication protocol is making Loki lose elo:

Code: Select all

Score of Loki_dev vs Loki 3.0.0 64-bit: 112 - 161 - 127 [0.439]
...      Loki_dev playing White: 54 - 81 - 65  [0.432] 200
...      Loki_dev playing Black: 58 - 80 - 62  [0.445] 200
...      White vs Black: 134 - 139 - 127  [0.494] 400
Elo difference: -42.8 +/- 28.3, LOS: 0.2 %, DrawRatio: 31.8 %
400 of 400 games finished.

I know the number of games doesn't allow for exact numbers, but it is certainly worse than Loki 3.0.
I made the UCI implementation work, and changed the way Loki listens for input from the GUI. Thanks to Sven Schüle, I managed to get it working by making the main-thread listen for input while a separate thread handles the searching. This is the current implementation which is run after parsing the "go" command:

Code: Select all

	// Step 4. Finally, run the search.
	
	Search::isStop = false;
	std::thread searcher(Search::runSearch, std::ref(pos), std::ref(info), num_threads);

	std::string interrupt = "";

	while (!Search::isStop.load()) {

		// Check if there's input waiting
		if (InputWaiting()) {
			std::getline(std::cin, interrupt);

			if (interrupt.find("stop") != std::string::npos) {
				Search::isStop = true;
				break;
			}

			else if (interrupt.find("quit") != std::string::npos) {
				Search::isStop = true;
				info->quit = true;

				break;
			}
		}
	}

	searcher.join();

This simply runs the search in a new thread, listens to see if there's any GUI input. If there is, it sets a stop flag if the command is either "stop" or "quit", and if there isn't it doesn't do anything until the search times out by itself. Now the only function used by Vice is the InputWaiting, which I am satisfied with. Thus, if I can fix the elo loss, I can get on to working on more fun stuff and be done with my UCI-rework.
In the search function, I call the following method:

Code: Select all

void check_stopped_search(SearchThread_t* ss) {
	// Step 1. Check for timed out search and set isStop = true if we're the main thread
	if (ss->info->timeset && getTimeMs() >= ss->info->stoptime) {
		ss->info->stopped = true;

		if (ss->thread_id == 0) {
			Search::isStop = true;
		}
	}

	// Step 2. Check if we've been told to stop
	if (Search::isStop.load()) {
		ss->info->stopped = true;
	}
}

Which checks for timeout and interruption by the GUI, as set by the std::atomic<bool> isStop flag.

I can only seem to think of three things that might explain this - rather big - elo loss:
1. Separate threads are slower than the main thread and thus, NPS is lowered which in turn causes higher time-to-depth numbers.
2. It takes too long to start up a thread for searching and join it again, which makes this version of Loki lose due to time trouble. By time trouble I do not neccessarily mean lose on time, but that it has less time to search and thus doesn't see as deep.
3. It takes too long to access the std::atomic<bool> variable. Before, the main thread (in search) would not constantly access it.

I'll firstly try to balance out the third issue by only checking every 4096 nodes instead of every 2047.

I/O is probably the worst thing about C++. It is way more cumbersome than in higher-level languages...

[mvanthoor]

UCI rework update: I have finished re-writing the UCI implementation to C++, but the CheckUp function, called in search, is still from Vice. I don't really know how to implement that optimally. I will try using a thread to listen for input from the GUI, but I fear it will be slow..

All in all, I would say the development of Loki is going great

Good I have a function that checks the board and game state, but it only runs in debug mode. It degrades engine performance by 75%.

My suggestion would (still) be to not try and implement more than one function at the time. Implement LMR, and try to get the most ELO-gain possible. Then implement LMP, and do the same there. Then implement staged move generation. In the end, when you know all three functions are working perfectly, you can fiddle with the values of LMR and LMP to try and increase playing strength even more.

[Sven]

@Niels: are you sure you did not change anything else than your UCI code? On your branch I also see some little changes regarding the TT size but I have been unable to see whether these were only formal changes ...

[niel5946]

My suggestion would (still) be to not try and implement more than one function at the time. Implement LMR, and try to get the most ELO-gain possible. Then implement LMP, and do the same there. Then implement staged move generation. In the end, when you know all three functions are working perfectly, you can fiddle with the values of LMR and LMP to try and increase playing strength even more.

You are right regarding working on one thing at a time, and I also think I am doing that. Whenever I want to work on something else, I create a branch from master and then make the changes. If they show an increase, I merge them with master. This way, I make sure that no changes that I make at the same time cancel each other out.
The reason I haven't worked on LMR/LMP/staged movegen lately has been that I wanted to get the UCI-rework out of the way. I/O isn't really my cup of tea, so I might as well get it done now.

@Niels: are you sure you did not change anything else than your UCI code? On your branch I also see some little changes regarding the TT size but I have been unable to see whether these were only formal changes ...

Where do you see that? I am just looking through the changes between master and the UCI-rework branch, and I can't spot anything about the TT. Other than a new way of parsing the setoption command ofc.

[Sven]

It seems the diff was too hard to understand for me ... I thought there were changes in uci.cpp in commit f7c9812 but now it appears to me that you only moved around some code regarding the "Hash" option without changing semantics.

My problem may also be related to being used to see smaller commits ...

[niel5946]

My problem may also be related to being used to see smaller commits ...

I'll take a note about that. Usually I make all changes that I can think of before making a commit in order to not have non-functioning - also partly functioning for that matter - code in the repo. But you're probably right; smaller commits are better