Skip to content

[TRTLLM-17131][chore] Clean up Muse Spark Open before the mergeback to main - #20638

Merged
brnguyen2 merged 2 commits into
NVIDIA:feat/musespark-openfrom
moraxu:musespark-open/branch-cleanup
Oct 21, 2026
Merged

brnguyen2 merged 2 commits into
NVIDIA:feat/musespark-openfrom
moraxu:musespark-open/branch-cleanup

Conversation

@moraxu

@moraxu moraxu commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

@coderabbitai summary

Description

Cleans up feat/musespark-open before its code is re-cut into PRs against main, so those PRs start from code main's reviewers can accept as is. The default configuration computes the same thing as before, apart from the MegaMoE fix below, which only affects models other than Muse Spark Open.

Dead and duplicate code.

  • tensorrt_llm/_torch/cute_dsl_kernels/mega_moe_nvfp4/ (20 files, 16k lines) is deleted. Nothing in tensorrt_llm/ imports it: main removed the package in [None][perf] update MegaMoE kernels for Blackwell and Rubin #17956, after [None][feat] Support the MegaMoE CuteDSL MoE backend for Qwen3.8-Flash-Next #19323 replaced it with cutedsl_megamoe. Only three constructor tests in test_megamoe_deferred_finalize.py still built its kernel, and they are deleted with it.
  • flashinfer.py defined zero_planned_split_kv_scratch twice. The first definition was shadowed by the second and is removed.
  • guided_decoder.py re-added methods that already existed, with identical bodies, and is restored to its merge-base state.
  • Fmha.validate_metadata had no callers, and fmha/interface.py is restored to its merge-base state.
  • The F811 entries in ruff-legacy-baseline.json that hid the two duplicates are removed.

MegaMoE routing-weight placement. The deferred-finalize change set apply_topk_in_fc1 = not deferred_finalize. That folded the routing weights into the FC1 output, before its NVFP4 quantization, for every model on MEGAMOE_CUTEDSL. Before that change, and on main today, only DeepSeek V4 and V4.1 take that route. The gate is now (DeepSeek V4 or V4.1) and not deferred_finalize: main's condition, plus the deferred-finalize exception.

Muse Spark Open kill switches. The five default-on switches are removed: MUSESPARK_OPEN_FUSED_RESIDUAL, _ROUTING_MEMO, _FROZEN_GATE_CACHE, _FUSED_FINALIZE_AR and _FUSED_PRE_EXPERT_QUANT. They existed to keep each fusion's unfused arm reachable for A/B runs on one image.

  • Every path is still selected by capability, and falls back to the reference composition wherever its kernel does not apply.
  • The tests now reach the fallbacks directly instead of through environment variables: CPU tensors, a layout the kernels do not take, a module whose cache is not filled yet, a cleared routing memo, or a monkeypatched capability check.
  • The opt-in experiments keep their switches for now.

All-reduce strategy. MUSESPARK_OPEN_ALLREDUCE is removed, and so is the model's silent override of the configured allreduce_strategy. Before this change, the attention output projection used NCCL unless the variable named another strategy, and the MoE combine always used NCCL, whatever allreduce_strategy said. Both all-reduces now use the configured strategy, except that AUTO, the default, becomes NCCL, and an explicit NCCL_SYMMETRIC is rejected at model construction, because it is the path that corrupts the attention output projection. The deployment guide states this. AUTO fails this model at TP4 in two ways:

  • AUTO routes the attention output projection's tiny decode messages through the NCCL symmetric-memory all-reduce, which corrupts them: the layer output becomes non-finite and decoding collapses to token 0.
  • On the MoE combine, AUTO's tunable all-reduce hangs engine warmup.

Test fix. test_query_combine_quant_region_matches_bf16_tensor_size failed on GB300 before this change, with CUDA_ERROR_INVALID_CONTEXT. The kernel probe it calls needs a current CUDA context, and the test never created one. It now creates one, and skips without an SM100-family GPU.

Test Coverage

Run on one 4x GB300 node with these changes, and with the deleted package hidden from the installed wheel. The run covered 26 files:

  • 20 Muse Spark Open modeling test files: the ones edited here, plus the MoE, routing, finalize, norm-gate, boundary, pre-attention, format and omni text-path tests;
  • test_megamoe_deferred_finalize.py;
  • the guided-decoder bitmask and rollback tests;
  • test_musespark_serve_extensions.py.

617 passed and 1 skipped. The skip needs a local tokenizer.

New or rewritten cases:

  • test_megamoe_deferred_finalize.py::test_backend_folds_routing_weights_into_fc1_only_for_deepseek_v4 covers five architectures. Without a config, Qwen3-MoE and Muse Spark Open keep the weights after FC2; DeepSeek V4 and V4.1 fold them into FC1. test_backend_deferred_finalize_selects_transformers_route now builds a DeepSeek V4 backend, so it checks that deferred finalize overrides the fold.
  • test_musespark_open_allreduce_strategy.py checks four things:
    • AUTO becomes NCCL;
    • every other strategy is used as configured, whether given as the enum or as the string the LLM args pass;
    • an explicit NCCL_SYMMETRIC is rejected;
    • both __init__s build their AllReduce from the resolver.
  • The kill-switch tests in test_modeling_musespark_open.py, test_musespark_open_pre_expert_quant.py, test_musespark_open_fusion_switches.py, test_musespark_open_preqkv_norm_ordering.py and test_musespark_open_cluster_boundary.py keep their assertions (bit-exact fallbacks, reduction-order bounds, cached-tensor identity), reached without environment variables.

pre-commit passes on the changed files.

PR Checklist

Please review the following before submitting your PR:

  • PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.

  • PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.

  • Test cases are provided for new code paths (see test instructions)

  • If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.

  • Any new dependencies have been scanned for license and vulnerabilities

  • CODEOWNERS updated if ownership changes

  • Documentation updated as needed

  • Update tava architecture diagram if there is a significant design change in PR.

  • The reviewers assigned automatically/manually are appropriate for the PR.

  • Please check this after reviewing the above items as appropriate for this PR.

GitHub Bot Help

To see a list of available CI bot commands, please comment /bot help.

…o main

- Delete the mega_moe_nvfp4 CuTe DSL package. Nothing in tensorrt_llm/
  imports it (main removed it in NVIDIA#17956); drop the three tests that
  built its kernel.
- Delete code that duplicates or shadows existing code: the first
  zero_planned_split_kv_scratch in flashinfer.py, the re-added
  GuidedDecoder methods and the uncalled Fmha.validate_metadata stub.
  Drop the F811 baseline entries that hid the duplicates.
- MegaMoE CuTe DSL: fold routing weights into FC1 only for DeepSeek V4
  and V4.1 again. The deferred-finalize change enabled the fold for
  every model on this backend; deferred finalize still takes the
  post-FC2 route.
- Muse Spark Open: remove the five default-on kill switches
  (MUSESPARK_OPEN_FUSED_RESIDUAL, _ROUTING_MEMO, _FROZEN_GATE_CACHE,
  _FUSED_FINALIZE_AR, _FUSED_PRE_EXPERT_QUANT). Each path is selected
  by capability and falls back to the reference composition; the tests
  reach the fallbacks without environment variables.
- Muse Spark Open: replace MUSESPARK_OPEN_ALLREDUCE with the configured
  allreduce_strategy. AUTO still becomes NCCL on the attention and MoE
  all-reduces; any other strategy is used as configured.

Signed-off-by: Michal Guzek <mguzek@nvidia.com>
@moraxu
moraxu requested review from a team as code owners October 19, 2026 22:59
@yuxianq

yuxianq commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

@yihwang-nv, could you help to review this PR as trt-llm-torch-attention-devs? Thanks~

Assignment explanation
Componentsbackends/flashinfer (45%); fmha/orchestration (25%); integration/attention-tests (30%)
Tagsfeatures:cuda-graphs (100%)
PR effort3 points — The description reports routing-weight and all-reduce corrections, supporting a provisional bugfix classification. Attention effort centers on verifying safe helper/hook removal and fallback-test equivalence; bulk MoE deletions do not drive the score.
Affinity (0–1)0.466 component/tag match
Outstanding points at selection54 → 57 (+3)
Points already reserved on this PR0
Effective weight1
Completed review points (preceding 168 hours)23
Combined points after assignment80
Demand-derived target80.00 points (soft share)
Additional fair share before assignment3.00 points
Share explanationWithin demand-derived share.
Selection rulebalance-first maintenance
Assignment scope (yihwang-nv)all

Affinity and workload are snapshots at selection, not live counters. Existing reservations on this PR are counted once.

@yuxianq
yuxianq requested a review from yihwang-nv October 19, 2026 23:11

@brnguyen2 brnguyen2 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving — the comments below are optional touch-ups, not blockers.

I checked the parts that could break. No references to mega_moe_nvfp4 or the removed MUSESPARK_OPEN_* switches remain in code, docs or examples. The deleted kernel-guard tests had no live counterpart: the cutedsl_megamoe path never forwards skip_topk_reduce, and the backend already rejects deferred finalize with a non-bf16 combine format. Muse Spark Open on MEGAMOE_CUTEDSL has to run with deferred finalize because the scheduler raises otherwise, so apply_topk_in_fc1 stays False for it before and after this change, which matches the description.

The all-reduce change does not affect the documented recipes, because examples/musespark_open/serve_config.yaml, both deployment guides and tests/integration/defs/accuracy/test_musespark_open.py all pin allreduce_strategy: NCCL. For anyone who set a different strategy, though, this is a real behavior change: before, the model silently forced NCCL; now ONESHOT, TWOSHOT, MNNVL, NCCL_SYMMETRIC and the rest reach both all-reduces. When this is re-cut against main, please call that out in the PR description. A line in the deployment guide's allreduce_strategy section saying "AUTO is replaced by NCCL for this model; other values are used as given" would also help.

Two smaller items are inline.

…-reduces

An explicit allreduce_strategy=NCCL_SYMMETRIC selects the symmetric-memory
path that corrupts the attention output projection's decode-step reduce,
which the AUTO-to-NCCL mapping exists to avoid, so model construction now
rejects it. The deployment guide states how allreduce_strategy is
treated for this model.

Signed-off-by: Michal Guzek <mguzek@nvidia.com>
@moraxu
moraxu requested a review from a team as a code owner October 20, 2026 20:42
@moraxu
moraxu requested review from chang-l and laikhtewari October 20, 2026 20:42
@brnguyen2
brnguyen2 merged commit 0288462 into NVIDIA:feat/musespark-open Oct 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants