lichess-bot-devs / lichess-bot

A bridge between Lichess bots and chess engines
GNU Affero General Public License v3.0
745 stars 440 forks source link

In FromPosition challenges: override selected UCI settings depending on the FEN #922

Closed Naphthalin closed 6 months ago

Naphthalin commented 6 months ago

Is your feature request related to a problem? Please describe. I'm the Lc0 dev behind the Leela Piece Odds bots on lichess https://lichess.org/player/bots, each accepting challenges for preselected starting positions from the appropriate side. I encountered the problem that some settings (especially Contempt, potentially others as well) are supposed to depend on the starting position and side to play for optimal performance, as e.g. playing knight odds as white and as black is quite different. However, the way lichess-bot works is that there is a static config.yml used once when starting up the bot, and challenges are received only later.

Describe the solution you'd like I already added a functionality to the "accept challenges" logic of is_supported in model.py which checks a stored file (plain text fenlist.txt, changing it to fenlist.json for the per-position options) whether the combination of position + color is accepted. I'd like to be able to use some syntax like fenlist.json to specify UCI options to override from the values set in config.yml when accepting a challenge.

Describe alternatives you've considered To some extent, having multiple bots (one for each level) already was an attempt to work around the issue, since it obviously allows one config per bot. There are other advantages so I'm not particularly unhappy with it, but there are some significant downsides to it, for example the handling of parallel challenges during times with higher demand. However, as mentioned above it turns out that we also want to use different settings for playing with white vs black, and splitting this up into even more bots doesn't make sense.

Additional context My code so far is still based on the pre-file-reorganization structure but obviously trivial to rebase, so if there is interest on your side to include specific handling of FromPosition challenges to check for a list of allowed starting FENs + color into lichess-bot, I'd be happy to contribute. Same goes with the per challenge override of settings, once I got it done -- but I believe I need some assistance in figuring out where exactly to connect the logic "found the position in the fenlist.json, this is the list of options to override" to actually overriding the options in the engine setup.

Thanks a lot for your help in advance! I really appreciate how easy it is in general to work with the lichess-bot API and provide interesting features directly to lichess users :)

Naphthalin commented 6 months ago

(updating this as I get deeper into the code)

Further comment on progress so far: After reading the code, it seems like I just need to find the appropriate place for calling engine.configure(override_options), but I haven't managed yet to find that place :)

Edit 1: Seems like the correct place is to change engine.play_move(..., engine_cfg, ...) to engine.play_move(..., uci_override | engine_cfg, ...) to merge the dicts.

Furthermore, since Game also contains initial_fen and is_white, I could either read the fenlist.json again here, or store uci_override as a property of Game.

Edit 2: engine_cfg is a Configuration, not a dict, so I can't simply merge them like this.

AttackingOrDefending commented 6 months ago

I've managed to make this work using the latest version of lichess-bot. You can see the code here. You can also pass the contents of fenlist.json as a parameter instead of reading them from the file every time.

There may be some problems with the way I'm updating the dict for a general use case, but I believe that the code will run correctly for that specific fenlist.json.

Naphthalin commented 6 months ago

Thanks a lot for your help! That looks already very promising.

There may be some problems with the way I'm updating the dict for a general use case, but I believe that the code will run correctly for that specific fenlist.json.

I think that the order of the option merging needs to be switched to make sure that the option is actually overridden if the key was present in the config, so changed_options |= options and then self.engine.configure(changed_options), but I'll have to try myself how it behaves. You were of course right, it's options | changed_options.

Naphthalin commented 6 months ago

I adapted your code to also handle the challenge rejection based on the fenlist.json and pushed it here. It certainly feels like there are a lot of places to change to get a game dependent config in.

I still have to test whether the option override works as intended if the key is already present in the config.yml. Tested, seems to work as intended :)

AttackingOrDefending commented 6 months ago

Indeed, generalizing the code to add it in lichess-bot will require quite a few changes.

Also, initialFen isn't a valid decline reason on lichess, so you should probably change this line to or self.decline_due_to(self.is_supported_fen(config), "generic")). You can see the only valid ones here.

The dicts also seem to be updating correctly.

MarkZH commented 6 months ago

Some comments:

  1. The game parameter in EngineWrapper.__init__() is not necessary since EngineWrapper does not use it. That should reduce the number of changed lines.
  2. I don't think the changes being discussed are fit to be included into lichess-bot since they are so specifically tailored to one user. As an alternative, we can create a couple of functions in a new file that users can edit to fit their purposes (like homemade.py). Here's a sketch of an idea:
    
    # game_specific_overrides.py

def is_supported_extra(game: model.Game) -> bool: return True

def game_options(game: model.Game) -> dict[str, Any]: return {}


These functions can then be called from wherever they are needed. For @Naphthalin, the bodies of `is_supported_extra()` and `game_options()` would come from the two outputs of [`extract_fenlist()`](https://github.com/Naphthalin/lichess-bot/blob/042c53613a35da3b225c135d2d1af1e6f530f446/lib/model.py#L15).
Naphthalin commented 6 months ago

@MarkZH I definitely agree that it is very specific. Your solution to provide a more general framework for this kind of use case (extra handling of challenges, game options override) looks great, and would help me massively in maintaining my fork for the Leela piece odds bots.

MarkZH commented 6 months ago

@Naphthalin: I created a PR to implement the idea in my last post. It needs testing, and I'm curious if it suits your purposes.