Standardize CLI flags on underscore spellings#507
Closed
eugenevinitsky wants to merge 8 commits into
Closed
Conversation
Config-derived flags are registered dash-style only, so the natural --env.num_agents spelling was rejected with 'unrecognized arguments' while hand-registered top-level flags (--num_scenarios) require underscores. Normalize underscore spellings to their registered dash form at parse time; flags registered with underscores are untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A few top-level flags are registered with underscores (--num_scenarios, --agent_index, --eval_simulation) while everything else is dash-style. Rewrite unregistered tokens to whichever form the parser knows, so both spellings work for every flag in both directions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
--num_scenarios, --agent_index, and --eval_simulation were the only flags registered with underscores; register them dash-style like every other flag and update docs/help text. Old underscore spellings keep working: main()'s eval-flag scan already accepts both, and normalize_flag_dashes rewrites the rest at parse time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Drop the underscore-spelling normalization and the dual-spelling entries in main()'s eval-flag scan. --num_scenarios, --agent_index, and --eval_simulation were the last underscore registrations; every flag is now dash-style and underscore spellings are rejected. Update the eval scripts that forwarded underscore spellings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
eugenevinitsky
changed the title
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
submit_cluster.py (--save-dir, --compute-config, --program-config, --gpu-type, --task-per-node, --max-pjob, --container-image, --container-overlay) and drive.py's --data-dir, plus every caller and doc reference. argparse dests are unchanged. torchrun's --nproc_per_node is external and stays as is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Register config-derived flags verbatim as their ini keys (drop the underscore-to-dash conversion) so flag == config key == dest == wandb key with no translation layer anywhere. Convert all hand-registered flags, script interfaces, internal command builders (manager.py subprocess eval, utils.py visualize argv, submit_cluster --args), and docs to match. External tool flags (torchrun --nproc-per-node/--rdzv-*, sbatch, pip, nvidia-smi, git) and --local-rank (torch launcher convention) keep their native spellings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
eugenevinitsky
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Standardizes the entire repo's CLI on underscore-style flags — the same spelling as the config keys they mirror:
drive.ini/default.inikeys (thefmt.replace("_", "-")conversion is gone), so--env.num_agentsis the flag, the argparse dest, the ini key, and the wandb key — no translation layer anywhere. Hand-registered flags (--load_model_path,--num_scenarios,--render_mode,--wandb_project, …) match.manager.pysubprocess-eval commands,utils.py's visualize argv (+visualize.c's parser),submit_cluster.py's--args key=valuepass-through (which previously converted underscores to dashes).submit_cluster.py(--save_dir,--compute_config, …),run_all_eval.sh,run_all_latest_eval.py,render_scenario.py,drive.py --data_dir.docs/cluster_training.md,docs/evaluation.md, benchmark README.Exceptions (external conventions, unchanged): torchrun's
--nproc-per-node/--rdzv-*, sbatch/pip/nvidia-smi/git flags, and--local-rank(injected by torch launchers).Why
argparse matches flag spellings literally, and the repo mixed two conventions: config-derived flags were dash-converted at registration while other flags (and all the cluster scripts) used underscores. The natural workflow — copy a key from
drive.iniand type--env.num_agents— failed withunrecognized arguments; the README's own examples had it wrong, andmain()had grown a dual-spelling workaround for a few eval flags. Registering flags verbatim as their config keys removes the footgun at its source: one spelling, zero conversions.Notes
load_config(top-level, dotted, multi-value,=valueform); dash spellings are rejected with a clear argparse error;submit_cluster.py --dryworks; end-to-end CPU training with underscore flags runs to a checkpoint.--local-rankexception.run_failure_scenarios.py:65passes--map_indices, a flag that doesn't exist anywhere in pufferlib; andvisualize.cdoesn't compile against currentdrive.h(missinggoal_behavior/puffersfields) — its flag strings were updated for consistency but the binary was already unbuildable.🤖 Generated with Claude Code