92 lines
5.0 KiB
Markdown
92 lines
5.0 KiB
Markdown
---
|
||
name: quant-pipeline-adversarial-review
|
||
description: "Review or audit the serverAppConfigs quantization campaign pipeline (convert-calib-kldref-score plus corpus builder/rechunker/pusher, fp8 stacker) for defects that silently waste GPU-hours or produce a number that is not the claimed number."
|
||
---
|
||
|
||
# Adversarial review of the quant campaign pipeline
|
||
|
||
Friction-earned 2026-10-29 across `scripts/convert-qwen38-w5nl64.sh`,
|
||
`run-qwen38-bf16.sh`, `make-calib-corpus.py`, `rechunk-kld-corpus.py`,
|
||
`calib-push.py`, `stack-qwen38-official-fp8.py`. Every class below was a real
|
||
defect in that tree. Review code, not vibes: quote the line, name the cost.
|
||
|
||
## Files in scope (one campaign = this set)
|
||
|
||
| Stage | File | Owns |
|
||
|---|---|---|
|
||
| corpus | `make-calib-corpus.py` | calib + kld JSONL, manifest |
|
||
| corpus | `rechunk-kld-corpus.py` | windows that fit the container's KV window |
|
||
| convert | `convert-<model>.sh` | plan → artifact per arm |
|
||
| tape | `run-qwen38-bf16.sh` | the rig's argv, mount, trap, gates |
|
||
| drive | `calib-push.py` | prefill traffic into the tap |
|
||
| fp8 path | `stack-qwen38-official-fp8.py` | expert stacking + PLE merge |
|
||
|
||
## Defect classes to hunt, in cost order
|
||
|
||
1. **Claimed identity that is only a length check.** A tool that writes TEXT back
|
||
out (`tok.decode(window)`) and asserts only counts is not proving token
|
||
identity — decode→encode can merge/split. Fix: re-encode each window and
|
||
compare id lists; refuse on mismatch. Same shape: an assertion on "N tokens
|
||
in, N tokens out" that counts windows rather than re-encoded text.
|
||
2. **A loop that cannot finish and cannot notice.** Drivers that re-push a corpus
|
||
`--max-rounds` times without measuring the sink. Compare the tap/output dir's
|
||
byte total per round and abort on zero growth; abort on zero successes; abort
|
||
on zero prompts. Without this the loss is silent for hours.
|
||
3. **`set -u` argument holes.** `--flag) VAR="$2"; shift 2` dies with "unbound
|
||
variable" when the flag is last. Guard `[ "$#" -ge 2 ]` in every such arm.
|
||
4. **Restore paths that leave the GPU holder alive.** `--rm` only fires when
|
||
podman itself exits, so a killed run leaves conmon holding the cards while
|
||
`systemctl is-active` says `active`. `restore()` MUST `podman rm -f "$C"`
|
||
before starting production. Prove with `podman ps` after a `kill`.
|
||
5. **Gates that run after the expensive phase.** A leftover container name, an
|
||
existing output, a non-writable tap dir, an unset figure — check them before
|
||
the plan/boot, not when podman/sed fails mid-run.
|
||
6. **Zero-exit with no artifact.** After `rc=0`, assert the output file exists
|
||
before `stat`/arithmetic on it.
|
||
7. **`assert` in a verifier.** `python -O` deletes it silently. Use `raise`.
|
||
8. **Warning where the code's own assumption breaks.** e.g. a 128-lane pack fed
|
||
an HD that is not a multiple of 128 — warning + continue writes a wrong table.
|
||
9. **Re-tokenization in loops.** A pool cycled 40× re-tokenizing each fragment
|
||
per pass; and a manifest recomputing a total by re-encoding the whole corpus.
|
||
Cache counts once, accumulate during generation.
|
||
10. **Substring tests standing in for key tests.** `"content" in line` before
|
||
`json.loads(line)["content"]` crashes on any list-shaped `.json`.
|
||
11. **Path guards compared as strings.** `A/../A/x` defeats an equality check;
|
||
add `realpath -m` on both sides.
|
||
12. **Shellcheck's honest false positive**: single-quoted `$CALIB` passed to a
|
||
converter that expands it from the container env is correct (SC2016). Say so
|
||
in the PR; do not "fix" it.
|
||
|
||
## Procedure
|
||
|
||
1. Inventory: `wc -l` the scripts + recipes; read each **completely** (ranges,
|
||
not whole files, but every line). Recipes carry the numbers scripts assert.
|
||
2. Read the tests that pin the argv (`tests/qwen38_reference.bats`'s `SHAPE`
|
||
fixture, `tests/qwen38_campaign.bats`) — they are the contract you must not
|
||
break, and the first place a stale number hides.
|
||
3. Mechanical gates:
|
||
```bash
|
||
python3 -m py_compile scripts/*.py
|
||
bash -n scripts/<each>.sh
|
||
shellcheck -S warning scripts/<each>.sh # never pass .py to it
|
||
./tests/run.sh
|
||
```
|
||
4. Fix the real ones only; keep diffs surgical; re-run the suite after the batch.
|
||
5. Smoke the changed path for real — smoke beats the suite:
|
||
```bash
|
||
# run a local script on the server's data via the corpus venv, no checkout needed
|
||
ssh dankserver '/home/dank/venvs/radcorpus/bin/python - --corpus <corpus> --out /tmp/<x> --force' \
|
||
< scripts/rechunk-kld-corpus.py
|
||
```
|
||
Import-level helpers can be exercised with
|
||
`importlib.util.spec_from_file_location` (e.g. a new `tap_bytes()`: empty→0,
|
||
one file→its size, absent→0).
|
||
6. Stacked PRs: a review commit goes on its own branch off the open one; a diff
|
||
that includes the parent commit is a stacked PR, not a mistake — say which PR
|
||
merges first in the body.
|
||
|
||
## Report
|
||
|
||
One table of defect → cost → fix. Name the failure the defect produced in
|
||
practice (hours wasted, wrong number), because that is what makes it reviewable.
|