Repository navigation
Conversation
_patch_vllm_radio_layerscale_loader matched RadioModel.load_weights as a single
20-line snippet. The super_vl_rl_v0.25.1 fork that the nemo-ci rl-gym images
ship reshaped that body -- it gates the legacy branch on is_legacy, inserts an
"elif not is_legacy:" native-mapping branch (layer_scale{1,2}.lambda1 ->
ls{1,2}), and makes the weight_loader call shard-aware -- so the match fails and
the patch returns without applying.
That failure is silent and expensive. Super Omni checkpoints are folded exports
carrying no ls1/ls2 tensors, megatron_cfg.freeze_vision_model keeps the vision
tower out of the refit, and NeMo-RL forces load_format=dummy, so this patch's
initializer_factor fill is the only code path that ever sets LayerScale. Without
it every residual branch of the vision tower is scaled by dummy-init noise, at
unchanged step time and with no error raised.
Measured on a 16-node Super Omni GRPO run, identical recipe, data and
checkpoint, step 1:
reward token_mult_prob_error masked seqs
patch applies 0.574 1.0305 11 / 2048
patch no-ops 0.216 1.4503 1775 / 2048
after this change 0.556 1.0305 6 / 2048
Split the rewrite into two independent edits anchored on text that is stable
across both variants, so a future reshape of one branch cannot silently disable
the other. The required edit now warns and refuses to write a half-applied patch
rather than returning quietly.
Verified against both stock vLLM 0.25.1 and the fork's radio.py: the patch
applies, the result parses, the fill loop lands inside load_weights before its
return, the fork's native-mapping branch and shard-aware weight_loader survive,
and a second application is a no-op.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: DanialTaheri <smohsenitahe@nvidia.com>
yfw
approved these changes
Sep 5, 2026
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
_patch_vllm_radio_layerscale_loadermatchesRadioModel.load_weightsas asingle 20-line snippet. The
super_vl_rl_v0.25.1fork that the
nemo-cirl-gymimages ship reshaped that body in three ways:is_legacyelif not is_legacy:native-mapping branch was inserted before theif vllm_key ...tail (it mapslayer_scale{1,2}.lambda1->ls{1,2})Any one of those breaks the match, so the patch takes its
logger.warningpathand returns without applying.
Why it matters
The failure is silent and expensive:
ls1/ls2tensorsmegatron_cfg.freeze_vision_model: truekeeps the vision tower out of the refitload_format=dummy(overriding the recipe'sauto)So this patch's
initializer_factorfill is the only code path that ever setsLayerScale. When it no-ops, every residual branch of the RADIO tower is scaled
by dummy-init noise — with unchanged step time and no exception. The sole
evidence is one line in
ray-driver.log:Measured on a 16-node Super Omni GRPO run — identical recipe, data and
checkpoint, step 1:
train/token_mult_prob_errormean_seq_mult_prob_error87% of the batch was being masked out by the logprob-error threshold.
How
Split the rewrite into two independent edits anchored on text stable across
both variants, so a future reshape of one branch cannot silently disable the
other:
ls1/ls2skip, so explicitLayerScale weights load rather than only being backfilled.
initializer_factorfill beforereturn loaded_params, anchored onloaded_params.add(vllm_key)\n\n return loaded_params(unique in bothvariants).
The required edit now warns and refuses to write a half-applied patch rather
than returning quietly, so the next drift is diagnosable from one log line.
self.config.initializer_factoralso becamegetattr(self.config, "initializer_factor", 1.0)— the checkpoint configs omitthe field and currently rely on
RadioConfig's default.Test plan
Applied against both stock vLLM 0.25.1 and the fork's
radio.py, assertingthat it applies, the result parses, the fill loop lands inside
load_weightsbefore its return, the fork's native-mapping branch and shard-aware
weight_loadersurvive, and a second application is a no-op. All pass on both.End-to-end: 16-node Super Omni GRPO run on the fork-based container with
load_format=dummyunchanged —Could not apply ...went from 40+ occurrencesacross the cluster to 0, and step-1 metrics match the stock-vLLM baseline to
five decimal places (see table above).
🤖 Generated with Claude Code