Progress on Blunder

[algerbrex]

After 768 games :

Code: Select all

2021/12/16 10:48:56 The position the best move was sent in is: 
8 | r . . . . r k . 
7 | i i i n b i i i 
6 | . . . . . . . . 
5 | . . . I . . . . 
4 | . . I I . . b . 
3 | I . . . K N . . 
2 | . I . . B . I I 
1 | R N . . . . . R 
   ----------------
    a b c d e f g h 

turn: white
castling rights: 
en passant: none
fen: r4rk1/pppnbppp/8/3P4/2PP2b1/P3KN2/1P2B1PP/RN5R w - - 1 17
zobrist hash: 0xabcf9ff4b9c74143
rule 50: 1
game ply: 34

2021/12/16 10:48:56 command ["quit\n"] was sent to the engine

2021/12/16 10:48:56 command ["quit\n"] was sent to the engine

2021/12/16 10:48:56 command ["quit\n"] was sent to the engine

2021/12/16 10:48:56 command ["quit\n"] was sent to the engine

2021/12/16 10:48:56 command ["quit\n"] was sent to the engine

If you want the whole logfile I will send you a download link in PM.

Hopefully it gives you a clue!

Good luck.

Hey Ed, don't worry about sending that full log file, never mind. I see what the problem is. If Blunder is being sent the command:

Code: Select all

position fen r4rk1/pppnbppp/8/3P4/2PP2b1/P3KN2/1P2B1PP/RN5R w - - 1 1 moves

Then I would expect it to crash! It's expecting one or more moves after the "moves" subcommand is sent. I wasn't aware that the suffix could be sent without any moves. This is a pretty easy fix for me, just check if there are actually any moves after "moves" before I assume there are.

I was able to reproduce the exact error you sent me by sending the above command into Blunder:

Code: Select all

panic: runtime error: index out of range [252] with length 64

goroutine 1 [running]:
blunder/engine.MoveFromCoord(0xc000016136, {0xc000016136, 0x1})
        C:/Users/deanm/Desktop/blunder/engine/move.go:100 +0x1db
blunder/engine.(*UCIInterface).positionCommandResponse(0xc0001e6000, {0xc0000160f0, 0x4c})
        C:/Users/deanm/Desktop/blunder/engine/uci.go:84 +0x3ab
blunder/engine.(*UCIInterface).UCILoop(0xc0001e6000)
        C:/Users/deanm/Desktop/blunder/engine/uci.go:262 +0x50f
blunder/engine.RunCommLoop()
        C:/Users/deanm/Desktop/blunder/engine/comm.go:124 +0x6d4
main.main()
        C:/Users/deanm/Desktop/blunder/blunder/main.go:6 +0x17

So I'm pretty confident this is the same issue.

[Rebel]

Cool...

[mvanthoor]

Hey Ed, don't worry about sending that full log file, never mind. I see what the problem is. If Blunder is being sent the command:

Code: Select all

position fen r4rk1/pppnbppp/8/3P4/2PP2b1/P3KN2/1P2B1PP/RN5R w - - 1 1 moves

Then I would expect it to crash! It's expecting one or more moves after the "moves" subcommand is sent. I wasn't aware that the suffix could be sent without any moves. This is a pretty easy fix for me, just check if there are actually any moves after "moves" before I assume there are.

This is an edge-case I didn't even consider in Rustic, so I just quickly tested it:

Code: Select all

position fen r4rk1/pppnbppp/8/3P4/2PP2b1/P3KN2/1P2B1PP/RN5R w - - 1 1 moves

I expected it to work because of command parsing works in the engine.

Rustic doesn't really parse commands. It splits on space into a vector and iterates over it. When encountering "fen", it just starts to rebuild the FEN-string again by concatenating everything. On encountering "moves", it creates a vector of moves. All that is wrapped into an enum variant and sent to the engine. It reads the FEN and performs the moves in the move list... which can be none, so it just does nothing. When encountering an illegal move in the move list it outputs an error and stops executing moves.

When iterating through the vector, it ignores everything that is not a token. If a token is encountered, it is assumed that everything after that token belongs to it, until the next token is encountered. If something is after a token that shouldn't be there (a string instead of a number, etc...) it is either empty or 0 after the parsing. If the mistake is critical (like not being able to load a FEN), the operation is aborted.

This also takes care of different parameter orders. I often see engines that expect a GUI to send "wtime / winc / btime / binc" in that order, but that doesn't have to be the case. This way of parsing therefore works with tokens out of order. In Rustic, this input works (it crashes many other engines):

Code: Select all

position moves c2c4 e7e6 fen rnbqkbnr/ppp1pppp/8/3p4/3P4/8/PPP1PPPP/RNBQKBNR w KQkq - 0 1

So this case is handled in Rustic by the sheer luck (?) of how parsing was implemented

I hope this is useful for you or someone else in the future. If a programming language is used that can easily explode strings into vectors and then iterate over then, this is a very easy way to get very fault-tolerant parsing. It _isn't_ the fastest way though, but compared to the move generator, search loop and evaluation function, everything else doesn't really matter.

(If someone finds some combination of tokens and data in a UCI-command that can crash Rustic, let me know. I'll credit you for finding it. I'm not going to give you €2.56 for every bug though, like Donald Knuth did at some point for mistakes in his books )

[algerbrex]

This is an edge-case I didn't even consider in Rustic, so I just quickly tested it:

Code: Select all

position fen r4rk1/pppnbppp/8/3P4/2PP2b1/P3KN2/1P2B1PP/RN5R w - - 1 1 moves

I expected it to work because of command parsing works in the engine.

As didn't I I thought it'd be safe to assume "moves" would never be sent without...well...moves. Guess I was wrong. I was curious to see how another engine handled this input Blunder choked on and MinimalChess 0.6 also handles it without any problem, as I'm sure many other engines do as well.

Rustic doesn't really parse commands. It splits on space into a vector and iterates over it. When encountering "fen", it just starts to rebuild the FEN-string again by concatenating everything. On encountering "moves", it creates a vector of moves. All that is wrapped into an enum variant and sent to the engine. It reads the FEN and performs the moves in the move list... which can be none, so it just does nothing. When encountering an illegal move in the move list it outputs an error and stops executing moves.

When iterating through the vector, it ignores everything that is not a token. If a token is encountered, it is assumed that everything after that token belongs to it, until the next token is encountered. If something is after a token that shouldn't be there (a string instead of a number, etc...) it is either empty or 0 after the parsing. If the mistake is critical (like not being able to load a FEN), the operation is aborted.

That sounds like more of a robust design than what I had I did the same design when implementing the "go" command since I did expect those to be sent in any order, but it didn't occur to me I should also use the same design the "position" command.

I hope this is useful for you or someone else in the future. If a programming language is used that can easily explode strings into vectors and then iterate over then, this is a very easy way to get very fault-tolerant parsing. It _isn't_ the fastest way though, but compared to the move generator, search loop and evaluation function, everything else doesn't really matter.

Yup. For Go this would be

Code: Select all

strings.Fields()

Which I use quite frequently throughout parsing input into Blunder. So not sure why it didn't occur to me to use it again here.

(If someone finds some combination of tokens and data in a UCI-command that can crash Rustic, let me know. I'll credit you for finding it. I'm not going to give you €2.56 for every bug though, like Donald Knuth did at some point for mistakes in his books )

Noted

[Ras]

I'm replacing tabs with spaces already at an early input processing stage, and multiple spaces with one space. If the last registered character before line end is a space, that gets discarded. Hence, the move list is empty, which is treated like the end of the normal move list.

For the position command, the order is actually specified in UCI, so I treat out of order parameters as an error and reject the command with the error message illegal position: FEN / startpos missing (no crash, of course). Interestingly, the go command does not specify the order, but what most engines including Shredder itself get wrong is if the searchmoves list is not the last parameter. Any subsequent parameters are just ignored.

Btw., also test your engines with as many parallel single-threaded games as you have logical cores, and then using short time controls like 10s per game. There are quite a few engines suffering from a race condition between the output and the input thread which makes the engine either hang or do illegal moves.

[mvanthoor]

I'm replacing tabs with spaces already at an early input processing stage, and multiple spaces with one space. If the last registered character before line end is a space, that gets discarded. Hence, the move list is empty, which is treated like the end of the normal move list.

That's why I split on white space, which includes tabs, spaces, and whatever else counts as white space according to Rust.

For the position command, the order is actually specified in UCI, so I treat out of order parameters as an error and reject the command with the error message illegal position: FEN / startpos missing (no crash, of course). Interestingly, the go command does not specify the order, but what most engines including Shredder itself get wrong is if the searchmoves list is not the last parameter. Any subsequent parameters are just ignored.

*shrug* In Rustic, as long as all the information is in the string, it'll work. Parameter order is not a problem. Even if there are too many parameters is OK. If a GUI would send "wtime 5000 6000 7000 winc 1000", the base time would be 7000. (I could change it to start skipping stuff as soon as the first valid value was found, if I wanted to.)

Btw., also test your engines with as many parallel single-threaded games as you have logical cores, and then using short time controls like 10s per game. There are quite a few engines suffering from a race condition between the output and the input thread which makes the engine either hang or do illegal moves.

My engine has both an input and an output thread The main engine thread does do receive and send though. Never had a problem, but I never test with logical cores. That was considered not a good idea in the past. I don't know if that has changed.

[Ras]

Never had a problem, but I never test with logical cores.

That race condition typically manifests only with all logical cores loaded with games. I'd be curious whether Rustic can cope with that without taking extra measures in the code. My design actually solves that problem at its core - I just don't discard messages. Instead, they go into a ring buffer which acts like a queue, except isready/start/stop which are processed immediately, also during search.

[mvanthoor]

That race condition typically manifests only with all logical cores loaded with games. I'd be curious whether Rustic can cope with that without taking extra measures in the code. My design actually solves that problem at its core - I just don't discard messages. Instead, they go into a ring buffer which acts like a queue, except isready/start/stop which are processed immediately, also during search.

I'll run a test tonight; but AFAIK, this should not be a problem. Rustic uses the Crossbeam crate as it provides unbounded channels between threads. It should not lose any messages. (When I implemented this, std-channels only supported bounded channels, and Crossbeam was faster. I don't know if both are still true today.)

[Ras]

It should not lose any messages.

The issue isn't actually losing messages, it's taking the UCI spec literally and consciously discarding messages like position/go while searching. If the "search ended" state does not propagate in time to the input thread, then messages are erroneously discarded. This is a risky design, but it can be duct taped via resetting the discard flag and applying a memory barrier right before sending the bestmove, and also another memory barrier in the input thread before reading the discard flag.

[mvanthoor]

It should not lose any messages.

The issue isn't actually losing messages, it's taking the UCI spec literally and consciously discarding messages like position/go while searching. If the "search ended" state does not propagate in time to the input thread, then messages are erroneously discarded. This is a risky design, but it can be duct taped via resetting the discard flag and applying a memory barrier right before sending the bestmove, and also another memory barrier in the input thread before reading the discard flag.

In Rustic everything you could want is possible (or should be possible) as long as it is somehow logical. You can set up a new position during a search and when you stop and then start the search again, the engine will search the new position. I even intend to change it so it works the same way as it does in the XBoard interface: when setting up a new position during search the engine stops the search, sets up the position and then restarts.

Sending "go" while already searching is not discarded, but because the search is already "go", the command has no effect. You can send "go", or "go infinite"; whatever you want. Parameter order doesn't really matter. Everything that isn't known is discarded. If you give a parameter 5 values where you should have given it one, the last one is the one that counts. You don't even have to send "uci" or "ucinewgame" after the engine starts. Everything is ready to receive any UCI command as soon as the engine starts. Same with XBoard, where possible. It'll either work, or it'll return an error, but not crash.

Rustic doesn't really follow the UCI specification very strictly. It accepts most things in most situations and in most orders. It either will work, or return an error or info-string such as "You can't do this right now because ...". (If it doesn't, I missed an edge-case somewhere.) I dislike it when engines need PERFECT adherence to the UCI or XBoard protocol and crash when there's a mistake somewhere. I often use an engine to quickly have a position analyzed on the command-line without even having to start a GUI.

I did run your test: 8 games concurrently, also using the logical cores, 1000 games. No forfeits, disconnects, or stalls. I ran the current development version (3.38.100) against the version that ran in ZaTour (3.1.112), which normally scores +40 to +50; this time it scored +41 Elo. All seems good.

But I digress... this is Blunder's progress thread.