-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[feat](kt-kernel): opt-in skip of CPU copies for GPU-resident experts #2085
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -211,6 +211,7 @@ class AMX_MOE_TP : public AMX_MOE_BASE<T, AMX_MOE_TP<T>> { | |
| config_.expert_num, nullptr, | ||
| [this, physical_to_logical_map](int expert_id) { | ||
| // printf("Load layer %d [%d/%d]\n", config_.layer_idx, expert_id, config_.expert_num); | ||
| if (config_.skip_gpu_expert_cpu_copy && config_.should_skip_expert((int64_t)expert_id)) return; | ||
|
Contributor
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. There are two other loader paths in
Please add the same [this, physical_to_logical_map, prefix](int task_id) {
int64_t expert_idx = task_id / mat_type_all;
if (config_.skip_gpu_expert_cpu_copy && config_.should_skip_expert((int64_t)expert_idx)) return;
expert_idx = expert_map(physical_to_logical_map, expert_idx);
...
Author
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. Good catch — fixed in 54e0943: the config_.load work-stealing lambda now early-returns via should_skip_expert(expert_idx) (the same physical index used to access gate/up/down_bb_) before any buffer deref. |
||
| uint64_t logical_expert_id = expert_map(physical_to_logical_map, expert_id); | ||
| { | ||
| size_t scale_size = config_.intermediate_size * sizeof(float); | ||
|
|
@@ -254,6 +255,9 @@ class AMX_MOE_TP : public AMX_MOE_BASE<T, AMX_MOE_TP<T>> { | |
| config_.expert_num * mat_type_all * mat_split, | ||
| [this, physical_to_logical_map, prefix, mat_type_all, mat_split](int task_id) { | ||
| int64_t expert_idx = task_id / (mat_type_all * mat_split); | ||
| if (config_.skip_gpu_expert_cpu_copy && config_.should_skip_expert(expert_idx)) { | ||
| return; | ||
| } | ||
| uint64_t logical_expert_id = expert_map(physical_to_logical_map, expert_idx); | ||
| uint8_t mat_class = (task_id % (mat_type_all * mat_split)) / mat_split; | ||
| uint8_t mat_split_idex = task_id % mat_split; | ||
|
|
@@ -291,6 +295,7 @@ class AMX_MOE_TP : public AMX_MOE_BASE<T, AMX_MOE_TP<T>> { | |
| [this, nth, physical_to_logical_map](int task_id) { | ||
| int64_t expert_idx = task_id / nth; | ||
| uint64_t logical_expert_id = expert_map(physical_to_logical_map, expert_idx); | ||
| if (config_.skip_gpu_expert_cpu_copy && config_.should_skip_expert((int64_t)logical_expert_id)) return; | ||
| int ith = task_id % nth; | ||
| // gate part | ||
| gate_bb_[logical_expert_id]->from_mat( | ||
|
|
@@ -309,6 +314,7 @@ class AMX_MOE_TP : public AMX_MOE_BASE<T, AMX_MOE_TP<T>> { | |
| [this, nth, physical_to_logical_map](int task_id) { | ||
| int64_t expert_idx = task_id / nth; | ||
| uint64_t logical_expert_id = expert_map(physical_to_logical_map, expert_idx); | ||
| if (config_.skip_gpu_expert_cpu_copy && config_.should_skip_expert((int64_t)logical_expert_id)) return; | ||
| int ith = task_id % nth; | ||
| // down part | ||
| down_bb_[logical_expert_id]->from_mat( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -566,6 +566,7 @@ class AMX_MXFP8_MOE_TP : public AMX_MOE_BASE<T, AMX_MXFP8_MOE_TP<T>> { | |
| nth * config_.expert_num, nullptr, | ||
| [this, nth, physical_to_logical_map](int task_id) { | ||
| uint64_t expert_idx = task_id / nth; | ||
| if (config_.skip_gpu_expert_cpu_copy && config_.should_skip_expert((int64_t)expert_idx)) return; | ||
|
Contributor
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. In If To prevent silent crashes, please add a safety check at the beginning of if (gate_bb_[expert_id] == nullptr) {
throw std::runtime_error("Cannot write weights to buffer for skipped/GPU-resident expert");
}
Author
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. This path is safe by construction: write_weights_to_buffer is only reached from the sglang full-GPU-prefill pipeline, whose mask-aware callers (_prepare_weight_fp8/_prepare_weight_mxfp4) partition experts and submit ONLY cpu_expert_ids to the wrapper — a skipped (GPU-resident) expert never reaches it. Adding a null-check inside would silently mask a caller bug rather than surface it, so I have left it unguarded deliberately. Happy to add an assert instead if maintainers prefer. |
||
| uint64_t logical_expert_id = expert_map(physical_to_logical_map, expert_idx); | ||
| int ith = task_id % nth; | ||
| // FP8: no >> 1 (1 byte/element, not nibble-packed) | ||
|
|
@@ -580,6 +581,7 @@ class AMX_MXFP8_MOE_TP : public AMX_MOE_BASE<T, AMX_MXFP8_MOE_TP<T>> { | |
| nth * config_.expert_num, nullptr, | ||
| [this, nth, physical_to_logical_map](int task_id) { | ||
| uint64_t expert_idx = task_id / nth; | ||
| if (config_.skip_gpu_expert_cpu_copy && config_.should_skip_expert((int64_t)expert_idx)) return; | ||
| uint64_t logical_expert_id = expert_map(physical_to_logical_map, expert_idx); | ||
| int ith = task_id % nth; | ||
| size_t weight_offset = logical_expert_id * config_.hidden_size * config_.intermediate_size; | ||
|
|
@@ -593,6 +595,7 @@ class AMX_MXFP8_MOE_TP : public AMX_MOE_BASE<T, AMX_MXFP8_MOE_TP<T>> { | |
| config_.expert_num, nullptr, | ||
| [this, physical_to_logical_map](int task_id) { | ||
| uint64_t expert_idx = task_id; | ||
| if (config_.skip_gpu_expert_cpu_copy && config_.should_skip_expert((int64_t)expert_idx)) return; | ||
| uint64_t logical_expert_id = expert_map(physical_to_logical_map, expert_idx); | ||
| size_t scale_count = | ||
| (static_cast<size_t>(config_.intermediate_size) * config_.hidden_size) / config_.quant_config.group_size; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -132,6 +132,7 @@ def __new__( | |
| # Inference-specific parameters | ||
| cpu_save: bool = False, | ||
| max_deferred_experts_per_token: Optional[int] = None, | ||
| skip_gpu_expert_cpu_copy: bool = False, | ||
| # Mode and method selection | ||
| method: str = "AMXINT4", | ||
| numa_nodes: Optional[List[int]] = None, | ||
|
|
@@ -221,6 +222,7 @@ def __new__( | |
| chunked_prefill_size=chunked_prefill_size, | ||
| cpu_save=cpu_save, | ||
| max_deferred_experts_per_token=max_deferred_experts_per_token, | ||
| skip_gpu_expert_cpu_copy=skip_gpu_expert_cpu_copy, | ||
| method=method, | ||
| numa_nodes=numa_nodes, | ||
| swiglu_limit=swiglu_limit, | ||
|
|
@@ -235,6 +237,15 @@ def __new__( | |
| f"mode='sft' (method={method!r}); SFT backends do not " | ||
| f"implement the V4-2604B clamp." | ||
| ) | ||
| # SFT trains every expert on CPU and needs full host copies of all | ||
| # expert weights, including GPU-resident ones; the inference-only | ||
| # CPU-copy skip must never reach a training run. | ||
| if skip_gpu_expert_cpu_copy: | ||
| raise ValueError( | ||
| "skip_gpu_expert_cpu_copy=True is not supported in " | ||
| "mode='sft'; SFT requires full CPU copies of all expert " | ||
| "weights (GPU-resident experts included)." | ||
| ) | ||
| return _create_sft_wrapper( | ||
| layer_idx=layer_idx, | ||
| num_experts=num_experts, | ||
|
|
@@ -318,6 +329,7 @@ def _create_inference_wrapper( | |
| chunked_prefill_size: int, | ||
| cpu_save: bool, | ||
| max_deferred_experts_per_token: Optional[int], | ||
| skip_gpu_expert_cpu_copy: bool, | ||
| method: str, | ||
| numa_nodes: Optional[List[int]] = None, | ||
| swiglu_limit: float = 0.0, | ||
|
|
@@ -364,6 +376,8 @@ def _create_inference_wrapper( | |
| f"environment while the current launch does not actually use " | ||
| f"MXFP4/MXFP8 weights — either unset the env or pass --kt-method MXFP4/MXFP8." | ||
| ) | ||
| if backend_cls is NativeMoEWrapper: | ||
| extra_kwargs["skip_gpu_expert_cpu_copy"] = skip_gpu_expert_cpu_copy | ||
|
Comment on lines
+379
to
+380
Contributor
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. While In To fix this, please:
Author
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. Intentional scoping, not an oversight: the user-facing flag is resolved (sglang side, kvcache-ai/sglang#64) into an effective flag that is only true for mask-static paths — MXFP4, or MXFP8 with dynamic expert update off — which are NativeMoEWrapper paths. AMXINT4/8 can be demoted at runtime by the dynamic-update mask rewrite, so an expert whose CPU copy was skipped could later be needed on CPU; wiring the flag through AMXMoEWrapper without that guarantee would trade a safe no-op for a correctness hazard. If a mask-static AMX mode lands later, plumbing it through is a small follow-up. |
||
| return backend_cls( | ||
| layer_idx=layer_idx, | ||
| num_experts=num_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.
The
config_.savebranches inawq-moe.hpp(around lines 583 and 631) are missingshould_skip_expertguards.When
config_.saveis enabled, the loops iterate over all experts from0toconfig_.expert_numand callwrite_weightswithgate_bb_[expert_idx].get(). Ifskip_gpu_expert_cpu_copyis true,gate_bb_[expert_idx]isnullptrfor skipped experts, causing a segfault whenwrite_weightsattempts to accessbuffer->b.Please add a guard to skip saving weights for skipped 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.
Fixed in 54e0943: both config_.save loops in awq-moe.hpp (OFFLINE ~:583 and ONLINE ~:631) now continue on should_skip_expert(expert_idx).