Skip to content

Commit 784f8e4

Browse files
macandro96claude
andcommitted
fix(ultra): use resolvable config placeholders and correct recipe references
Address review feedback on the Ultra recipes PR: - Replace `sif_dir: ???` / `_teachers.general: ???` with concrete placeholders so example configs resolve under `OmegaConf.to_container(resolve=True)`, fixing the blocking CI regression in test_config_validation.py. Fail-fast is preserved at the launch boundary: ultra_launch.sh now hard-requires SIF_DIR whenever the selected config interpolates `${sif_dir}` (mopd already guards NRL_GENERAL_TEACHER_PATH). - Correct nonexistent `student_rlvr.yaml` references to `student_rlvr1.yaml` in the launch guide and ultra_launch.sh. - Drop the tautological `save_optimizer is True` assert in the Ultra config contract test, keeping the meaningful schema + CheckpointManager checks. - Use a non-"interactive" reaper reason for the batch training submit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Anish Mahishi <amahishi@nvidia.com>
1 parent 6c466a1 commit 784f8e4

5 files changed

Lines changed: 22 additions & 12 deletions

File tree

docs/guides/nemotron-3-ultra.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ Set the following before each `bash ultra_launch.sh` invocation:
212212
| Variable | Purpose |
213213
|---|---|
214214
| `EXP_NAME` | Job name, W&B run name, and the suffix for output directories. Must be unique per run; same name across resubmissions resumes from the latest checkpoint. |
215-
| `CONFIG_PATH` | Path to the per-stage YAML config (e.g. `examples/configs/ultra/student_rlvr.yaml`). |
215+
| `CONFIG_PATH` | Path to the per-stage YAML config (e.g. `examples/configs/ultra/student_rlvr1.yaml`). |
216216
| `MODEL_PATH` | Initial policy checkpoint (HuggingFace repo id or local path). Student RLVR starts from the Ultra SFT checkpoint; the teacher stages start from the Student RLVR checkpoint; MOPD starts from the student (the Student RLVR checkpoint). |
217217
| `TRAIN_PATH` | Training data JSONL. |
218218
| `VAL_PATH` | Validation data JSONL. |

examples/configs/ultra/mopd.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,7 @@ reward_penalties:
758758
# until those recipes are ready they fall back to `general` here.
759759
# =============================================================================
760760
_teachers:
761-
general: ??? # Required — set by launcher via NRL_GENERAL_TEACHER_PATH
761+
general: /path/to/general_teacher # Placeholder so the config resolves; required at runtime — set by launcher via NRL_GENERAL_TEACHER_PATH
762762
rlhf: ${_teachers.general}
763763
ifbench: ${_teachers.general}
764764
reasoning: ${_teachers.general}

examples/configs/ultra/swe_teacher.yaml

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,13 @@ cluster:
3636
segment_size: 16
3737

3838
# =============================================================================
39-
# SIF image root — required, set by the launcher via SIF_DIR. The SWE agent's
40-
# container_formatter (in env.nemo_gym.swe_agents_train) resolves
41-
# `${sif_dir}/<subdir>/{instance_id}.sif` per SWE benchmark family.
39+
# SIF image root — required at runtime, set by the launcher via SIF_DIR. The SWE
40+
# agent's container_formatter (in env.nemo_gym.swe_agents_train) resolves
41+
# `${sif_dir}/<subdir>/{instance_id}.sif` per SWE benchmark family. The value
42+
# below is a placeholder so the config resolves at load/validation time;
43+
# ultra_launch.sh hard-requires SIF_DIR and overrides it for real runs.
4244
# =============================================================================
43-
sif_dir: ???
45+
sif_dir: /path/to/sif_images
4446

4547
# =============================================================================
4648
# Checkpointing

tests/unit/utils/test_config.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,9 +259,10 @@ def test_ultra_configs_satisfy_current_grpo_contract(config_path):
259259
config["_teachers"]["general"] = "/tmp/test-teacher"
260260

261261
resolved = OmegaConf.to_container(config, resolve=True)
262-
master_config = MasterConfig.model_validate(resolved)
263262

264-
assert master_config.checkpointing["save_optimizer"] is True
263+
# The real contract checks: the config validates against GRPO's MasterConfig
264+
# schema and the checkpointing block is accepted by CheckpointManager.
265+
master_config = MasterConfig.model_validate(resolved)
265266
CheckpointManager(master_config.checkpointing)
266267

267268

ultra_launch.sh

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ set -euo pipefail
1717
# Usage:
1818
#
1919
# EXP_NAME=ultra-student-rlvr-001 \
20-
# CONFIG_PATH=examples/configs/ultra/student_rlvr.yaml \
20+
# CONFIG_PATH=examples/configs/ultra/student_rlvr1.yaml \
2121
# MODEL_PATH=/path/to/sft_checkpoint \
2222
# TRAIN_PATH=/path/to/train.jsonl \
2323
# VAL_PATH=/path/to/val.jsonl \
@@ -72,7 +72,7 @@ set -euo pipefail
7272
# Required environment
7373
# =============================================================================
7474
: "${EXP_NAME:?EXP_NAME is required (used for job name, W&B run, checkpoint/log dirs)}"
75-
: "${CONFIG_PATH:?CONFIG_PATH is required (e.g. examples/configs/ultra/student_rlvr.yaml)}"
75+
: "${CONFIG_PATH:?CONFIG_PATH is required (e.g. examples/configs/ultra/student_rlvr1.yaml)}"
7676
: "${MODEL_PATH:?MODEL_PATH is required (initial policy checkpoint, HF repo id or local path)}"
7777
: "${TRAIN_PATH:?TRAIN_PATH is required (training data jsonl path)}"
7878
: "${VAL_PATH:?VAL_PATH is required (validation data jsonl path)}"
@@ -106,6 +106,13 @@ if [[ ! -f "${CONFIG_PATH}" ]]; then
106106
exit 1
107107
fi
108108

109+
# The SWE teacher recipe interpolates `${sif_dir}/...` paths at runtime. The
110+
# exemplar config carries only a placeholder, so hard-require SIF_DIR whenever
111+
# the selected config actually uses it (mirrors the mopd teacher-path guard).
112+
if grep -q '${sif_dir}' "${CONFIG_PATH}"; then
113+
: "${SIF_DIR:?SIF_DIR is required for the SWE recipe (directory of apptainer .sif images)}"
114+
fi
115+
109116
# =============================================================================
110117
# Project root and code root
111118
# =============================================================================
@@ -266,7 +273,7 @@ if [[ "${ENABLE_MTP_INFERENCE}" == "1" ]]; then
266273
fi
267274

268275
# =============================================================================
269-
# Job shape — defaults match the 256-node student_rlvr.yaml
276+
# Job shape — defaults match the 256-node student_rlvr1.yaml
270277
#
271278
# Training: 64 nodes ( 256 GPUs) — Megatron training backend
272279
# vLLM: 172 nodes ( 688 GPUs) — async generation, EP=8 instances at TP=8
@@ -834,7 +841,7 @@ SBATCH_OUTPUT=$(sbatch \
834841
${SLURM_QOS:+--qos="${SLURM_QOS}"} \
835842
${EXCLUDE_NODES:+--exclude="${EXCLUDE_NODES}"} \
836843
${SLURM_RESERVATION:+--reservation="${SLURM_RESERVATION}"} \
837-
--comment='{"OccupiedIdleGPUsJobReaper":{"exemptIdleTimeMins":"60","reason":"interactive","description":"interactive debugging"}}' \
844+
--comment='{"OccupiedIdleGPUsJobReaper":{"exemptIdleTimeMins":"60","reason":"batch","description":"batch training run"}}' \
838845
"${RAY_SUB}")
839846

840847
echo "${SBATCH_OUTPUT}"

0 commit comments

Comments
 (0)