This game, Goldfish gave away a mate to a draw by repetition. Running version was v1.7

As you suggested <a class="user-mention notranslate" data-hovercard-type="user" data-h

This is quite common apparently. Another one: <a href="https://lichess.org/ROTrEoyB/"

This occurred again recently: check, check, check... <a href="https://lichess.org/

Goldfish repeated position in a mate in 3 about goldfish HOT 14 CLOSED

bsamseth commented on July 18, 2024

Goldfish repeated position in a mate in 3

from goldfish.

Comments (14)

bsamseth commented on July 18, 2024 2

Yep, did something similar myself.

It looks somewhat good for a while, and

info depth 8 seldepth 10 nodes 18413 time 24 nps 0 score mate 4 pv b2e2 e1d1 f4d5 f3f4 d5c3 d1c1

is just a single move away from the full mating sequence (one of them anyway).

The main issue is how it suddenly believes that it has a mate in 2. Secondary I'm pretty sure that once a mate is reported, a deeper search shouldn't be able find a way out (i.e. give a non-mate score). But this happens a couple of places.

These might be related issues, or they might be two bugs. In any case they are quite severe bugs. I'm considering a solid refactoring of the search logic, as it has become a bit of a mess and hard to think through.

Similarly, I really want the search to be incorporated into the tests. Its been outside simply due to lazyness, and the fact that search really only works well when used via the UCI interface. So testing the results would either require some refactoring to properly allow search to be called from tests, or something like python-chess could be used to parse the output properly.

This looks like it might take some time to properly work out...

@ddugovic Thanks for helping me look into this. If you want to have a crack at it that would be much appreciated. I'll try and make some time for this between everything else that I should be doing!

from goldfish.

bsamseth commented on July 18, 2024 1

Oo, Goldfish... I'll track this down when I have the time, but it'll probably be a little while.

from goldfish.

bsamseth commented on July 18, 2024 1

As you suggested @ddugovic, the issue was with quiescent. I was storing quiescent results in the ttable, which turns out to be a bad idea. Thought first that just marking them with depth zero would be enough, but no. Thanks for the suggestion, it really sped things up!

Seeing as this was a pure bug, I've just merged this straight away and updated the lichess-bot server to run this version. I'll set of a tournament to see how this affected play. Benchmark scores took a heavy hit with this change, but it also might be that the bug is common enough that this is still an improvement in strength. In any case, I prefer that Goldfish not miss such things, even if that means it will play slightly worse.

from goldfish.

ddugovic commented on July 18, 2024 1

Excellent! Glad to help and best of luck with eventual performance tuning (I've never successfully integrated quiescent search with TT, although perhaps there's somehow a way to do it), and thanks again for sharing this open-source project!

from goldfish.

bsamseth commented on July 18, 2024 1

Well, turns out it wasn't quite that simple. It happened again, here https://lichess.org/hEQgRWnK. I'll have to do some more testing. Reopening the issue.

from goldfish.

bsamseth commented on July 18, 2024 1

This is quite common apparently. Another one: https://lichess.org/ROTrEoyB/.

I've tracked this down partly. The bug started with v1.7.4, which was the introduction of mate distance pruning.

Goldfish/src/search.cpp

Lines 544 to 554 in 6ed378c

    
           // Mate distance pruning: 
        
           // Even if we mate at the next move our score 
        
           // would be at best CHECKMATE - ply, but if alpha is already bigger because 
        
           // a shorter mate was found upward in the tree then there is no need to search 
        
           // because we will never beat the current alpha. Same logic but with reversed 
        
           // signs applies also in the opposite condition of being mated instead of giving 
        
           // mate. In this case return a fail-high score. 
        
           alpha = std::max(-Value::CHECKMATE + ply, alpha); 
        
           beta = std::min(Value::CHECKMATE - (ply + 1), beta); 
        
           if (alpha >= beta) 
        
               return alpha;

This implementation is believed to be correct. Taken more or less verbatim from Stockfish, and seen elsewhere in close to identical forms.

Somehow this pruning method, which is always theoretically safe to do, results in "fake" mates being reported (faster than possible mates). Removing MDP, i.e. reverting to version v1.7.3 seems to fix the issue. At the current version, there are still issues after removing MDP though, so some more digging to do.

Even if removing MDP resolves this issue, it's not really a good fix, as this should be a safe thing to do. It points to some underlying bug or vulnerability which is triggered by this.

from goldfish.

ddugovic commented on July 18, 2024 1

I fully agree with all your analyses and conclusions & recommendations.

That all said (especially including other things you should be doing) I suppose I recommend ignoring this issue until #26 is resolved? One Stockfish refactor which particularly impressed me started with official-stockfish/Stockfish@444d99b#diff-5ff237612aa333bf47ea78c51ed743a8 - being able to run "go bench", "go perft", etc. from any position (as well as from the command line using FEN files) helped me quickly script a variety of parameterized tests for debugging my crazyhouse-SF fork. While the same might be infeasible for Goldfish, anything which eases scripting tests with parameters will probably considerably help with debugging complex problems.

from goldfish.