Skip to content

misc - #59

Closed
Qubitium wants to merge 0 commit into
mainfrom
devin/grouped-moe-dispatch
Closed

misc#59
Qubitium wants to merge 0 commit into
mainfrom
devin/grouped-moe-dispatch

Conversation

@Qubitium

@Qubitium Qubitium commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@Qubitium Qubitium self-assigned this Jul 28, 2026
@devin-ai-integration

Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration
devin-ai-integration Bot force-pushed the devin/grouped-moe-dispatch branch from e02fc86 to 51ec4bd Compare July 28, 2026 00:59

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 2 potential issues.

Open in Devin Review

Comment thread defuser/modeling/moe_experts_interface.py Outdated
Comment on lines +238 to +256
for expert_idx in active_experts.tolist():
expert = getattr(self, str(expert_idx))
gate_weights.append(_extract_expert_dense_weight(expert.gate_proj, hidden_dim, intermediate_dim, hidden_states.dtype, device))
up_weights.append(_extract_expert_dense_weight(expert.up_proj, hidden_dim, intermediate_dim, hidden_states.dtype, device))
down_weights.append(_extract_expert_dense_weight(expert.down_proj, intermediate_dim, hidden_dim, hidden_states.dtype, device))
if gate_weights[-1] is None or up_weights[-1] is None or down_weights[-1] is None:
raise RuntimeError(
f"Unable to extract dense weights for expert {expert_idx} in {self.__class__.__name__}"
)
gate_biases.append(_extract_expert_bias(expert.gate_proj, intermediate_dim, hidden_states.dtype, device))
up_biases.append(_extract_expert_bias(expert.up_proj, intermediate_dim, hidden_states.dtype, device))
down_biases.append(_extract_expert_bias(expert.down_proj, hidden_dim, hidden_states.dtype, device))

gate_stack = torch.stack(gate_weights, dim=0)
up_stack = torch.stack(up_weights, dim=0)
down_stack = torch.stack(down_weights, dim=0)

grouped_mm = torch.nn.functional.grouped_mm
gate_out = grouped_mm(selected_hidden, gate_stack, offs=offsets)

@devin-ai-integration devin-ai-integration Bot Jul 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Grouped fast path silently changes numerics for quantized backends that use integer GEMM

_grouped_mm_dequant_experts_forward always dequantizes expert weights to hidden_states.dtype and runs an fp16/bf16 grouped_mm (defuser/modeling/moe_experts_interface.py:238-256). For QuantLinear backends whose per-expert forward normally performs a true low-precision/integer GEMM, this dequantize-then-fp-matmul substitution can produce numerically different outputs than the original per-expert loop. The PR only validates equivalence for TritonV2Linear (max diff 0) and dense nn.Linear bf16. Worth confirming that other quant backends stay within acceptable tolerance since this path becomes the default for CUDA fp16/bf16 inference.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Addressed. The orientation probe also acts as a numeric guard: it compares the projection's native forward() against the dense dequantized GEMM (x @ weight + bias). If the two do not agree within tolerance, the class is marked unsupported and linear_loop_experts_forward falls back to the original per-expert loop. This automatically excludes backends whose integer/fused kernels cannot be reproduced by a dense matmul.

A fake-quantized regression fixture is added to exercise the [in, out] dequantize_weight convention in tests/test_moe_experts_interface.py::test_grouped_mm_experts_fake_quant_square_orientation.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

Open in Devin Review

Comment on lines +482 to +487
except Exception as exc: # noqa: BLE001
if DEBUG_ON:
logger.debug(
f"grouped_mm experts forward failed for {self.__class__.__name__}: {exc}; "
f"falling back to per-expert loop"
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Repeated failing fast-path attempts make the optimized path slower than before

When the grouped fast path throws at runtime, the failure is swallowed (except Exception at defuser/modeling/moe_experts_interface.py:482) without disabling the fast path, so every subsequent call redoes the whole expensive grouped setup and then throws again before falling back.
Impact: On a model whose experts pass the eligibility check but whose grouped kernel actually fails, each layer does the costly dequantize-and-stack work, errors out, and then runs the full per-expert loop anyway on every token, making the "optimization" consistently slower than the original code.

Why the guard never gets disarmed after a runtime failure

_can_use_grouped_mm (defuser/modeling/moe_experts_interface.py:180-217) only validates that dense weights can be extracted for expert0; it never actually invokes grouped_mm. It caches success via self._grouped_mm_ok = True. Thus a module can pass the gate yet still fail inside _grouped_mm_dequant_experts_forward when torch.nn.functional.grouped_mm rejects the shapes/alignment (e.g. a dim that is a multiple of 8 but not the kernel's required multiple, since the check at :204 is % 8).

On failure, the handler at :482-487 logs (only if DEBUG_ON) and falls through to _per_expert_linear_loop_experts_forward, but leaves self._grouped_mm_ok set to True and never sets self._grouped_mm_failed. So the next forward re-enters the fast path (:479), rebuilds all active-expert weight stacks in _project (:319-346), calls grouped_mm again, raises again, and only then runs the per-expert loop — permanently doubling work on the hot MoE path.

Setting self._grouped_mm_failed = True (and clearing _grouped_mm_ok) inside the except handler would latch the fallback after the first runtime failure.

Suggested change
except Exception as exc: # noqa: BLE001
if DEBUG_ON:
logger.debug(
f"grouped_mm experts forward failed for {self.__class__.__name__}: {exc}; "
f"falling back to per-expert loop"
)
except Exception as exc: # noqa: BLE001
self._grouped_mm_ok = False
self._grouped_mm_failed = True
if DEBUG_ON:
logger.debug(
f"grouped_mm experts forward failed for {self.__class__.__name__}: {exc}; "
f"falling back to per-expert loop"
)
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@devin-ai-integration devin-ai-integration Bot changed the title perf(moe): grouped GEMM fast path for linear_loop experts dispatch revert(moe): keep Defuser as per-expert fallback, move grouped GEMM dispatch to GPT-QModel-Ultra Jul 28, 2026
@devin-ai-integration
devin-ai-integration Bot force-pushed the devin/grouped-moe-dispatch branch from 6fa0fb4 to ccee99c Compare July 28, 2026 03:37
@Qubitium Qubitium changed the title revert(moe): keep Defuser as per-expert fallback, move grouped GEMM dispatch to GPT-QModel-Ultra misc Jul 28, 2026
@Qubitium
Qubitium deleted the devin/grouped-moe-dispatch branch July 28, 2026 04:26
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.

1 participant