Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add instances file_name to game_registry #3

Open
AnneBeyer opened this issue Nov 25, 2024 · 4 comments
Open

Add instances file_name to game_registry #3

AnneBeyer opened this issue Nov 25, 2024 · 4 comments

Comments

@AnneBeyer
Copy link
Collaborator

Some game variants differ only in the instances (matchit, for example), so different entries for different versions only need to get a different game_name (so that results are not overwritten) and a different instance_file, but can use the same game_path.
The GameMaster could then first check if instance_file is given in the game_spec and only if not revert to the default (instances.json). We should think about whether game_spec can overwrite (or even replace) the command line argument -i though.

@davidschlangen
Copy link

That makes sense (making it possible to specify an instance file in a game spec, and giving this game spec a specific name).

The predence issue indeed is complicated. My feeling would be that anything that is specified in a call (i.e., through a command line argument) should be taken as taking precedence. But we do want (or at least I think that was the idea?) to make it possible to provide the game spec on the command line as well (just as a model spec can be given on the command line as a json). So that could lead to an inconsistent specification.... Could this be caught when parsing the command line? 1. If -i is given, and only a game name, .... nah, that doesn't work...

How about changing the mechanism in which -i is interpreted, by trying to unify this value into the instance_file field. If there's something in there already, this fails, and the call fails?

@Gnurro
Copy link
Contributor

Gnurro commented Nov 28, 2024

This would solve an issue I ran into with consolidating the wordle variants: The results path is determined by the game_name value in the game registry entry, meaning that all results are stored under results/wordle - if the variant instances are selected through the CLI argument, the results still get stored under results/model_pair/wordle, and the directory ends up containing model_pair/wordle/0_high_frequency_words_no_clue_no_critic and model_pair/wordle/0_high_frequency_words_clue_no_critic etc. Adding the instances used to the game entry would allow for both the run arguments and the results to have the same directory structure as before the consolidation in a more straight-forward way than having to add specific code all the way from cli.py to the game's specific code.

In general I'd prefer that any game/variant-specific info is clearly set in the game registry, and does not rely on additional CLI run arguments - meaning I'd rather get rid of the -i / --instances CLI argument and have its role taken over by different game registry entries entirely.

@Gnurro
Copy link
Contributor

Gnurro commented Jan 2, 2025

Added this to my current working branch: Gnurro/clembench@5a225a8

@Gnurro
Copy link
Contributor

Gnurro commented Jan 14, 2025

PR covering this is open: AnneBeyer/clembench#4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants