Ser/De configurations and Batch Solve

[bkushigian]

EDIT: I've added batch_solve functionality to this PR. I should probably disentangle it, but I'm developing it in parallel to serialization so I'm just bundling them.

Overview

We should be able to run the same configuration for many different boards/ranges/etc. For instance, the same tree should be used for many 100bb cash game SPR scenarios, only changing ranges and pot/stack sizes/rake/etc.

This requires saving/loading configurations. Performing this should be easy for the user.

This PR uses serde to serialize/deserialize CardConfig and TreeConfig. To facilitate this I had to do some refactors so that some checking logic (e.g., that bet sizings vs raise sizings were well formed) is in the correct place.

Tasks

• Make "flop" optional in config?
  • This might make some things weird, and we would need to think about how we can ensure that a CardConfig always has a flop. I think this should be doable but will require poking around and thinking about different ways that things can be used.
• Move bet_size_from_str to TryFrom<&str>
• batch_solve: Make compression_level, verbose, and silent options
• batch_solve: Handle cases where config/boards are already in the directory (say after a partial batch solve)
• update CardConfig::with_cards docs to reflect that this is not mutable, and that we check to ensure that the same number of cards exist (...also, @JacobVanGeffen might argue that we allow any updates to cards and not restrict then number of cards allowed)
• Include added/removed lines in config, probably just at the top level of the config json

The Configurations

The following data needs to be present when running a solve:

• CardConfig: contains

  • ranges (OOP and IP)
  • flop cards
  • turn card
  • river card

• TreeConfig: used to build an ActionTree, this contains:

  • initial_state: BoardState
  • starting_pot
  • effective_stack
  • rake info
  • flop_bet_sizes
  • turn_bet_sizes
  • river_bet_sizes
  • turn_donk_sizes
  • river_bet_sizes
  • add_allin_threshold
  • force_allin_threshold
  • merging_threshold

• Additionally:

  • added_lines
  • removed_lines

A couple things to think about:

1. Data Duplication: card_config's flop/turn/river determine tree_config's initial_state value, and this might be annoying for a user to fill out/can lead to buggy configurations.

2. Sometimes, as in the case of a batch solve where we run over many different boards, the flop/turn/river cards aren't known in the configuration. PioSOLVER handles this by specifying dummy cards and then having the batch solve module accept a set of flops separately.

3. Adding and Removing Lines: This is not captured in TreeConfig, but this should be! The current implementation stores this in an ActionTree and then modifies this (or the PostFlopGame directly) to add/remove lines. However, adding and removing lines is a crucial part of specifying a tree.

Encoding Configurations

For now, I think just bundling everything into a JSON object

{
  "tree_config": {...},
  "card_config": {...},
  "added_lines": [...],
  "removed_lines": [...]
}

is the easiest way, but we should think about better ways to do this in the future.

Batch Solve

We need to be able to easily specify a tree configuration and some boards and have the solver go "vroom". I've added batch_solve as a binary src/bin.

[JacobVanGeffen]

When we save the config from batch solves, we should probably not include "flop" in the config, since there could be multiple flops, and it's also captured by the separate boards.txt output (not sure if that was the name we used).

Alternatively, we could output a list of flops here within the card config, but that would beg us to redesign the actual CardConfig struct to contain a list of flops, which is not something that sounds appealing.

[bkushigian]

See discussion at top of PR: sometimes we want to include the flop. We don't know at save time whether or not we want the flop. Basically we just include dummy cards, and if the user wants to overwrite this then they can (via --boards or --board-files).

I'd be open to making flop/turn/river optional (turn and river currently are actually, could make flop optional as well). But they should definitely be able to specify flops if they'd like.

[JacobVanGeffen]

Feel like we should overwrite boards.txt and config.json when --overwrite is specified.

[bkushigian]

Yeah...I've been thinking about this. If --overwrite is specified then we definitely want to overwrite the configs. But there are other situations to consider.

What if config.json and boards.txt already exist after completing part of a batch solve (e.g., 10/20 boards)? I want to skip existing solves, so no overwrite, but use existing configs. This just happened to me: my computer restarted and I had 950/1755 sims run. To continue I needed to delete config.json and boards.txt (which were copied in from another location) so I could point at the original location. Easy enough, but kind of a pain.

I think there should be another mode: Make the required config argument optional, and make the boards specification optional. If no config is passed in, look for config.json. If no boards are passed in via --boards or --boards-file, look for boards.txt.

I need to map out different scenarios and how I'd want batch_solve to work.

Here is an overview of different scenarios:

1. --overwrite is toggled: just overwrite existing everything
2. --overwrite is not toggled, --config CONFIG_JSON is specified, and config.json is present:

• if the CONFIG_JSON path points to config.json then we are fine...we can overwrite with the same contents without issue (easier than special casing.
• if the CONFIG_JSON path points to another file...print error and halt? I think this is safest

3. --overwrite is not toggled --boards BOARDS... or --boards-file BOARD_FILE is provided, and boards.txt exists

• If BOARDS_FILE points to boards.txt, then just overwrite...it's the same contents
• If BOARDS_FILE points elsewhere or if BOARDS is provided

[JacobVanGeffen]

Can we make enable_compression an argument?

[JacobVanGeffen]

nit: also, probably want verbose and silent options.

[bkushigian]

yeah we definitely can, on the todo list!

[JacobVanGeffen]

I personally find this function a bit unintuitive. I would've expected with_cards to consume self instead of taking it as reference, and also to update flop/turn/river no matter the previous number of dealt cards. IIUC what's actually happening is that a copy of the config is made, keeping only range.

It would make more sense to me if we had CardConfig::from_range_and_cards(ranges, cards), which essentially did the same thing, but required the caller to explicitly reference old_config.cards when constructing the new config.

[bkushigian]

Blah::with_blarg is typically immutable. To modify a Blah I'd use a Blah::set_blarg.

I'll update the doc string to reflect that.

Yeah, I wasn't sure what to do if the number of cards wasn't the same. It kinda makes no sense to take a turn range and apply it to the flop, but maybe someone wants to do that? But I feel like FlopCardConfigs, TurnCardConfigs, and RiverCardConfigs should have different types (like, not actually).

If you really want a flop range w/ a flop+turn you can construct it directly.

Happy to discuss further though! I'm really not sure!

[JacobVanGeffen]

Left some comments but looks awesome!

[JacobVanGeffen]

Also, why is this folder named bin? Just b/c you're defining a CLI here?

I think the underlying functionality for this can go in something like src/game/utils/batch, along with aggregate report functionality. We can define the CLI in a separate file if needed.

[bkushigian]

Yeah, we can move the functionality to a module. I still have some design decisions to consider, as we discussed before (I think there is an issue with the notion of "batch solving" which is basically "run a bunch of individual sims" when really we are creating a library of related sims, and this library should have certain operations that can be performed on it. This should resolve a lot of the ambiguities on what to do when files do/do not exist.