Repository navigation
[MM] Move Nemotron VL image normalization to device - #57924
Conversation
Co-authored-by: OpenAI Codex <codex@openai.com> Signed-off-by: Jakub Zakrzewski <jzakrzewski@nvidia.com>
Co-authored-by: OpenAI Codex <codex@openai.com> Signed-off-by: Jakub Zakrzewski <jzakrzewski@nvidia.com>
Co-authored-by: OpenAI Codex <codex@openai.com> Signed-off-by: Jakub Zakrzewski <jzakrzewski@nvidia.com>
fdf8c4d to
ee18b49
Compare
|
Documentation preview: https://vllm--57924.org.readthedocs.build/en/57924/ |
|
Hey @jzakrzew - can you please run a baseline correctness image only and a video eval for nemotron 3 nano omni [the newer nano vl variant] with the relevant snippet e.g. https://huggingface.co/nvidia/Nemotron-3-Nano-Omni-30B-A3B-Reasoning-NVFP4#general-invocation-1%C3%97gpu-eg-1%C3%97b200, and then run with this layered on top, and prove the accuracy is on par? Then, append the script that runs these evals to the description for future reference. |
| |--------------------|--------------------------------------|-------------------------------------------------------------| | ||
| | `qwen2-vl` | `Qwen2VLForConditionalGeneration` | `Qwen/Qwen2-VL-2B-Instruct`, etc. | | ||
| | `qwen2.5-vl` | `Qwen2_5_VLForConditionalGeneration` | `Qwen/Qwen2.5-VL-3B-Instruct`, etc. | | ||
| | `nano-nemotron-vl` | `NemotronH_Nano_VL_V2` | `nvidia/NVIDIA-Nemotron-Nano-12B-v2-VL-BF16`, etc. | |
There was a problem hiding this comment.
There was a problem hiding this comment.
Added to the table.
| size: tuple[int, int] | None = None, | ||
| norm_mean: torch.Tensor | None = None, | ||
| norm_std: torch.Tensor | None = None, | ||
| do_cpu_normalize: bool = True, |
There was a problem hiding this comment.
I'd support removing the optionality completely and just forcing gpu norm as part of the pr. This would simplify this change a lot. Any specific reason you would want this branching?
There was a problem hiding this comment.
although only if/when safe to do so, https://github.com/vllm/project/vllm/commit/2fa490470ddf0fe578f7b70cd3aa8f6c1e06b2c6 - I see it might not be universally supported yet? Or was that just August 6 and now there are no caveats?
There was a problem hiding this comment.
Bottom line, please check if this flag is still necessary or we can go all in. The answer may be yes of course
There was a problem hiding this comment.
this is just a function-level default value to preserve the API. The actual value used in normal execution path comes from do_normalize argument to NanoNemotronVLProcessor.__init__
There was a problem hiding this comment.
Keeping the CPU path. main forces mm_device_do_normalize off under the Rust frontend, so this model still needs it. The global --no-mm-device-do-normalize from #50411 is the switch, and it's what the before/after evals in the PR conversation use: "off" is bit-identical to main. The processor-level default only preserves the API.
|
Also: Please get perf numbers for same table for https://huggingface.co/nvidia/Nemotron-3-Nano-Omni-30B-A3B-Reasoning-NVFP4 as that is the new and mainly used checkpoint of this arch |
| rescale_factor=1.0 / 255.0, | ||
| ) | ||
| if multimodal_config.mm_device_do_normalize | ||
| else FusedInputNorm.identity() |
There was a problem hiding this comment.
I think rather than have this identity thing, if mm_devide_do_normalize can actually be False, have input norm be FusedInputNorm | None, and then only call it if input_norm is not None, so as not to incur a perf hit for non device normalized runs?
There was a problem hiding this comment.
Done: input_norm is FusedMMInputNorm | None and is skipped when off. build_mm_input_norm() doesn't fit this checkpoint: it stores norm_mean/norm_std, so the helper would return the identity.
| else: | ||
| assert pixel_values.ndim == 3 | ||
| flattened = pixel_values.reshape(-1, shape[-1]) | ||
| normalized = self.input_norm(flattened, self.llm_dtype) |
There was a problem hiding this comment.
Can you prove this produces the same dtype we had before post-norm?
There was a problem hiding this comment.
Or I guess the question is - this seems to change when the norm happens at least, no? So the accuracy evals you will run will prove whether or not that changes anything
There was a problem hiding this comment.
Both paths emit llm_dtype (bf16). The CPU path is bit-identical to main (110/110 tensors), and without a resize the device norm is bit-identical to the CPU norm; with a resize, the only difference is the uint8 rounding. c8cfbb1 adds a test that runs the processors' uint8 output through the device norm and checks the dtype against the CPU path. On accuracy, the evals in the PR conversation show no significant change on any set.
Co-authored-by: Codex <codex@openai.com> Signed-off-by: Jakub Zakrzewski <jzakrzewski@nvidia.com>
Co-authored-by: Codex <codex@openai.com> Signed-off-by: Jakub Zakrzewski <jzakrzewski@nvidia.com>
…malization Assisted-by: Codex Signed-off-by: Jakub Zakrzewski <jzakrzewski@nvidia.com>
|
This pull request has merge conflicts that must be resolved before it can be |
…malization Signed-off-by: Jakub Zakrzewski <jzakrzewski@nvidia.com>
…malization Signed-off-by: Brandon Pelfrey <bpelfrey@nvidia.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
FusedMMInputNorm dispatches to a Triton kernel on GPU platforms, so feeding it CPU tensors fails there. Move the module and pixels to the platform device, as tests/model_executor/layers/test_mm_input_norm.py does. Also give the vision layernorm helper model an input_norm, which extract_feature now reads. Signed-off-by: Brandon Pelfrey <bpelfrey@nvidia.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Run the processors' uint8 output through the model's device normalization and check it against the CPU-normalized pixels: same dtype, values within the resize rounding, for the image, video and dynamic-resolution layouts. Signed-off-by: Brandon Pelfrey <bpelfrey@nvidia.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@netanel-haber @jzakrzew I'm helping Jakub complete the remaining testing for this PR and get it merged. Here are the before/after evals and the NVFP4 perf numbers you asked for; the script to reproduce them is attached. Accuracy. Nemotron-3-Nano-Omni NVFP4 with the model card invocation, on one build of this PR: two servers that differ only by
No set shows a significant drop. BF16 pooled over 12,988 questions: −0.11 [−0.25, +0.03]. The Video-MME long interval is wide (300 videos); a second build gave −0.11. Perf (NVFP4, H100).
Next: I pushed two test commits (085ebd0, c8cfbb1).
I've also replied on the inline threads. Ready for re-review. Reproduce: attached as |
|
eval_57924.py |
| | `llama-nemotron-vl-embed` | `LlamaNemotronVLForEmbedding` | `nvidia/llama-nemotron-embed-vl-1b-v2` | | ||
| | `llama-nemotron-vl-rerank` | `LlamaNemotronVLForSequenceClassification` | `nvidia/llama-nemotron-rerank-vl-1b-v2` | | ||
| | `mistral3` | `Mistral3ForConditionalGeneration` | `mistralai/Mistral-Small-3.2-24B-Instruct-2506`, etc. | | ||
| | `nano-nemotron-vl` | `NemotronH_Nano_VL_V2` | `nvidia/Nemotron-3-Nano-Omni-30B-A3B-Reasoning-NVFP4` | |
There was a problem hiding this comment.
Nit: registry.py maps four architectures to NemotronH_Nano_VL_V2 (L542-545), including NemotronH_Super_Omni_Reasoning_V3, so the Super checkpoints also get device normalization from this PR. Could additional rows be added for super models as well?
There was a problem hiding this comment.
seems like NemotronH_Super_Omni_Reasoning_V3 has no models in HF? I added the other two.
List Nano Omni and Super VL checkpoint architectures separately without widening the table. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Jakub Zakrzewski <jzakrzewski@nvidia.com>
9976267 to
8880629
Compare
|
This pull request has merge conflicts that must be resolved before it can be |
…malization Signed-off-by: Jakub Zakrzewski <jzakrzewski@nvidia.com> Co-authored-by: Codex <noreply@openai.com> # Conflicts: # tests/models/multimodal/test_nano_nemotron_vl.py
|
LGTM - all of my review items were completed. |
|
/ci run |
|
✅ Triggered Buildkite CI #98333 for commit ℹ️ This PR is 11 commits behind upstream |

Purpose
Move image and video rescaling/normalization for the Nano Nemotron VL models from CPU preprocessing to the model device. This follows the shared GPU-side normalization mechanism introduced in #50411.
Before this change, the processor converted raw
uint8pixels tofloat32before bicubic interpolation, normalized them on the CPU, and returned pixels in the model dtype. This made resizing more expensive and doubled the amount of pixel data transferred forbfloat16models.This PR:
NemotronH_Nano_VL_V2intomm_device_do_normalizeuint8through preprocessing and transfer (restoring pre-Optimize nemotron VL image/video preprocessing #40283 numerical behavior)FusedInputNormimmediately before the C-RADIO encoderNCHWinputs and dynamic-resolution packed patchesBicubic interpolation remains under
torch.compile. Both normalization modes, with and without resizing, produce one Dynamo graph each with zero graph breaks under the Inductor backend.Performance
The renderer and executor are separate processes, so the timings below are
reported per stage and are not added together. Executor work may overlap with
subsequent renderer work.
Setup:
bfloat16One 1536 x 1536 image
32 video frames at 512 x 512
Test Plan
The tests cover:
uint8through compiled bicubic interpolation when device normalization is enabledbfloat16agreement between CPU normalization andFusedInputNormNCHWand dynamic-resolution packed-patch layoutsTest Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.