mi355x experiments - #5
aditchawdhary wants to merge 1 commit into
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # A Job's pod template is immutable, so `kubectl apply` over a prior run of | ||
| # the same name fails ("field is immutable"). Delete any existing Job first | ||
| # (a completed/failed one, or one being relaunched) so create always works. | ||
| kubectl delete job "${job}" --ignore-not-found --wait=true |
There was a problem hiding this comment.
Relaunching a shard now deletes the existing Kubernetes Job before creating its replacement, regardless of whether it is still running. If the documented rerun command is used during a multi-hour sweep, this terminates the active pods and loses the experiment currently in progress. The previous kubectl apply behavior did not destroy running Jobs; please check the Job state or require explicit confirmation before deleting it.
| dir="${dir%/}" | ||
| # Completed run dirs never change; skip any dir whose remote byte size | ||
| # matches what we recorded after its last successful pull. | ||
| if grep -qxF "${size} ${dir}" "${SIZES}"; then |
There was a problem hiding this comment.
This treats a matching du -sk value as proof that a shard has not changed, but the Job continues writing results and telemetry into that directory throughout the run. File contents can change within already allocated filesystem blocks without changing this rounded size, so a later sync may be skipped and leave the local and Google Drive copies stale or containing a partial JSON snapshot.
| env: | ||
| VLLM_ROCM_USE_AITER: '1' | ||
| vllm_args: | ||
| - --trust-remote-code |
There was a problem hiding this comment.
This passes an unpinned moonshotai/Kimi-K2.5 reference to vLLM with --trust-remote-code. If the upstream default revision is compromised, its Python code executes in a networked pod that contains HF_TOKEN and has writable access to the shared model cache and result storage, enabling credential disclosure and cross-run data tampering. The same pattern also appears in kimi-k25-int4-repro.yaml and kimi-k25-beat.yaml; pin the model revision or avoid executing repository-provided code.
How this was verified: The manifest values are passed verbatim to vllm serve, while the target pod receives HF_TOKEN and mounts /mnt/shared read-write.
| def _stat(report: dict, needles: tuple[str, ...], stat: str, reject: tuple[str, ...] = ()): | ||
| """Best numeric value whose key path contains every needle and the stat word. | ||
|
|
||
| Ties break toward the deepest (most specific) path, so a nested | ||
| ``metrics/time_to_first_token_ms/p99`` beats a summary field that happens to | ||
| share a word. Returns None when nothing matches -- never a fabricated 0. | ||
| """ | ||
| want = _STAT_ALIASES.get(stat, (stat,)) | ||
| best = None | ||
| for keypath, value in _walk(report): | ||
| if not isinstance(value, (int, float)) or isinstance(value, bool): | ||
| continue | ||
| joined = "/".join(keypath) | ||
| if any(n not in joined for n in needles): | ||
| continue | ||
| if any(r in joined for r in reject): | ||
| continue | ||
| if not any(w in joined for w in want): | ||
| continue | ||
| depth = len(keypath) | ||
| if best is None or depth > best[0]: | ||
| best = (depth, float(value)) | ||
| return None if best is None else round(best[1], 2) | ||
|
|
||
|
|
||
| def _scalar(report: dict, needles: tuple[str, ...], reject: tuple[str, ...] = ()): | ||
| """Shallowest numeric whose key path contains every needle, ignoring stat | ||
| words. For fields guidellm emits as a bare scalar rather than a stat block.""" | ||
| best = None | ||
| for keypath, value in _walk(report): | ||
| if not isinstance(value, (int, float)) or isinstance(value, bool): | ||
| continue | ||
| joined = "/".join(keypath) | ||
| if any(n not in joined for n in needles) or any(r in joined for r in reject): | ||
| continue | ||
| if best is None or len(keypath) < best[0]: | ||
| best = (len(keypath), float(value)) | ||
| return None if best is None else round(best[1], 2) |
There was a problem hiding this comment.
These helpers infer the guidellm schema by matching substrings in arbitrary nested key paths and selecting values based on path depth. Without a representative report fixture and tests for the supported keys and units, a schema change or similarly named sibling field can silently produce believable but incorrect throughput or latency values. Please add fixture-based coverage for the mappings used by the published metrics.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Comments Outside DiffThese findings sit on lines the diff does not cover, so they could not be posted inline. Each one leaves this list once its file changes.
|
This branch has not been deployed
1 participant
This PR is not safe to merge until active Jobs are protected from destructive relaunches, result synchronization detects content changes reliably, and remote model code is pinned or isolated from credentials and shared storage.
Findings
Summary
This PR adds MI355X Kimi experiment configurations, multi-pass serving sweeps, InferenceX-compatible metric reporting, profiling analysis, Kubernetes launch controls, and chunked result synchronization.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR M[MI355X experiment manifest] --> L[launch_mi355x_shards.sh] L -->|substitute shard, path, image| J[Kubernetes Job] J --> H[Runtime harness] H --> V[vLLM server] H -->|multiple traffic passes| G[guidellm] G --> R[Shared shard results] V --> T[Telemetry and optional rocprof trace] T --> R R --> C[Chunked local mirror] C --> D[Google Drive copy] C --> A[Metrics and Pareto analysis]Reviews (1) · Last reviewed commit: "mi355x experiments"