Skip to content

feat(video): add Gym support for sync and async GRPO - v2 - #3648

Merged
yfw merged 7 commits into
mainfrom
ehsan/video-rl-review-refactor
Aug 21, 2026
Merged

feat(video): add Gym support for sync and async GRPO - v2#3648
yfw merged 7 commits into
mainfrom
ehsan/video-rl-review-refactor

Conversation

@ehosseiniasl

@ehosseiniasl ehosseiniasl commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Add end-to-end static-prompt video support for NeMo Gym GRPO with synchronous and asynchronous Megatron training.

This PR:

  • accepts one local video in the initial Gym prompt, either as a source video or a cached-frame manifest;
  • carries video metadata, policy frame tensors, native token IDs, and log probabilities through Gym rollout, batching, generation, and policy training;
  • matches stock vLLM 0.25.1 Nemotron video sampling on the policy side and registers the compatible video loader without requiring a custom vLLM fork;
  • trains the canonical processor-expanded Nemotron Omni model through the pinned Megatron-Bridge integration;
  • supports both synchronous and asynchronous video GRPO;
  • adds dataset preparation, product recipes, documentation, focused tests, and nightly coverage.

Scope

This integration supports static prompt media: one video known before rollout and attached to the initial user turn. Environment-produced video in later trajectory turns is not supported by this PR.

Generic GRPO step-limit semantics, SingleController behavior, async lifecycle/checkpoint changes, and unrelated refit ordering are intentionally outside this PR. It does not patch vLLM source interpolation behavior.

Validation

  • Ruff formatting and lint checks passed.
  • Pyrefly passed for supported changed modules.
  • Exact rebased candidate: 704 passed, 6 skipped across recipe/config, dataset conversion, Gym video, multimodal rollout, Nemotron preprocessing, and stock-vLLM compatibility coverage.
  • The 2-node synchronous run is logging raw, unmasked sequence probability error and reward to W&B; exact-final 2-node synchronous and 16-node asynchronous evidence will be added before merge.
  • No credentials, personal launchers, internal accounts, or debugging-only artifacts are included.

Related integrations

Both upstream integrations are merged, and the RL submodules are pinned to commits containing them. No separate Megatron-LM change is required.

@copy-pr-bot

copy-pr-bot Bot commented Aug 14, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added the Documentation Improvements or additions to documentation label Aug 14, 2026
@ehosseiniasl ehosseiniasl changed the title feat(video): add Gym support for sync and async GRPO - refactored feat(video): add Gym support for sync and async GRPO - v2 Aug 14, 2026
@ehosseiniasl ehosseiniasl self-assigned this Aug 14, 2026
@ehosseiniasl
ehosseiniasl marked this pull request as ready for review August 14, 2026 16:32
@ehosseiniasl
ehosseiniasl requested review from a team as code owners August 14, 2026 16:32
@ehosseiniasl
ehosseiniasl force-pushed the ehsan/video-rl-review-refactor branch from 785eeab to c7adbfe Compare August 14, 2026 23:26
@github-actions

Copy link
Copy Markdown

✅ Submodule Fast-Forward Check Results

Check based on commit: c7adbfe (PR #3648 from ehsan/video-rl-review-refactor)

✅ Submodules that are properly updated:

Gym: ✅ PR branch is ahead of main branch (fast-forward)

All submodule changes look good! ✨

Comment thread nemo_rl/environments/nemotron_utils.py Outdated
@aroshanghias-nvd

Copy link
Copy Markdown
Contributor

This video path is static prompt video only: one clip known before rollout, processed on the driver, then glued onto the first user turn.

That matches image Gym for the initial prompt pixels, but not for env-produced media. Image GRPO indexes input_image per turn after Gym (_index_per_turn_images_attach_multimodal_data_to_user_message). Video never does the equivalent:

  • Gym postprocess still only understands input_image.
  • PackedTensors are built from the dataset example, not from Gym output.
  • reattach_static_multimodal_payload writes that payload onto the first user message and returns.

A future multi-turn env that returns a new input_video each turn would drop those clips: they would not be processed, and they would not land on the right turn.

Fine for the current recipes (mp4 is in the user prompt). If later Gym tasks emit video mid-trajectory, video needs the image shape: Gym returns input_video on the turn that introduced it, NeMo-RL indexes per turn, then resize/normalize into PackedTensors after the trajectory exists. Pre-Gym processing can stay for the initial clip (overlength / G-repeat dedup) only.

Worth stating so this is not mistaken for a general video-Gym contract.

@aroshanghias-nvd

Copy link
Copy Markdown
Contributor

Please drop ehsan_scripts/ before merge to main. This is not a public launch surface.

The in-tree recipes already cover the productized config:

  • examples/nemo_gym/grpo_nemotron_omni_30ba3b_video_{sync,async}.yaml
  • examples/nemo_gym/prepare_video_dataset.py
  • docs/guides/models/nemotron/nemotron-3-nano-omni.md

ehsan_scripts/ is a personal/internal validation launcher: folder named after the author, hardcoded NVIDIA SLURM accounts (nemotron_edge_omni, nemotron_omni_vision), default interactive partition, /opt/rl_main_vg_runtime.env, and internal run prefixes (rl_main_vg2_sync_video_review_refactor_unlimited). README examples also use /lustre:/lustre. None of that belongs on main.

tests/unit/test_video_grpo_recipes.py currently reads those scripts (SCRIPT_DIR = ... / "ehsan_scripts"). Keep the YAML assertions; delete or rewrite test_video_grpo_launchers_keep_compiler_caches_node_local so it does not depend on that folder.

I do not see author names or /lustre/fs1/... user paths in the recipes/docs added by this PR. The leftover cleanup is this folder + the test that pins it.

Comment thread nemo_rl/environments/nemo_gym_video.py
Comment thread tests/unit/test_prepare_video_dataset.py
Comment thread nemo_rl/algorithms/grpo.py Outdated
Comment thread nemo_rl/algorithms/async_utils/trajectory_collector.py Outdated
Comment thread nemo_rl/models/generation/vllm/video_utils.py Outdated
Comment thread nemo_rl/algorithms/async_utils/trajectory_collector.py Outdated
Comment thread ehsan_scripts/README.md Outdated
Comment thread examples/nemo_gym/grpo_nemotron_omni_30ba3b_video_async.yaml Outdated
Comment thread nemo_rl/algorithms/grpo_sync.py Outdated
Comment thread nemo_rl/algorithms/grpo_sync.py
Comment thread nemo_rl/experience/rollout_manager.py Outdated
Comment thread nemo_rl/experience/rollouts.py
Signed-off-by: Ehsan Hosseini Asl <ehsan.hosseiniasl@gmail.com>
ehosseiniasl added a commit that referenced this pull request Aug 18, 2026
Address all actionable review comments for PR #3648.

- centralize image, video, and audio content aliases across preprocessing
- validate cached frames as one video source and normalize local image aliases
- align Nemotron Omni tubelet placeholders and timestamps with vLLM metadata
- materialize one strict frame-sampling contract for rollout and policy paths
- reattach deduplicated media to sync and async Gym message logs consistently
- remove obsolete optional-epoch handling from SingleController step clamping
- document video runtime, cache, epoch, and raw-TMPE contracts
- add focused config, media, vLLM, and executable 2n/16n regression coverage

Signed-off-by: Ehsan Hosseini Asl <ehsan.hosseiniasl@gmail.com>
@ehosseiniasl
ehosseiniasl force-pushed the ehsan/video-rl-review-refactor branch from c7adbfe to fda58a4 Compare August 18, 2026 20:39
@ehosseiniasl
ehosseiniasl requested a review from a team as a code owner August 18, 2026 20:39
Comment thread tests/unit/models/generation/test_vllm_video_utils.py
ehosseiniasl added a commit that referenced this pull request Aug 19, 2026
Address all actionable review comments for PR #3648.

- centralize image, video, and audio content aliases across preprocessing
- validate cached frames as one video source and normalize local image aliases
- align Nemotron Omni tubelet placeholders and timestamps with vLLM metadata
- materialize one strict frame-sampling contract for rollout and policy paths
- reattach deduplicated media to sync and async Gym message logs consistently
- remove obsolete optional-epoch handling from SingleController step clamping
- document video runtime, cache, epoch, and raw-TMPE contracts
- add focused config, media, vLLM, and executable 2n/16n regression coverage
- pin Gym to merged video Responses API integration NVIDIA-NeMo/Gym#2324

Signed-off-by: Ehsan Hosseini Asl <ehsan.hosseiniasl@gmail.com>
@ehosseiniasl
ehosseiniasl force-pushed the ehsan/video-rl-review-refactor branch from fda58a4 to 3b96de2 Compare August 19, 2026 03:25
@github-actions

Copy link
Copy Markdown

✅ Submodule Fast-Forward Check Results

Check based on commit: 3b96de2 (PR #3648 from ehsan/video-rl-review-refactor)

✅ Submodules that are properly updated:

Gym: ✅ PR branch is ahead of main branch (fast-forward)

All submodule changes look good! ✨

@github-actions

Copy link
Copy Markdown

✅ Submodule Fast-Forward Check Results

Check based on commit: 40e3161 (PR #3648 from ehsan/video-rl-review-refactor)

✅ Submodules that are properly updated:

Gym: ✅ PR branch is ahead of main branch (fast-forward)

All submodule changes look good! ✨

@rohitrango

Copy link
Copy Markdown
Contributor

/ok to test 40e3161

@ehosseiniasl ehosseiniasl added the CI:L2 Run doctests, unit tests, functional tests, and convergence tests label Aug 20, 2026
@github-actions

Copy link
Copy Markdown

✅ Submodule Fast-Forward Check Results

Check based on commit: 9a64fea (PR #3648 from ehsan/video-rl-review-refactor)

✅ Submodules that are properly updated:

Gym: ✅ PR branch is ahead of main branch (fast-forward)

All submodule changes look good! ✨

@rohitrango
rohitrango dismissed their stale review August 20, 2026 15:56

reasons addressed in latest changes

@rohitrango rohitrango added CI:Lfast Runs a fast test suite and re-use nightly `main` container (but sync dependencies to PRs version) and removed CI:L2 Run doctests, unit tests, functional tests, and convergence tests labels Aug 20, 2026
@rohitrango

Copy link
Copy Markdown
Contributor

/ok to test 9a64fea

Signed-off-by: rohitrango <rohit.rango@gmail.com>
@rohitrango

Copy link
Copy Markdown
Contributor

/ok to test 54f5234

@github-actions

Copy link
Copy Markdown

✅ Submodule Fast-Forward Check Results

Check based on commit: 54f5234 (PR #3648 from ehsan/video-rl-review-refactor)

✅ Submodules that are properly updated:

Gym: ✅ PR branch is ahead of main branch (fast-forward)

All submodule changes look good! ✨

rohitrango
rohitrango previously approved these changes Aug 20, 2026
Signed-off-by: rohitrango <rohit.rango@gmail.com>
@github-actions

Copy link
Copy Markdown

✅ Submodule Fast-Forward Check Results

Check based on commit: a99a34c (PR #3648 from ehsan/video-rl-review-refactor)

✅ Submodules that are properly updated:

Gym: ✅ PR branch is ahead of main branch (fast-forward)

All submodule changes look good! ✨

1 similar comment
@github-actions

Copy link
Copy Markdown

✅ Submodule Fast-Forward Check Results

Check based on commit: a99a34c (PR #3648 from ehsan/video-rl-review-refactor)

✅ Submodules that are properly updated:

Gym: ✅ PR branch is ahead of main branch (fast-forward)

All submodule changes look good! ✨

@rohitrango

Copy link
Copy Markdown
Contributor

/ok to test a99a34c

@github-actions

Copy link
Copy Markdown

✅ Submodule Fast-Forward Check Results

Check based on commit: ef5ac5e (PR #3648 from ehsan/video-rl-review-refactor)

✅ Submodules that are properly updated:

Gym: ✅ PR branch is ahead of main branch (fast-forward)

All submodule changes look good! ✨

@rohitrango

Copy link
Copy Markdown
Contributor

/ok to test ef5ac5e

@yfw
yfw enabled auto-merge (squash) August 21, 2026 03:33
@yfw
yfw merged commit 7571a7d into main Aug 21, 2026
85 of 86 checks passed
@yfw
yfw deleted the ehsan/video-rl-review-refactor branch August 21, 2026 05:05
@yfw yfw mentioned this pull request Aug 21, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI:Lfast Runs a fast test suite and re-use nightly `main` container (but sync dependencies to PRs version) Documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants