Skip to content

[None][fix] Pass num_fused_shared_experts in FP8 MoE fallback-tactic path #17362

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
brnguyen2 wants to merge 2 commits into from
Closed

[None][fix] Pass num_fused_shared_experts in FP8 MoE fallback-tactic path #17362

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fcbed76
[None][fix] Pass num_fused_shared_experts in FP8 MoE fallback-tactic …
brnguyen2 Aug 6, 2026
10c3fd7
Address trivial review comments
brnguyen2 Aug 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions tensorrt_llm/_torch/custom_ops/trtllm_gen_custom_ops.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -903,8 +903,8 @@ def __init__(
# that influence tactic validity here. e.g. we are tuning FC1 and FC2 so the routing
# type does not matter
def unique_id(self):
return (self.top_k, self.intermediate_size, self.local_num_experts,
self.act_type)
return (self.top_k, self.num_fused_shared_experts,
self.intermediate_size, self.local_num_experts, self.act_type)

def get_runner(self):
instance_key = ()
Expand All @@ -922,8 +922,8 @@ def get_fallback_tactic(self, hidden_size: int,
the profiling cache. The result depends only on the key below, so
memoize it the same way ``runner_dict`` memoizes the runner.
"""
key = (self.top_k, hidden_size, self.intermediate_size,
self.local_num_experts, num_tokens)
key = (self.top_k, self.num_fused_shared_experts, hidden_size,
self.intermediate_size, self.local_num_experts, num_tokens)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

@zhaoyangwang-nvidia zhaoyangwang-nvidia Aug 7, 2026

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.

This tuple is both the memo key and the positional argument list for get_valid_configs(*key) on line 931 — which is why #15297 updated the explicit get_valid_tactics() call site but missed this one. Could the two be split (pass the arguments explicitly, keep key as just the key), so both call sites look the same to a grep? Fine as a follow-up.

@brnguyen2 brnguyen2 Aug 7, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Agreed. That tuple doing double duty as memo key and positional argument list for get_valid_configs(*key) is exactly why #15297 updated the explicit call site but missed this one. Splitting them so both call sites spell out the arguments (and the key stays just a key) would make this greppable; worth doing as a follow-up. Since the identical fix already merged via #17328, this PR is being closed, so the split would go as a separate cleanup against main.

tactic = FP8BlockScaleMoERunner.fallback_tactic_dict.get(key)
if tactic is None:
tactic = tuple(
Expand Down
Loading