Conversation
…hanging
When a prompt group fails permanently (for example, a prompt that a
generation engine deterministically rejects on every retry), its target
weight version is left short in the replay buffer. Two collector behaviors
then combine into a deadlock:
1. _calculate_target_weights starts the target window at
generation_weight_version + 1, so a short target at or below the current
version is invisible to gap-filling: no worker is ever dispatched to
regenerate the missing trajectories.
2. _collection_loop parks on an unbounded _generation_limit_cleared.wait().
The event is only set by a weight update, but training cannot step
without the missing trajectories, so the wait never returns.
The trainer then sits at the buffer wait forever ("Wait iteration N" with a
constant buffer size) while all generation nodes idle. Observed in
production on an async GRPO VLM run where one over-long prompt exceeded
max_model_len on every attempt.
Fix both halves:
- Extend the target window down to the first target training has not
consumed (min(current + 1, last_consumed + 1)). Both callers already skip
targets at or below last_consumed_target, so only genuine holes surface
and steady-state behavior is unchanged.
- Bound the generation-limit pause with a 60s re-check of
_should_pause_for_generation_limits so the collector wakes up and
gap-fills the shortfall with fresh prompts instead of deadlocking.
Signed-off-by: Pulkit Kumar <pulkitk@nvidia.com>
(cherry picked from commit 971b72acc9634092fdccf62570ffd8b2ed2eab54)
…ests - _calculate_target_weights no longer performs a Ray RPC: callers fetch last_consumed_target once and pass it in, removing the duplicate actor round-trip and keeping the helper unit-testable against mock buffers. - Extract the generation-limit pause into _pause_for_generation_limits and make the immediate post-clear() condition re-check the primary recovery path: a worker's set() landing between the pause check and the clear() is no longer lost. The timed re-check remains as a safety net only, with the interval lifted to a module constant. - Update the two window tests for the new signature (steady-state last_consumed_target keeps their expectations unchanged) and add regression tests for: the widened window over unconsumed failed targets, the set()-racing-clear() interleaving, the timeout re-check release, and the normal event-set wake path. Signed-off-by: Pulkit Kumar <pulkitk@nvidia.com> (cherry picked from commit 1bce48d27be2a0c867e53f87c61b2c4d41ec93db)
…er runs Async GRPO VLM training crashed mid-run with "Expanded-sequence media alignment failed: found 32160 valid placeholders for 40800 projected features". The generation engine sizes image tiles per request (shrinking them as prompts approach max_model_len); the training-side attach path re-processed the original images under the processor's static config budget, so budget-bound rows (e.g. 16 frames at 2400x1080 in a near-32k prompt) produced more projected vision features than the rollout's placeholder tokens, and the model forward raised on every rank. Derive the truth from the rollout itself instead of mirroring the engine's budget arithmetic: - multimodal_utils: parse per-image <img><image>*N</img> placeholder runs out of the rollout token ids; verify the processor's native output against them and, on mismatch, re-process each image pinned to the exact run length (budget pin via the image processor's max_model_len clamp, with a deterministic exact-grid resize fallback). Raise rather than attach misaligned media. - nemo_gym actor: pass each turn's placeholder runs into the attach; with deduplicate_multimodal_data on, keep the omission only when the statically-budgeted pre-attached tensors provably match the rollout's first-turn runs (predicted via the processor's own grid math), else attach rollout-matched tensors actor-side. - rollouts: the driver-side static reattach never overwrites media the actor already attached; video rows are unaffected (their tensors attach at datum time and their turns carry no extracted images). Verified offline against the checkpoint processor (exact-count repair across a 7-size x 7-budget sweep; the crash row now yields exactly matching feature counts; non-binding rows byte-match the previous fast path) and in production: the failing run resumed through the previously fatal batch and trained to completion with no alignment errors. Signed-off-by: Pulkit Kumar <pulkitk@nvidia.com> (cherry picked from commit b69d6457a0e063ba2ebdaae85e5ad763ab27cb46)
…ng dummy configure_generation_config unconditionally overwrote vllm_cfg.load_format with "dummy" for every training run, discarding any value set in the recipe YAML. With dummy weights, the generation engine depends on the trainer->generation refit to supply every parameter. For models with modules the refit does not cover -- e.g. a VLM whose vision tower is not part of the refit mapping -- those modules stay randomly initialized for the entire run. The failure is silent and easy to misdiagnose: the model still produces fluent, mostly well-formatted text (the language stack receives real weights via refit), but anything conditioned on the uncovered module is garbage. On a vision pointing/tracking GRPO task this showed up as step-1 rewards of ~0.09 (near random-guess level: ~60% of outputs parsed fine, generations were full-length, but localization was at chance) versus ~0.53-0.60 on the identical model/data/recipe once vLLM loaded real weights at startup. A/B verified by flipping only this setting. Fix: use setdefault so an explicit recipe value (e.g. policy.generation.vllm_cfg.load_format: auto) is honored, while keeping the existing defaults (dummy for training, auto for eval) for configs that do not set the key. The downstream forced overrides (sparse refit transports, non-MTP speculative decoding) are unchanged. Signed-off-by: Pulkit Kumar <pulkitk@nvidia.com> (cherry picked from commit 8bdbb88d978aae9a7b99325bed2ba92048b27bf6)
The nemotron_h_omni HF pipeline's NemotronH_Omni_Reasoning_V3Processor / NemotronH_Super_Omni_Reasoning_V3Processor implement the identical placeholder contract as the already-registered Nano variant (<image> runs expanded via __call__, image_token/start/end attributes; verified against the checkpoint's processing_nemotron_h_omni.py). Without registration the NemoGym multimodal guard fails at actor construction (job 6857981). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit d2471978ea67d21dd52255dbf7d0b805ecef875c)
Same registration-gap class as the placeholder-style fix (d2471978e): NemotronH_Omni_Reasoning_V3Processor (and the Super_Omni-named variant) belong to the same video-native family as the already-listed Nano V3 processor. Without registration, nemo_gym_video treats them as legacy frame-as-image processors and injects video_as_images=True, which vLLM's native nano_nemotron_vl integration rejects (job 6905470: 'NanoNemotronVLProcessor.__init__() got an unexpected keyword argument'). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 2c1ff8d473622c6696cde78b27d0572b61b0aa3d)
…ks verifier cherry-pick (287906d4, local) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e image sources _extract_input_images_from_message forwarded any function_call_output string with a data:/http(s)://file:// prefix to resolve_to_image. Real trajectories carry tool outputs like "file:///project/assets/resource.txt: Unsupported scheme." or a printed URL list; PIL/requests then raised inside _postprocess_nemo_gym_to_nemo_rl_result, the exception escaped run_rollouts, and the collector aborted and re-dispatched the whole 512-prompt batch. Since the offending text lives in the prompt's prefilled history, every retry failed the same way and the step could never complete (job 7068977). - function_call_output is only an image when it is an inline data:image/ URL, which is the only form image_tools_agent / gym_v_agent emit. Bash/curl text that merely starts with a URL no longer triggers a network fetch or file open. - Content-part image sources that fail to resolve are logged and skipped (_append_resolved_image) instead of raising. The existing placeholder-run parity check still hard-fails if a genuine image goes missing. Tool text was already plain text for the policy (Gym's vllm_model only treats image_url content parts as images), so training and generation now agree. Signed-off-by: adithyare <adithya.r@gmail.com>
Scoped port from ehsan/super35-video-rpb-tmpe-fix (6bf9228) onto the merged super-v3.5-posttraining tip, limited to what the combined-manifest experiment requires: - data/processors.py (+ small data/* hunks): allow one NeMo-Gym manifest to mix static-video rows with still-image rows (previously a hard failure), and fix the dtype mismatch that broke mixed batches; per-row image_max_num_tiles spec support. - multimodal_utils: add the uses_fixed_tile_image_processor predicate the ported processor code depends on. - generation/vllm/config.py: fail fast on the unsupported legacy vllm_cfg.video_loader key (the config typo behind the original failure). - algorithms/utils.py: keep the processor chat template in sync with the tokenizer when a template override is applied. - Gym submodule: current head plus a cherry-pick of the SAV tracking verifier (resources_servers/sav_tracks) — no pointer rewind. Deliberately excluded (tracked separately): refit pause/resume (superseded upstream per branch owner), vLLM private-API monkeypatches and the 0.20 serving shim, Megatron-Bridge registry surgery and other old-container affordances, tiling-parity/rollout attach changes, load_format handling, reference recipes and personal launchers. Signed-off-by: Pulkit Kumar <pulkitk@nvidia.com>
With data.default.video_sampling_style set, nemo_gym_data_processor routed EVERY row through nemo_gym_example_to_video_datum_spec. Its message extractor accepts only image and video-frame parts, so agentic rows whose assistant turns carry output_text parts died with 'Unsupported Gym multimodal content type: output_text' (job 7113826, 11,987 rows of the setup_2 blend). Only rows with _is_video_frame parts (cached CapRL frames) or a native video part now take the video path; SA-V still-image rows and text rows use the standard Gym placeholder as the mixed-manifest port (f1dfb70) intended. Signed-off-by: adithyare <adithya.r@gmail.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
…rocessors used by the joint MOPD runs) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
arendu
force-pushed
the
adithyare/super_v3.5_joint_mopd_v4
branch
from
September 14, 2026 19:55
c295aa1 to
14e9352
Compare
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.
What does this PR do ?
Add a one line overview of what this PR aims to accomplish.
Issues
List issues that this PR closes (syntax):
Usage
# Add a code snippet demonstrating how to use thisBefore your PR is "Ready for review"
Pre checks:
Additional Information