Skip to content

[Knowledge] Component code must scope or restore process-global torch state changes #166

Description

@Dong1017

Submitted per the knowledge-rule template. Proposed by the owner of the MiniMax-H3 encoder incident below; happy to review the resulting knowledge PR.

Owner 类型 / Owner Type

共享代码模块 (component)

Owner / 模块或模型名称

Diffusion components + Model Executor (process-global state)

Owner 依据 / Owner Evidence

Source: process-global state writes live on model/component execution paths. Active cases in the repo today:

  • vllm_omni/model_executor/models/minimax_music3/acoustic.py:172-176 — bare writes of allow_tf32 / set_float32_matmul_precision / cudnn.enabled / enable_cudnn_sdp(False), never restored
  • vllm_omni/model_executor/models/qwen3_tts/qwen3_tts_code2wav.py:699-712 — global TF32 opt-in with an explicit isolation-boundary comment ("intended for deployments where Code2Wav runs in its own Stage1 worker process") — the recommended pattern for intentional process-lifetime switches
  • vllm_omni/diffusion/models/internvla_a1/model_internvla_a1.py:472set_float32_matmul_precision("high")
  • vllm_omni/model_executor/models/nemotron_voicechat/.../duplex_ear_tts.py:980set_default_dtype
  • vllm_omni/model_executor/models/dynin_omni/dynin_omni_token2audio.py:116 — runtime os.environ write
  • Positive reference: vllm_omni/diffusion/models/hunyuan_image3/layers/nvidia/_cudnn.py — scoped context manager around torch.backends.cudnn.benchmark/deterministic (save/restore)

Tests: tests/diffusion/models/minimax_h3/test_minimax_h3_encoder_sdpa.py — regression test asserting the save/restore scope of the cuDNN SDPA global flag (added by the #6566 bugfix).

Maintenance boundary: any Python code that runs on request/component execution paths (diffusion models, model_executor models, shared components). Startup entrypoints (entrypoints/, engine startup) are out of scope — one-time startup-time settings belong to the Configuration rules.

触发 / Trigger

Apply when a PR diff adds or modifies any of the following patterns — grep the changed files first and check each hit:

  • torch.backends.* assignments or enable/disable calls (enable_cudnn_sdp, sdp_kernel, allow_tf32, cudnn.benchmark, cudnn.enabled, ...)
  • torch.set_float32_matmul_precision / torch.set_default_dtype / torch.set_default_device
  • torch.manual_seed / generator reset (global RNG)
  • Runtime (non-startup) os.environ writes
  • mp.set_start_method(..., force=True)
  • monkeypatch / mock.patch of shared objects

必须 / Must

  1. Save the previous value and restore it in a finally block, or wrap the mutation in a scoped context manager — the exact pattern of the vllm-omni #6566 bugfix (PR #6710 "[Bugfix] Scope MiniMax-H3 cuDNN SDPA state", +12/−8):

    old = torch.backends.cuda.cudnn_sdp_enabled()
    torch.backends.cuda.enable_cudnn_sdp(True)
    try:
        ...
    finally:
        torch.backends.cuda.enable_cudnn_sdp(old)

    See also hunyuan_image3/layers/nvidia/_cudnn.py for the context-manager variant.

  2. If the change is intentionally process-lifetime (e.g. TF32 speedup for the whole worker), promote it to an explicit platform/startup-time configuration and document the isolation boundary in a comment at the write site (which process/stage owns it) — following the qwen3_tts code2wav pattern. Never hide it inside component constructors or forward paths.

  3. For SDPA/precision switches, evaluate whether DP/USP rank-asymmetric execution (e.g. text_encoder_tp_size=1 means only rank 0 runs the text encoder) makes the global state inconsistent across ranks, and provide same-seed cross-rank output consistency evidence.

禁止 / Must not

  • Write process-global state without restoring it (bare enable_cudnn_sdp / allow_tf32 / set_float32_matmul_precision / cudnn.* writes). This is the exact defect that caused vllm-omni #6566.
  • Claim distributed safety from single-GPU green tests — the #6566 incident was silent on single GPU and only surfaced under DP2 (latent for 21 days after introduction).
  • Wrap the usage in try/except in a way that can skip the finally-restore.
  • Silently write os.environ from component paths inside a shared worker process.
  • Unconditionally reset global RNG (torch.manual_seed) on per-request paths, polluting sampling of other requests/stages in the same process.

验收 / Acceptance

  • Every added/modified global write site has try/finally or a context manager, or is a documented startup-time configuration with an isolation-boundary comment.
  • Save/restore logic has a regression test (semantics of tests/diffusion/models/minimax_h3/test_minimax_h3_encoder_sdpa.py: the pre-change value is restored).
  • For SDPA/precision-switch PRs: under DP2 with two same-seed concurrent requests, decoded audio is bitwise identical and video SSIM/PSNR meets the alignment threshold (the verification methodology from vllm-omni #6566).

来源 / Sources

  • vllm-project/vllm-omni issue #6566 — incident: MiniMax-H3 text encoder leaked the cuDNN SDPA process-global flag; under DP2 with text_encoder_tp_size=1 only rank 0 executes the encoder, so ranks entered the video VAE with different global SDPA state and the VAE auto mode selected different fused backends — silent numerical divergence (video SSIM 0.982), no error raised
  • vllm-project/vllm-omni PR #5691 — introduced, merged 2026-08-03; dormant 21 days, harmless on single GPU
  • vllm-project/vllm-omni PR #6550 — exposed: author validated a parallel matrix and compared same-seed outputs across DP replicas (SSIM 0.982); a no-LoRA Base control reproduced it, correctly attributing it as a pre-existing bug
  • vllm-project/vllm-omni PR #6710 — the #6566 bugfix, merged 2026-08-29: "[Bugfix] Scope MiniMax-H3 cuDNN SDPA state" — save + try/finally restore, +12/−8; the reference implementation for Must-1
  • vllm-project/vllm-omni PR #6555 — CI follow-up: DP2 smoke test asserts only assert_video_valid + audio stream presence, no cross-rank consistency check — evidence that validity assertions cannot catch silent drift

Owner 确认 / Owner Confirmation

  • I confirm this rule belongs to the owner above and is not valid for only a single PR.
  • I am willing to review the final knowledge-base PR generated from it.

附:Dedup notes vs the existing knowledge base

  • general/review/rules.md REV-1b (conditional-branch symmetry) governs divergent implementations of the same semantics per backend/dtype — not global-state leaks. No existing REV rule covers global state.
  • repos/vllm-omni/review/guides/strict-review-checklist.md already has "anything under get_default_* is inherited by every model on that platform — propose the per-model override, not a global flip" and the platform-twin rule (platform.py changed → grep rocm/xpu). Both are platform-layer defaults; neither covers component code mutating torch process-global state at runtime.
  • The checklist's "Never mutate shared config objects" covers object attributes (hf_config pollution of later readers) vs process-level backend switches (SDPA backend selection drift) — different mechanism; this rule should coexist with, not replace, that one.
  • Placement suggestion: if the maintainer classifies it as diffusion/model-executor component semantics → repos/vllm-omni/components/{diffusion,model-executor}/rules.md; if cross-repo (any torch serving repo shares the risk) → promote to a general REV-4. Submitted at component level; the maintainer decides on promotion.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions