Stockfish: Illegal move and lost game in Losers

[lantonov]

[Event "ICS rated losers match"]
[Site "freechess.org"]
[Date "2017.02.04"]
[Round "-"]
[White "zepler"]
[Black "FishTest"]
[Result "1-0"]
[WhiteElo "1901"]
[BlackElo "2166"]
[TimeControl "120+10"]
[Variant "losers"]
[Annotator "1... +0.50"]

    1. a4 c6 {+0.50/31 52} 2. a5 Qxa5 {+4.04/3} 3. Rxa5 g6 {+0.00/30 20} 4.
     Rxa7 Rxa7 {+0.00/32 0.2} 5. c3 Ra4 {-0.30/31 2.6} 6. Qxa4 e6 {+0.00/31 12}
    7. Qxc6 dxc6 {-0.42/31 11} 8. b4 Bxb4 {+1.63/1} 9. cxb4 b5 {-0.38/28 22}
    10. d4 Ne7 {+0.00/26 18} 11. Nd2 Kd8 {+0.00/29 51} 12. Kd1 Kc7 {+1.64/25 7}
    13. Kc2 Rg8 {+1.29/29 58} 14. Kd3 Na6 {+19.35/24 6} 15. e4 Nxb4+ {+23.70/1}
    16. Ke2 e5 {+1000.07/100 8} 17. dxe5 Bh3 {+1000.06/116} 18. Nxh3 f6
    {+1000.05/127 0.7} 19. exf6 Ra8 {+1000.04/127} 20. fxe7 Ra3 {+1000.03/127}
    21. Bxa3 Kd6 {+1000.02/127} 22. Bxb4+
    {FishTest resigns} 1-0

The last move by my opponent 22. Bxb4 was labeled illegal by the program (legality checking on) and SF resigned in a won position (FishTest is SF)

[ddugovic]

Will fix:

setoption name UCI_Variant value losers
position fen 8/4P2p/2pk2p1/1p6/1n2P3/B6N/3NKPPP/5B1R w - - 1 22
go depth 1
info depth 1 seldepth 1 multipv 1 score mate -1 nodes 1 nps 500 tbhits 0 time 2 pv a3b4
bestmove a3b4

setoption name UCI_Variant value losers
position fen 8/4P2p/2pk2p1/1p6/1n2P3/B6N/3NKPPP/5B1R w - - 1 22 moves a3b4
go depth 20
info depth 0 score mate 0
bestmove (none)

[ianfab]

Thanks a lot for the report. In command line usage it does recognize 22. Bxb4+ to be the only legal move, but it claims that the position after the move is mate. So it could be a problem with check evasions, probably caused by the fact that the black king can take the pawn on e7, but would still be in check. So this might have to be fixed in can_capture_losers.

[ddugovic]

@ianfab is probably correct since in this similar position Black easily finds Kxd7. Likely Stockfish isn't considering the x-ray defense of the e7-pawn in your game (or rather, that the move generator isn't properly generating moves but probably is correctly validating them):

setoption name UCI_Variant value losers
position fen 8/3P3p/2pk2p1/1p6/1n2P3/B6N/3NKPPP/5B1R w - - 1 22 moves a3b4
go depth 20
info depth 1 seldepth 1 multipv 1 score cp -187 nodes 1 nps 500 tbhits 0 time 2 pv d6d7

[ianfab]

Should be fixed by #240. The king on d6 blocked the bishop on b4 from covering e7, so it thought that Kxe7 is a valid capture. It was then detected to be an illegal move by is_legal, so there was no legal move left.

[lantonov]

Wow, this game was played half an hour ago and the fix is already in pull request. Outstanding response!

[ianfab]

@lantonov Thanks again for the report. Playing against other engines and humans perhaps is a good way to find such issues, since in selfplay Stockfish might avoid to get into such a position.

[ianfab]

@lantonov Since you apparently play rated games on FICS, can you estimate Stockfish's playing strength in losers compared with other engines and humans?

[lantonov]

               rating     RD      win    loss    draw   total   best
     Blitz      3123     65.9     792      64     302    1158   3123 (04-Feb-2017)
     Standard   3053     75.0     252      20      85     357   3054 (03-Feb-2017)
     Lightning  3303     68.7     344      17      98     459   3315 (17-Dec-2016)
     Wild       2782     63.7     421       9      78     508   2926 (04-Apr-2014)
     Crazyhouse 1984    205.7       2       0       0       2
     Suicide    2987     46.9      62      13       5      80   3000 (01-Feb-2017)
     Losers     2106    147.1       5       1       0       6

Recently, my computer account in FICS was extended for variants. Here are my ratings. The last 3 are with this engine.

[ddugovic]

Submitted STC (and if successful, LTC) test with bounds [-6, 2]. In hindsight I should have used "Fix" in my commit message:
http://35.161.250.236:6543/tests/view/5895e9286e23db014ac09f7f

[ianfab]

@ddugovic Your patch looks good to me, but I think it should be tested against my fix, since these are the two alternatives.

Furthermore, it would be great if you could split your patch (and maybe even the testing) into two parts: One, where you rewrite the move generator but keep can_capture_losers to use it in search.cpp. This commit should not change bench (losers/all) and perft, since it is supposed to be no functional change. In a second commit, you could then remove can_capture_losers and replace it in the search.

Edit: And I would prefer to explicitly copy the FENs in benchmark.cpp, since this would allow to modify them for losers chess, e.g. by adding the above position.

[ddugovic]

That seems reasonable. I will:

1. Merge your PR
2. Add tests to benchmark.cpp
3. Rewrite the move generator and keep can_capture_losers and test bench and perft
4. Replace can_capture_losers and test my entire patch as a simplification with bounds [-6, 2].

[ianfab]

Excellent. With this procedure it will be easy to understand and validate the changes.

[ddugovic]

Indeed. At the moment my branch is generating incorrect perft counts, so my patch is not (yet) ready.