feat(mopd): add Super Omni multimodal distillation - #3430
Conversation
d60004f to
d3e02b4
Compare
768375e to
cc54510
Compare
Route row-aligned multimodal inputs to non-colocated teachers and optionally clear vLLM encoder outputs after quiesced weight updates. Signed-off-by: Ilia Karmanov <ikarmanov@nvidia.com>
Add production and smoke overlays, a thin launcher, deterministic circle-count data preparation, and focused usage documentation. Signed-off-by: Ilia Karmanov <ikarmanov@nvidia.com>
Install each Gym service into its newly created venv when a managed Python interpreter is selected globally. Signed-off-by: Ilia Karmanov <ikarmanov@nvidia.com>
5d75673 to
441229e
Compare
yfw
left a comment
There was a problem hiding this comment.
Multi-agent review of the Super Omni image-MOPD change (7 agents: RL / Gym / design / bug / test experts plus an adversarial challenge round).
Overall this is solid work. I verified the parts easiest to get wrong and they hold up: AsyncLLM.reset_encoder_cache() is real and correctly used on the pinned vLLM 0.25.1; the staleness it fixes is genuine (vLLM's EncoderCacheManager keys on mm_hash, which is weight-independent, and retains finished entries until capacity pressure); the Gym contract matches exactly (make_example signature, return shape, agent_ref discriminator, both config_paths) and the data-prep script runs clean; node accounting (1 gen + 1 teacher + 8 policy = 10) and teacher TP8xEP8 are valid; and the row-alignment logic including DP-pad duplication is correct.
One blocking item: both new tests/test_suites/vlm/mopd-*.sh omit the # ===== BEGIN CONFIG ===== block, which fails an existing non-GPU unit test. Suggestions are inline; I verified the fix takes that suite from 1 failed, 14 passed to 15 passed.
The rest are suggestions. Two are worth reading carefully because a partial fix makes things worse: the config-conventions item is a paired edit (exemplar + reference config, or CI goes red), and the encoder-cache flag is silently ignored on transports other than the three you instrumented.
A few things this review deliberately does not ask for, having considered and rejected them: routing the reset through invalidate_kv_cache (would couple a correctness requirement to an opt-in staleness policy — your placement is better), renaming the recipes to vlm_mopd- (would fail test_all_recipes_start_with_algo_hyphen), and a before/after benchmark for the cache reset.
Generated by Claude Code
tools/launch extracts NUM_NODES/GPUS_PER_NODE/STEPS_PER_RUN/MAX_STEPS/ NUM_RUNS/NUM_MINUTES from a delimited CONFIG block and exits 1 when the markers are absent, which failed test_dry_run_does_not_fail_and_prints_total_gpu_hours (disabled.txt does not exempt drivers from that glob). Wrap the existing vars in the 10n8g driver and add a mirroring block to the smoke wrapper, keeping NUM_NODES/NUM_MINUTES exported for the exec'd child. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
Document the new flag's default in the exemplar YAML and its paired reference config (test_reference_configs_up_to_date requires both), and make normalize_vllm_refit_config raise when the flag is combined with a refit transport whose weight-landing path never resets the encoder cache (nixl, sparse-delta, custom checkpoint engines). Only the collective/IPC and nccl_reshard async paths implement the reset; on any other transport the flag was a silent no-op that kept stale vision embeddings across refits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
The existing tests cover all-image and all-text groups; a single teacher receiving both takes the filter branch that keeps a PackedTensor with a None segment, and nothing asserted the surviving None stays row-aligned with its token row. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
The guide was added to the docs/index.md toctree but not to the Nemotron landing page that lists every sibling guide. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
…her_alias Replace the SPDX two-liner with the standard Apache block (and fix the ruff isort blank-line failure it sat next to), and remove default_teacher_alias from the recipe: resolve_reference_aliases checks strict_agent_name_match before the fallback branch, so with strict matching enabled the alias could never be used. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
prefetch_omni_envs.yaml mirrored only the Super Omni GRPO recipe's five Gym servers, so images built with NEMO_GYM_PREFETCH_CONFIGS=examples/nemo_gym/prefetch_omni_envs.yaml had no baked venv for the circle-click GRPO recipe or the new circle-count MOPD recipe, forcing an on-node uv venv build (network egress plus a writable /opt/gym_venvs) at first startup. Add both servers; these are the only Gym-using omni recipes not already covered. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
|
/ok to test 870aa0d |
|
LGTM. Just some failures in CI. |
The lint workflow's ratchet requires every file with zero pyrefly errors to be listed in project-includes; the new examples/nemo_gym/nemotron-3-super-omni/prepare_circle_count_mopd_data.py type-checks clean, so CI failed until it was whitelisted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
|
/ok to test eec4023 |
normalize_vllm_refit_config is also called by worker-side NIXL setup with partial generation configs that carry no vllm_cfg key, so the new guard's direct subscript raised KeyError (test_configure_nixl_worker_ignores_other_configs). Use a presence check instead; the guard still raises when the flag is actually set on an unsupported transport. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
|
/ok to test ec99431 |
What does this PR do?
Adds image MOPD for Nemotron 3 Super Omni on top of the Super GRPO support merged in #3494.
Two runtime changes are still needed for MOPD:
The PR also adds:
Relationship to #3494
This branch is based on current
mainand uses #3494's Super recipe, launcher, Gym integration, provider setup, and dynamic-image handling unchanged.It does not restore placeholder sanitization. That work remains separate in #3631 and NVIDIA-NeMo/Megatron-Bridge#5573. The circle-count data used here has one structured image per row and no literal media tokens, so it does not depend on that work.
MTP is disabled for this MOPD recipe (
mtp_num_layers: 0). The MTP recipes added by #3494 are unaffected.Validation
Four-node, three-step smoke:
1687836, completed with exit code 0input_imagepayloads;0.00173;[-0.639, 0.491];0.00313;0.643.Local checks:
Test plan