Add H100 MiniMax-M3 NVMe and tiered AgentX sweep #2775
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -289,13 +289,36 @@ else | |
|
|
||
| export GPU_COUNT="${GPU_COUNT:-${TP:?TP must be set}}" | ||
|
|
||
| SALLOC_TIME_LIMIT="${SALLOC_TIME_LIMIT:-300}" | ||
| salloc --partition="$SLURM_PARTITION" --account="$SLURM_ACCOUNT" \ | ||
| --gres="gpu:$GPU_COUNT" --exclusive --time="$SALLOC_TIME_LIMIT" \ | ||
| --no-shell --job-name="$RUNNER_NAME" | ||
| JOB_ID=$(squeue --name="$RUNNER_NAME" -u "$USER" -h -o %A | head -n1) | ||
| if [[ -z "$JOB_ID" ]]; then | ||
| echo "ERROR: failed to resolve H100 Slurm allocation" >&2 | ||
| exit 1 | ||
| fi | ||
| cleanup_allocation() { | ||
| local rc=$? | ||
| trap - EXIT INT TERM | ||
| scancel "$JOB_ID" 2>/dev/null || true | ||
| exit "$rc" | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| trap cleanup_allocation EXIT INT TERM | ||
|
Comment on lines
+301
to
+307
Contributor
There was a problem hiding this comment. 🔴 BLOCKING: The NVMe KV directory leaks on every successful run. Why it matters: On the success path, the pre-existing Fix: Delete the trailing # cleanup_allocation (EXIT trap) removes the NVMe dir, then scancels.i.e. remove line 345 entirely — the EXIT trap already performs |
||
|
|
||
| NVME_CONTAINER_MOUNT="" | ||
| if [[ "${KV_OFFLOADING:-none}" == "nvme" || "${KV_OFFLOADING:-none}" == "dram+nvme" ]]; then | ||
| NVME_HOST_ROOT="/mnt/numa0/enroot/cache/group-$(id -g)" | ||
| NVME_HOST_DIR="$NVME_HOST_ROOT/inferencex-kv-$JOB_ID" | ||
| srun --jobid="$JOB_ID" bash -c " | ||
| set -e | ||
| test -w '$NVME_HOST_ROOT' | ||
| mkdir -m 700 '$NVME_HOST_DIR' | ||
| findmnt -T '$NVME_HOST_DIR' | ||
| " | ||
| NVME_CONTAINER_MOUNT=",$NVME_HOST_DIR:/kv-offload" | ||
| export NVME_OFFLOAD_DIR=/kv-offload | ||
| fi | ||
|
There was a problem hiding this comment. Cancelled jobs leak NVMe cacheMedium Severity Each NVMe job creates a host directory Additional Locations (1)  Reviewed by Cursor Bugbot for commit b52c639. Configure here. |
||
|
|
||
| # flock-serialize the enroot import so concurrent sweep jobs on the same | ||
| # shared NFS path don't race each other into 'File already exists' (race | ||
|
|
@@ -317,12 +340,10 @@ else | |
|
|
||
| srun --jobid=$JOB_ID \ | ||
| --container-image=$SQUASH_FILE \ | ||
| --container-mounts=$GITHUB_WORKSPACE:/workspace/,$HF_HUB_CACHE_MOUNT:$HF_HUB_CACHE,$AIPERF_MMAP_CACHE_HOST_PATH:/aiperf_mmap_cache$NVME_CONTAINER_MOUNT \ | ||
| --no-container-mount-home \ | ||
| --container-workdir=/workspace/ \ | ||
| --no-container-entrypoint --export=ALL,PORT=8888,AIPERF_DATASET_MMAP_CACHE_DIR=/aiperf_mmap_cache \ | ||
| bash benchmarks/single_node/${SCENARIO_SUBDIR}${EXP_NAME%%_*}_${PRECISION}_h100${SPEC_SUFFIX}.sh | ||
|
|
||
| fi | ||
|
There was a problem hiding this comment. Offload files persist after allocation endsHigh Severity Allocation teardown now only runs Reviewed by Cursor Bugbot for commit e631356. Configure here.
Collaborator
Author
There was a problem hiding this comment. The launcher-side cleanup |
||
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.
🟡 WARNING: All TP ranks are pointed at the same
disk_path($NVME_OFFLOAD_DIR/cache.bin) whiledisk_capacity_bytesis computed per rank (8 TB / TP).Why it matters: The per-rank capacity math implies the connector is instantiated once per TP worker (consistent with the existing
vllm-simpleusage indsv4_fp4_b200_vllm_mtp.sh, which passescpu_bytes_to_use_per_rank). If the fork's disk backend does not internally shard/suffix the path per rank, 8 workers will open and write the same 1 TB-capacity file and silently corrupt each other's cached blocks — producing wrong "cache hit" data rather than a crash. I couldn't verify the fork'sSimpleCPUOffloadConnectordisk backend (source unavailable in this review), and the PR's one-off validation covered the tieredOffloadingConnectorpath, not this NVMe-only path.Fix: Confirm the disk backend appends a rank identifier to
disk_path(and note it in a comment), or make it explicit in the config, e.g. a per-rank template/directory instead of a singlecache.bin. If it does shard internally, feel free to resolve this.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.
Confirmed in the patched vLLM backend:
SimpleCPUOffloadConnectorrewrites the configured path asf"{disk_path}.rank_{device.index or 0}"(vllm/v1/simple_kv_offload/worker.py), so TP8 createscache.bin.rank_0throughcache.bin.rank_7, each with the configured per-rank capacity. Added an inline comment beside the config to make this non-obvious behavior reviewable.