Repository navigation
feat(orchestration): auto-default model_call_capture_dir per benchmark - #3320
Merged
Merged
Conversation
observability_enabled=true requires an absolute model_call_capture_dir
(ModelCallCaptureConfig validator), and it has to be unique per submission
or CaptureStore (one file per rollout_id, not per submission) silently
appends different runs' captures into the same files.
This ties the default directly to each benchmark's own real output
directory (remote_bench_dir, already computed once per benchmark in
build_sbatch_script's loop) -- a benchmark author now only has to write
observability_enabled: true and gets a correct, real, collision-free path
for free. An explicit model_call_capture_dir still wins if set, mirroring
the existing health_check.port-from-service.port default precedent
(VllmServiceConfig._default_health_check).
Has to happen in Python, rather than as a ${...} Hydra interpolation:
remote_bench_dir isn't computed until well after SubmitConfig validation,
which itself runs after Hydra has already resolved every interpolation in
the submitted YAML -- there is no config-time value to reference.
Signed-off-by: Tomasz Grzegorzek <tgrzegorzek@nvidia.com>
6 of 9 tasks
prokotg
added
area:orchestration
marta-sd
reviewed
Sep 11, 2026
…onfig name Signed-off-by: Tomasz Grzegorzek <tgrzegorzek@nvidia.com>
marta-sd
approved these changes
Sep 11, 2026
jkyi-nvidia
pushed a commit
that referenced
this pull request
Sep 11, 2026
#3320) ## Summary Auto-derive `model_call_capture_dir` per benchmark when `observability_enabled: true` is set on a Slurm-submitted benchmark, instead of requiring the user to write it out by hand. - `observability_enabled=true` requires an absolute `model_call_capture_dir` (`ModelCallCaptureConfig` validator), and it must be unique per submission — `CaptureStore` writes one file per `rollout_id`, not per submission, so a reused path silently merges different runs' captures into the same files. - New `_with_default_capture_dir()` in `slurm_script.py` derives the path from each benchmark's own real remote output directory (`remote_bench_dir`, already computed once per benchmark in `build_sbatch_script`'s loop), giving a correct, collision-free path for free. - An explicit `model_call_capture_dir` in the config always wins over the default, mirroring the existing `health_check.port`-from-`service.port` precedent (`VllmServiceConfig._default_health_check`). - Has to happen in Python rather than via `${...}` Hydra interpolation: `remote_bench_dir` isn't known until well after `SubmitConfig` validation, by which point Hydra has already resolved every interpolation in the submitted YAML. ## Changes - `nemo_gym/orchestration/executors/slurm_script.py` — `_with_default_capture_dir()`, wired into `build_sbatch_script` - `examples/slurm_vllm_1IN.yaml` — updated example comment - `tests/unit_tests/test_slurm_script.py` — unit + integration coverage for default injection, explicit override, and observability-off cases - `tests/unit_tests/test_cli_eval_submit.py` — minor cleanup of a dead assertion ## Test plan - [ ] `pytest tests/unit_tests/test_slurm_script.py -x` - [ ] `pytest tests/unit_tests/test_cli_eval_submit.py -x` --------- Signed-off-by: Tomasz Grzegorzek <tgrzegorzek@nvidia.com> Signed-off-by: Junkeun Yi <jkyi@nvidia.com>
This branch was previously deployed
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
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.
Summary
Auto-derive
model_call_capture_dirper benchmark whenobservability_enabled: trueis set on a Slurm-submitted benchmark, instead of requiring the user to write it out by hand.observability_enabled=truerequires an absolutemodel_call_capture_dir(ModelCallCaptureConfigvalidator), and it must be unique per submission —CaptureStorewrites one file perrollout_id, not per submission, so a reused path silently merges different runs' captures into the same files._with_default_capture_dir()inslurm_script.pyderives the path from each benchmark's own real remote output directory (remote_bench_dir, already computed once per benchmark inbuild_sbatch_script's loop), giving a correct, collision-free path for free.model_call_capture_dirin the config always wins over the default, mirroring the existinghealth_check.port-from-service.portprecedent (VllmServiceConfig._default_health_check).${...}Hydra interpolation:remote_bench_dirisn't known until well afterSubmitConfigvalidation, by which point Hydra has already resolved every interpolation in the submitted YAML.Changes
nemo_gym/orchestration/executors/slurm_script.py—_with_default_capture_dir(), wired intobuild_sbatch_scriptexamples/slurm_vllm_1IN.yaml— updated example commenttests/unit_tests/test_slurm_script.py— unit + integration coverage for default injection, explicit override, and observability-off casestests/unit_tests/test_cli_eval_submit.py— minor cleanup of a dead assertionTest plan
pytest tests/unit_tests/test_slurm_script.py -xpytest tests/unit_tests/test_cli_eval_submit.py -x