Add H100 Hopper throughput autotune and server plan integration - #34
Add H100 Hopper throughput autotune and server plan integration#34Jackson57279 wants to merge 2 commits into
Conversation
…plans. Introduce GpuFamily::H100 cluster profiles, tier-9 autotune rules for single-GPU Hopper throughput (paged KV, FlashAttention-3, CUDA graphs), and apply those plans when loading the server so batch mode, KV dtype, and scheduler limits follow autotune output.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 17 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="oxidize-server/src/main.rs">
<violation number="1" location="oxidize-server/src/main.rs:45">
P3: Startup logs can report the wrong batch mode when autotune overrides `--batch-mode`, which makes production debugging and rollout validation misleading. Consider logging the computed `batch_mode` value (or emitting both requested/effective modes) so telemetry matches the runtime path.</violation>
</file>
<file name="oxidize-server/src/runtime/paged.rs">
<violation number="1" location="oxidize-server/src/runtime/paged.rs:131">
P2: Requests can stall indefinitely when prefill chunk size is set to 0, because the scheduler never makes prefill progress with a zero chunk. Clamping `prefill_chunk_size` to at least 1 when building `SchedulerConfig` avoids this no-progress state.</violation>
</file>
<file name="oxidize-core/src/autotune/rules.rs">
<violation number="1" location="oxidize-core/src/autotune/rules.rs:559">
P2: H100 throughput overrides are applied even for partial GPU offload, because the new guard only checks `n_gpu_layers > 0` instead of full offload. That can force paged/FA3/cuda-graphs/decode-batch settings on mixed CPU+GPU runs where the profile assumptions do not hold; consider gating this tier to `n_gpu_layers == model.layer_count`.</violation>
<violation number="2" location="oxidize-core/src/autotune/rules.rs:601">
P2: The conditional `if matches!(plan.weight_plan, WeightPlan::W4A16) && model.layer_count >= 48` block at the end of `tier9_hopper_throughput` always sets `plan.max_decode_batch = 16`, but that value was already set unconditionally earlier in the same function. The conditional is a no-op — it does not alter `max_decode_batch` regardless of weight plan or layer count.
The presence of the guard (`W4A16 && layer_count >= 48`) suggests it was meant to apply a *different* batch size for large W4A16 models (likely a higher value like 32) but the literal `16` was reused inadvertently. As written, the entire `if` block can be removed without changing behavior. If a larger batch size for large W4A16 models was intended, the conditional value should be corrected.</violation>
</file>
<file name="oxidize-core/src/cluster/gpu_cluster.rs">
<violation number="1" location="oxidize-core/src/cluster/gpu_cluster.rs:134">
P2: H100 nodes will be labeled as not MIG-capable even though Hopper supports MIG, which can skew scheduling/capability decisions that rely on `nvidia.com/mig.capable`. The H100 profile would be safer with `mig_capable: true` (and policy-specific MIG enablement kept separate).</violation>
</file>
<file name="oxidize-cpp/include/oxidize/autotune.hpp">
<violation number="1" location="oxidize-cpp/include/oxidize/autotune.hpp:23">
P2: `ModelFingerprint` fields `layer_count`, `num_kv_heads`, and `head_dim` are declared but never populated or consumed. `fingerprint_model_file()` only sets `file_size_bytes`, and `plan_cpu()` never references the new fields. Readers of the struct (and the test that laboriously sets them) will reasonably assume these values influence the tuning plan, but they don't. Consider either (a) populating them from GGUF metadata in `fingerprint_model_file()` and using them in H100/KV-sizing logic, or (b) removing them until a consumer exists.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| SchedulerConfig { | ||
| max_num_batched_tokens: args.prefill_batch_size, | ||
| prefill_chunk_size: planned_prefill_chunk.min(args.prefill_batch_size.max(1)), |
There was a problem hiding this comment.
P2: Requests can stall indefinitely when prefill chunk size is set to 0, because the scheduler never makes prefill progress with a zero chunk. Clamping prefill_chunk_size to at least 1 when building SchedulerConfig avoids this no-progress state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At oxidize-server/src/runtime/paged.rs, line 131:
<comment>Requests can stall indefinitely when prefill chunk size is set to 0, because the scheduler never makes prefill progress with a zero chunk. Clamping `prefill_chunk_size` to at least 1 when building `SchedulerConfig` avoids this no-progress state.</comment>
<file context>
@@ -99,3 +88,121 @@ pub fn build_paged_runtime(args: &Args, runtime: Arc<ModelRuntime>) -> Arc<Paged
+
+ SchedulerConfig {
+ max_num_batched_tokens: args.prefill_batch_size,
+ prefill_chunk_size: planned_prefill_chunk.min(args.prefill_batch_size.max(1)),
+ max_num_running_seqs,
+ }
</file context>
| prefill_chunk_size: planned_prefill_chunk.min(args.prefill_batch_size.max(1)), | |
| prefill_chunk_size: planned_prefill_chunk.max(1).min(args.prefill_batch_size.max(1)), |
| model: &ModelFingerprint, | ||
| plan: &mut TuningPlan, | ||
| ) { | ||
| if inv.gpu_family != Some(crate::gpu_cluster::GpuFamily::H100) || plan.n_gpu_layers == 0 { |
There was a problem hiding this comment.
P2: H100 throughput overrides are applied even for partial GPU offload, because the new guard only checks n_gpu_layers > 0 instead of full offload. That can force paged/FA3/cuda-graphs/decode-batch settings on mixed CPU+GPU runs where the profile assumptions do not hold; consider gating this tier to n_gpu_layers == model.layer_count.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At oxidize-core/src/autotune/rules.rs, line 559:
<comment>H100 throughput overrides are applied even for partial GPU offload, because the new guard only checks `n_gpu_layers > 0` instead of full offload. That can force paged/FA3/cuda-graphs/decode-batch settings on mixed CPU+GPU runs where the profile assumptions do not hold; consider gating this tier to `n_gpu_layers == model.layer_count`.</comment>
<file context>
@@ -501,6 +551,67 @@ fn tier8_pipeline(inv: &HardwareInventory, model: &ModelFingerprint, plan: &mut
+ model: &ModelFingerprint,
+ plan: &mut TuningPlan,
+) {
+ if inv.gpu_family != Some(crate::gpu_cluster::GpuFamily::H100) || plan.n_gpu_layers == 0 {
+ return;
+ }
</file context>
| memory_mib: 81_920, | ||
| tdp_watts: 700, | ||
| nvlink: true, | ||
| mig_capable: false, |
There was a problem hiding this comment.
P2: H100 nodes will be labeled as not MIG-capable even though Hopper supports MIG, which can skew scheduling/capability decisions that rely on nvidia.com/mig.capable. The H100 profile would be safer with mig_capable: true (and policy-specific MIG enablement kept separate).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At oxidize-core/src/cluster/gpu_cluster.rs, line 134:
<comment>H100 nodes will be labeled as not MIG-capable even though Hopper supports MIG, which can skew scheduling/capability decisions that rely on `nvidia.com/mig.capable`. The H100 profile would be safer with `mig_capable: true` (and policy-specific MIG enablement kept separate).</comment>
<file context>
@@ -114,6 +124,18 @@ pub fn profile(family: GpuFamily) -> GpuProfile {
+ memory_mib: 81_920,
+ tdp_watts: 700,
+ nvlink: true,
+ mig_capable: false,
+ time_slice_replicas: 1,
+ network_class: "infiniband",
</file context>
|
|
||
| struct ModelFingerprint { | ||
| uint64_t file_size_bytes = 0; | ||
| size_t layer_count = 0; |
There was a problem hiding this comment.
P2: ModelFingerprint fields layer_count, num_kv_heads, and head_dim are declared but never populated or consumed. fingerprint_model_file() only sets file_size_bytes, and plan_cpu() never references the new fields. Readers of the struct (and the test that laboriously sets them) will reasonably assume these values influence the tuning plan, but they don't. Consider either (a) populating them from GGUF metadata in fingerprint_model_file() and using them in H100/KV-sizing logic, or (b) removing them until a consumer exists.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At oxidize-cpp/include/oxidize/autotune.hpp, line 23:
<comment>`ModelFingerprint` fields `layer_count`, `num_kv_heads`, and `head_dim` are declared but never populated or consumed. `fingerprint_model_file()` only sets `file_size_bytes`, and `plan_cpu()` never references the new fields. Readers of the struct (and the test that laboriously sets them) will reasonably assume these values influence the tuning plan, but they don't. Consider either (a) populating them from GGUF metadata in `fingerprint_model_file()` and using them in H100/KV-sizing logic, or (b) removing them until a consumer exists.</comment>
<file context>
@@ -12,16 +13,35 @@ struct HardwareInventory {
struct ModelFingerprint {
uint64_t file_size_bytes = 0;
+ size_t layer_count = 0;
+ size_t num_kv_heads = 0;
+ size_t head_dim = 0;
</file context>
| } | ||
| } | ||
|
|
||
| if matches!(plan.weight_plan, WeightPlan::W4A16) && model.layer_count >= 48 { |
There was a problem hiding this comment.
P2: The conditional if matches!(plan.weight_plan, WeightPlan::W4A16) && model.layer_count >= 48 block at the end of tier9_hopper_throughput always sets plan.max_decode_batch = 16, but that value was already set unconditionally earlier in the same function. The conditional is a no-op — it does not alter max_decode_batch regardless of weight plan or layer count.
The presence of the guard (W4A16 && layer_count >= 48) suggests it was meant to apply a different batch size for large W4A16 models (likely a higher value like 32) but the literal 16 was reused inadvertently. As written, the entire if block can be removed without changing behavior. If a larger batch size for large W4A16 models was intended, the conditional value should be corrected.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At oxidize-core/src/autotune/rules.rs, line 601:
<comment>The conditional `if matches!(plan.weight_plan, WeightPlan::W4A16) && model.layer_count >= 48` block at the end of `tier9_hopper_throughput` always sets `plan.max_decode_batch = 16`, but that value was already set unconditionally earlier in the same function. The conditional is a no-op — it does not alter `max_decode_batch` regardless of weight plan or layer count.
The presence of the guard (`W4A16 && layer_count >= 48`) suggests it was meant to apply a *different* batch size for large W4A16 models (likely a higher value like 32) but the literal `16` was reused inadvertently. As written, the entire `if` block can be removed without changing behavior. If a larger batch size for large W4A16 models was intended, the conditional value should be corrected.</comment>
<file context>
@@ -501,6 +551,67 @@ fn tier8_pipeline(inv: &HardwareInventory, model: &ModelFingerprint, plan: &mut
+ }
+ }
+
+ if matches!(plan.weight_plan, WeightPlan::W4A16) && model.layer_count >= 48 {
+ plan.max_decode_batch = 16;
+ }
</file context>
| let (model_opt, paged_opt) = if args.batch_mode == BatchMode::Paged { | ||
| if let Some(runtime) = model { | ||
| let paged = build_paged_runtime(&args, runtime.clone()); | ||
| let batch_mode = effective_batch_mode(&args, loaded_model.autotune_plan.as_ref()); |
There was a problem hiding this comment.
P3: Startup logs can report the wrong batch mode when autotune overrides --batch-mode, which makes production debugging and rollout validation misleading. Consider logging the computed batch_mode value (or emitting both requested/effective modes) so telemetry matches the runtime path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At oxidize-server/src/main.rs, line 45:
<comment>Startup logs can report the wrong batch mode when autotune overrides `--batch-mode`, which makes production debugging and rollout validation misleading. Consider logging the computed `batch_mode` value (or emitting both requested/effective modes) so telemetry matches the runtime path.</comment>
<file context>
@@ -33,24 +33,26 @@ async fn main() {
- let (model_opt, paged_opt) = if args.batch_mode == BatchMode::Paged {
- if let Some(runtime) = model {
- let paged = build_paged_runtime(&args, runtime.clone());
+ let batch_mode = effective_batch_mode(&args, loaded_model.autotune_plan.as_ref());
+ let (model_opt, paged_opt) = if batch_mode == BatchMode::Paged {
+ if let Some(runtime) = loaded_model.runtime {
</file context>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Triage note (maintainer pass): green CI, self-contained, and the server wiring ( Before merge, the test plan still has two unchecked boxes:
The first is cheap and should be green already given the matrix passes — please tick it or say if it's blocked. The H100 manual check can be a post-merge follow-up since the tier-9 rules are guarded behind |
1 participant
Summary
load_model_runtime_with_planapplies plan values to server args (batch mode, KV dtype, TurboQuant, prefill chunk size) and configures the PagedAttention scheduler/block pool from the plan.Test plan
cargo test -p oxidize-core h100_31bcargo test -p oxidize-server effective_batch_modecargo test -p oxidize-server server_autotunecargo test -p oxidize-cpp(autotune_test)oxidize-server --model <q4_k_m.gguf> --auto --print-planon H100 hardwareMade with Cursor
Summary by cubic
Adds H100 Hopper as a GPU family and a throughput autotune plan; the server now applies this plan to auto-enable paged KV, Q4 TurboQuant, FlashAttention-3, CUDA graphs, and plan-based scheduler/block-pool sizing on H100 when
--auto. No breaking changes; override any setting with explicit flags.New Features
oxidize-core: addsWeightPlan/AttentionKernel, chunked prefill, max decode batch, TPS estimates, and tests.oxidize-server: newload_model_runtime_with_planandeffective_batch_mode; applies plan to args (threads, ctx size, batch mode, KV dtype, TurboQuant, prefill chunk); paged runtime sizes BlockPool KV dtype and Scheduler (prefill chunk, max decode batch) from the plan; adds--prefill-chunk-size(default 16).gpu-cluster generatesupports--family h100with a default H100 node pool; API server path uses the plan to build a paged runtime; plan JSON now includeskv_quantization, weight/attention/CUDA fields.oxidize-cpp): exposes new plan fields, detects H100 vianvidia-smi, updates JSON/summary output, adds an H100 throughput test.Dependencies
anyhowto1.0.103to address RUSTSEC-2026-0190.Written for commit 36ee327. Summary will update on new commits.