-
-
Notifications
You must be signed in to change notification settings - Fork 16.4k
[Feature] Support recording expert indices for rollout router replay #28284
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.
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
Changes from all commits
d584121
bf247fd
411ea43
fe28549
5bf3ef7
2119bad
9567fe4
58e39b3
1732205
27170a0
4cb1aa8
2cb3259
8df9d70
bfa1b2b
b4f321e
1cad5bc
322d7ce
80bcce9
7406227
fd46e9a
bb059c4
45da3ad
e4ee3ec
d675090
fd3ed05
d3b0d20
b505464
e52228c
5dadff1
0f5e95f
bdf799b
bc17280
aeb469e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1352,6 +1352,7 @@ def __str__(self): | |
| f"disable_custom_all_reduce={self.parallel_config.disable_custom_all_reduce}, " # noqa | ||
| f"quantization={self.model_config.quantization}, " | ||
| f"enforce_eager={self.model_config.enforce_eager}, " | ||
| f"enable_return_routed_experts={self.model_config.enable_return_routed_experts}, " # noqa | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing async scheduling disable for routed experts featureMedium Severity The PR notes explicitly state that async scheduling should be disabled when |
||
| f"kv_cache_dtype={self.cache_config.cache_dtype}, " | ||
| f"device_config={self.device_config.device}, " | ||
| f"structured_outputs_config={self.structured_outputs_config!r}, " | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just call it
return_routed_experts?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine. However, the verl framework currently uses this parameter when integrating with vLLM, so changing it would require coordinated changes in verl as well.