Skip to content

fix(claim): size the MoE bandwidth floor from active parameters - #1150

Merged
AlexsJones merged 1 commit into
AlexsJones:mainfrom
jayhemnani9910:fix/claim-moe-active-bandwidth
Oct 17, 2026
Merged

AlexsJones merged 1 commit into
AlexsJones:mainfrom
jayhemnani9910:fix/claim-moe-active-bandwidth

Conversation

@jayhemnani9910

Copy link
Copy Markdown
Contributor

What

llmfit claim sets the bandwidth floor as min_tps × weights / efficiency, where weights is the whole model. For a MoE model, decode reads only the active experts per token, and fit.rs estimate_tps already sizes speed from active_parameters. So claim asks MoE models for far more bandwidth than fit says they need. For large MoE models no device can match:

$ llmfit claim deepseek-ai/DeepSeek-V3
# fit:    tok/s ≈ bandwidth × 55% / 397.0 GB  ⇒  bandwidth ≥ 14438 GB/s for ≥ 20 tok/s
$ llmfit claim Qwen/Qwen3-30B-A3B
# fit:    tok/s ≈ bandwidth × 55% / 17.7 GB  ⇒  bandwidth ≥ 644 GB/s for ≥ 20 tok/s

Same on v1.1.17 and main. #807 fixed the same mismatch in plan.

Change

fit_bounds uses active_parameters × quant_bpp(quant) as the bytes read per token when the model is MoE and has an active count, and all weights otherwise (the same fallback as estimate_tps). The memory floor still covers all weights. The YAML comment shows the per-token size and marks it active:

$ llmfit claim deepseek-ai/DeepSeek-V3
# fit:    tok/s ≈ bandwidth × 55% / 21.5 GB active  ⇒  bandwidth ≥ 781 GB/s for ≥ 20 tok/s
$ llmfit claim Qwen/Qwen3-30B-A3B
# fit:    tok/s ≈ bandwidth × 55% / 1.9 GB active  ⇒  bandwidth ≥ 70 GB/s for ≥ 20 tok/s

Dense models are unchanged (Qwen3-8B stays at 173 GB/s). This keeps the simple bandwidth × efficiency / bytes formula; it does not copy estimate_tps's Tier 1 split or MoE overhead factors. It also uses the catalog's active_parameters as is, like estimate_tps does, so a repack with an undercounted active figure gets a low floor in both.

Tests

New bounds_use_active_parameters_for_moe_bandwidth (30B total / 3B active: memory 18 Gi, bandwidth 64; without an active count, 633). It fails on main (left: 633, right: 64).

cargo test: all green (llmfit-core lib 855 passed vs 854 on main). cargo fmt --check clean. cargo clippy --all-targets --all-features: 61 warnings, the same set as main.

Copilot AI balanced review requested due to automatic review settings October 17, 2026 05:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The change appears safe to merge.

Summary

The change sizes MoE bandwidth floors from active parameters while keeping enough memory for all weights.

  • MoE claims set bandwidth floors from active weights.

jayhemnani9910 explicitly leaves the extra MoE factors from estimate_tps out of scope and acknowledges using catalog active counts unchanged.

Reviews (1) · Last reviewed commit: "fix(claim): size the MoE bandwidth floor..." · Reviewed by Greptile

@AlexsJones AlexsJones left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Nice catch, claim asking DeepSeek-V3 for 14 TB/s while fit happily runs it was a clear sign the two had drifted.

Checked at f6c95cc:

  • fit_bounds now sizes the bandwidth floor from active_parameters × quant_bpp(quant) only when is_moe and an active count is present, and falls back to all weights otherwise. That's the same active_parameters.filter(|_| model.is_moe) gate estimate_tps uses in fit.rs, so the two stay in step.
  • The memory floor still uses weights_gb (all experts), which is right: the weights have to be resident even if only some are read per token.
  • read_gb uses the same quant (including a --quant override) as the weights estimate, so an override moves both bounds together.
  • The new test pins both branches (64 GB/s with an active count, 633 without), and the header only says active when the read size is actually smaller than the weights.

As you note, this floor skips estimate_tps's MoE overhead factor, so it's a little more permissive than fit for MoE models. That's fine for a scheduling floor and matches how the dense case already works.

CI is green on all three platforms plus fmt and clippy. Good to merge.

@AlexsJones
AlexsJones merged commit 2491243 into AlexsJones:main Oct 17, 2026
9 checks passed
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.

3 participants