Implement engine parameters UCI_Variant and Threads

[rpdelaney]

Nice, thank you! I should be able to look into this more closely sometime next week.

A thought: Right now, chess-annotator determines if a move needs an annotation via calculations in needs_annotation() and winning_chances(). I built that based on my understanding of how lichess does it.

But I've noticed that stockfish's evaluations of crazyhouse games tend to swing a lot more than they do for chess. I don't know if that's because crazyhouse positions are inherently "sharper" than they are in chess, or if there's some other property of stockfish-zh that I don't understand that makes it that way.

Anyway, I'm anticipating that unless something changes, chess-annotator will be much more verbose about annotating crazyhouse games. (I'm not very skilled at other variants, so I know much less about them, but I imagine something similar might obtain in antichess.)

I'd be interested in hearing your thoughts on that. Do you think needs_annotation() should use a different value from 7.5 when working with variant games? I was also considering adding a command-line switch to enable the user to set the threshold directly, or choose from a few pre-set verbosity profiles.

[ddugovic]

I'm unsure why Stockfish's evaluations of crazyhouse games fluctuate so much, but considering it just won the Computer Crazyhouse Championship it must be doing something right!

I do like your idea of parameterizing (command line switching) "winning chances delta" thresholds for each category of mistake (blunder, mistake, inaccuracy, ...?); maybe even allowing variant-specific thresholds?

[rpdelaney]

Congrats!

It seems we're on the same page. I'll look into adding options like that after this PR is merged.

On that topic:

1) Maybe we should skip the opening classification if it's a variant game. 2) I just tested it on this game: https://lichess.org/bIqjcbbb but it's throwing lots of exceptions about illegal uci moves. Were you able to get it to execute successfully on a variant game?

[rpdelaney]

Ah, I wasn't using the variant stockfish. Rookie mistake. Now it gets further, but it still died:

An unhandled exception occurred: <class 'KeyError'>
Traceback (most recent call last):
  File "/usr/lib/python3.6/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/lib/python3.6/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/home/ryan/src/games/annotator/annotator/__main__.py", line 703, in <module>
    main()
  File "/home/ryan/src/games/annotator/annotator/__main__.py", line 693, in main
    raise e
  File "/home/ryan/src/games/annotator/annotator/__main__.py", line 687, in main
    analyzed_game = analyze_game(game, args.gametime, args.engine, args.threads)
  File "/home/ryan/src/games/annotator/annotator/__main__.py", line 632, in analyze_game
    add_annotation(node, judgment)
  File "/home/ryan/src/games/annotator/annotator/__main__.py", line 286, in add_annotation
    variation = truncate_pv(node.board(), judgment["pv"])
  File "/home/ryan/src/games/annotator/annotator/__main__.py", line 267, in truncate_pv
    board.push(move)
  File "/home/ryan/.local/lib/python3.6/site-packages/chess/variant.py", line 651, in push
    self.pockets[self.turn].remove(move.drop)
  File "/home/ryan/.local/lib/python3.6/site-packages/chess/variant.py", line 604, in remove
    self.pieces[pt] -= 1
KeyError: 1

I'm not sure if this is a bug with chess-annotator or python-chess. I'll try to dig into it with a debugger when I get a chance.

[ddugovic]

I didn't attempt to perform opening classification for variants, although I believe a wealth of opening theory exists for crazyhouse, atomic, and antichess. That seems like a nice-to-have feature (although I'm curious why the KeyError occurs).

And thanks! I'm looking forward to letting Lichess users know about this game annotator; maybe in the future I'll find a way to integrate it with Lichess studies

[rpdelaney]

I reached out to Niklas to see if he has any feedback on that KeyError exception.

Some more test results:

1. 3-check seems to work: http://dpaste.com/3V308TM
2. chess960 throws the error not in UCI_Chess960 mode but position has non-standard castling rights and eventually crashes with an EngineTerminatedException. I think UCI_Chess960 needs to be set separately, which means we'll need additional variant detection logic when setting the UCI options: https://python-chess.readthedocs.io/en/latest/variant.html#uci
3. antichess seems to work: http://dpaste.com/0F39SHB Note that in python-chess there are supported variants for "suicide" and "giveaway" but there is no "antichess". I'm not sure which corresponds to which. It might be a good idea to test this with more antichess games.
4. atomic seems to work: http://dpaste.com/3YRHBD2
5. racing kings seems to work: http://dpaste.com/19ZNWH5
6. horde seems to work: http://dpaste.com/0VD43P5

I think that's everything?

I don't have a pgn viewer that can handle these variants, which complicates testing since it's a bit of a pain to verify that the engine variations are appropriate to the variant type. Lichess can import pgn of variant games, but it strips the annotations and variations, diminishing its value.

[rpdelaney]

@ddugovic Do you know how I can push commits to this PR? Or would I have to be a maintainer of your fork?

[ddugovic]

@rpdelaney I think it is possible even without being a maintainer (as I have "Allow edits from [upstream] maintainers" selected).

I assume this is how it works:

git remote add ddugovic git@github.com:ddugovic/python-chess-annotator.git
git checkout ddugovic/uci_variant
...
git push ddugovic uci_variant

[rpdelaney]

For my future reference, since I mucked this up a few times, I had to do this:

git remote add ddugovic git@github.com:ddugovic/python-chess-annotator.git
git fetch --all
git checkout ddugovic/uci_variant
...
git push ddugovic HEAD:uci_variant

Chess 960 seems to work with the last commit. Also, the KeyError I observed above is the result of a bug in my code that is now fixed in the master branch. I'm going to do a little bit more testing but this should be ready to merge in soon.

[rpdelaney]

Thanks again!