feat: Multimodal nemo gym compatible grpo pipeline - #3414
Conversation
|
Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
I think the following artifacts that depend on unavailable custom Gym components should be removed or replaced in this PR: If necessary, we should retain the intended sync/async multimodal coverage, but rewrite it using components available in the pinned Gym revision: circle_count_simple_agent for single-turn, and example_multi_turn_gymnasium_agent for multi-turn initial-image plus textual follow-up coverage. Later-turn media attribution can be covered with a focused NeMoRL unit test. |
…mbing and chat_template parity Folds three WIP commits (b16fca2 + 5486b57 + cd349d0): - Add tokenizer_config field to NemoGymConfig and processor reconstruction inside the actor for multi-turn multimodal postprocessing. - Add multimodal utilities (encode_images_in_examples, extract_multimodal_model_inputs, process_multimodal_chat, resolve_to_image) and their consumers in NemoGym. - Add sync single-step polish across nemo_gym.py / multimodal_utils.py / processors.py. - Maintain chat_template kwargs parity between the async vLLM worker and HF. Signed-off-by: rohitrango <rohit.rango@gmail.com> (cherry picked from commit 3215893) Signed-off-by: rohitrango <rohit.rango@gmail.com>
…ntroller pathway Reword of aa04447 (async plumbing). Adds the wiring in grpo.py and run_multimodal_grpo_nemo_gym.py so that multimodal NeMo-Gym rollouts flow through the async single-controller path introduced in the multimodal foundation commit. Signed-off-by: rohitrango <rohit.rango@gmail.com> (cherry picked from commit c755a25) Signed-off-by: rohitrango <rohit.rango@gmail.com>
…ygon-naming + multi-turn tool calling) Folds four WIP commits (28ba38e + 81f885f + ecf72e4 + 7171787): - Bump seq len and generation count on the doorkey smoke config. - Add tangram env smoke config and its launcher. - Add multi-turn multimodal tool calling / polygon-naming smoke config and launcher. Signed-off-by: rohitrango <rohit.rango@gmail.com> (cherry picked from commit b54e6a8) Signed-off-by: rohitrango <rohit.rango@gmail.com>
…m_to_nemo_rl_result Commit ec8333d added `nemo_gym_row: dict` as a new positional parameter to `NemoGym._postprocess_nemo_gym_to_nemo_rl_result` but never referenced it in the function body, and neither the sole in-tree caller (`_run_rollouts_iterator` in the same file) nor the six unit-test call sites (`tests/unit/environments/test_nemo_gym{,_router_replay}.py`) were updated to pass it. As a result every sync/async multimodal smoke recipe under `examples/nemo_gym/run_gymv_smoke.sh` crashed with TypeError: NemoGym._postprocess_nemo_gym_to_nemo_rl_result() missing 1 required positional argument: 'tokenizer' as soon as the first rollout came back from the Gym HTTP server — the `nemo_gym_result` positional was being consumed by the phantom `nemo_gym_row` slot, so `tokenizer` looked missing. Since the parameter is unused, the minimal fix is to remove it and restore the original 2-arg `(nemo_gym_result, tokenizer)` signature. This lines back up with all six test call sites (which were already passing 2 args) and the production caller at nemo_gym.py:442 (which was already passing 2 args), so no other files need touching. Signed-off-by: rohitrango <rohit.rango@gmail.com> (cherry picked from commit a2f8fd8) Signed-off-by: rohitrango <rohit.rango@gmail.com>
Signed-off-by: rohitrango <rohit.rango@gmail.com> (cherry picked from commit 7ddf223) Signed-off-by: rohitrango <rohit.rango@gmail.com>
Signed-off-by: rohitrango <rohit.rango@gmail.com> (cherry picked from commit 7bf55af) Signed-off-by: rohitrango <rohit.rango@gmail.com>
Signed-off-by: rohitrango <rohit.rango@gmail.com> (cherry picked from commit b7fc746) Signed-off-by: rohitrango <rohit.rango@gmail.com>
Signed-off-by: rohitrango <rohit.rango@gmail.com>
except assistant) Signed-off-by: rohitrango <rohit.rango@gmail.com> (cherry picked from commit 85a24b8) Signed-off-by: rohitrango <rohit.rango@gmail.com>
`_index_per_turn_images` gated its bucket flush on `role == "assistant"`, but `_postprocess_nemo_gym_to_nemo_rl_result` treats every item carrying truthy `generation_token_ids` as a trainable turn — including reasoning-only responses and `function_call` items whose role is not `"assistant"`. The mismatch left the batched flatten path with a `PackedTensor` for normal assistant turns and a missing entry for reasoning/tool-call turns, crashing `PackedTensor.flattened_concat` on async multimodal GRPO runs. Gate the flush on `generation_token_ids` directly so the per-turn image list stays aligned with the postprocess loop's `turn_idx`, and add regression tests for the reasoning-only and function_call cases. Also drop the unused `processor` kwarg from the async GRPO call site. Signed-off-by: rohitrango <rohit.rango@gmail.com> (cherry picked from commit 4c2537b) Signed-off-by: rohitrango <rohit.rango@gmail.com>
Add Args/Returns sections to image_to_data_url and encode_images_in_examples in nemo_rl/data/multimodal_utils.py. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: rohitrango <rohit.rango@gmail.com> (cherry picked from commit e9b89bc) Signed-off-by: rohitrango <rohit.rango@gmail.com>
|
/ok to test 062e999 |
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: rohitrango <rohit.rango@gmail.com>
Non-colocated 2n8g layout (vLLM TP=8 on node 1, Megatron TP=2/EP=8/CP=2
on node 2) for the single-turn Circle-Click NeMo-Gym environment.
Model points at the HF repo (nvidia/Nemotron-3-Nano-Omni-30B-A3B-Reasoning-BF16);
train/eval data_path use /path/to/{train,eval}_dataset.jsonl placeholders
so users wire in their own manifests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: rohitrango <rohit.rango@gmail.com>
The multimodal postprocessing path in _attach_multimodal_data_to_user_message assumes a placeholder-style processor: it reconstructs imgs_sizes / num_frames and builds the pixel_values PackedTensor with pad_to_max_shape=True. A non-placeholder VLM (e.g. Qwen2-VL / LLaVA-style) would silently produce wrong multimodal tensors instead of erroring. Fail loud at NemoGym.__init__ so the misconfiguration is caught at actor construction, well before any rollout. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: rohitrango <rohit.rango@gmail.com>
…click recipe Adds the required defaults: ../../vlm_grpo_3B_megatron.yaml key so the recipe passes the configs-minimize-check pre-commit hook (all recipes under examples/configs/recipes/**/*.yaml must inherit from an exemplar). Drops the standalone header preamble; provenance now lives in the adjacent clevr/mmpr sibling recipes and the commit history. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: rohitrango <rohit.rango@gmail.com>
|
/ok to test 68a538b |
Signed-off-by: rohitrango <rohit.rango@gmail.com>
|
/ok to test 4bb7e20 |
The circle-click recipe YAML landed without the driver script and suite entry that tests/unit/test_recipes_and_test_suites.py requires, so test_all_recipe_yamls_accounted_for_in_test_suites failed on a 229 vs 228 count mismatch. Add the missing driver. circle_click is a NeMo-Gym env, so it runs through run_grpo_nemo_gym.py rather than run_vlm_grpo.py, and the script regenerates its data via the resources server's generate_data.py (the committed example.jsonl has 5 rows and no agent_ref) with disjoint train/eval seeds. List it in disabled.txt rather than nightly.txt for now: the recipe has not been run end to end, so its reward threshold is an unvalidated smoke bound. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
a288c03 to
aa5ea0b
Compare
|
/ok to test aa5ea0b |
|
/ok to test 8ba3fe6 |
|
/ok to test f27dc6d |
|
/ok to test 234cdd7 |
What does this PR do ?
Adds multimodal, multiturn sync/async grpo with nemo gym environments.
Currently supports multi-image , multi-turn, tool calling on nemotron omni models (other models not verified).
Also contains some starter scripts.
Issues
List issues that this PR closes (syntax):
Supersedes #3325 (rebases on top of new force-pushed commits).
Usage
# Add a code snippet demonstrating how to use thisBefore your PR is "Ready for review"
Pre checks:
Additional Information
mult prob errors and reward validation on toy environments: