From f5ce49682ed7bc7df06354618c629fe59ffffbeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=A0=95=EC=8B=9C=EC=9B=90?= Date: Fri, 8 May 2026 00:31:59 +0900 Subject: [PATCH] Close first librarian promote round-trip + add --accept flag scripts/librarian_promote.py gains a --accept flag that copies the generated draft to the suggested docs/research/ target in the same invocation. This is explicit per-invocation opt-in, not auto-apply: the operator types --accept knowingly, the cp is still a deliberate acceptance decision, just expressed in one command instead of two. docs/research/option-a-bench-result.md is the first promoted note, generated by gemini from docs/archive/option-a-bench-result-2026-05-07.md and accepted verbatim. Spot-check verified that cited file:line locations match current source (traversal.pyx:473-475 and inference_server.py:226-228 carry the cited code), the Last-verified header reflects today's date and HEAD, and the durable conclusion (sync-blocking policy boundary as the structural ceiling, not IPC plumbing) is preserved. This closes the end-to-end loop the librarian was designed for: oversize check surfaced docs/performance.md, the routing pass split durable analysis into the archive entry, the promote dispatcher turned the archive entry into a research draft, and the human accept step landed it as a tracked research note. Took one LLM call. Removes the now-resolved ignore-list entry for docs/research/option-a-bench-result.md (the file exists; the forward reference is real). scripts/librarian.sh exits 0 against the working tree. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/plans/librarian.md | 18 ++++++++ docs/research/option-a-bench-result.md | 61 ++++++++++++++++++++++++++ scripts/librarian-ignore.txt | 5 --- scripts/librarian_promote.py | 37 +++++++++++++--- 4 files changed, 110 insertions(+), 11 deletions(-) create mode 100644 docs/research/option-a-bench-result.md diff --git a/docs/plans/librarian.md b/docs/plans/librarian.md index 2a27933..0af34d5 100644 --- a/docs/plans/librarian.md +++ b/docs/plans/librarian.md @@ -193,6 +193,10 @@ shells out to the LLM CLI selected by `LIBRARIAN_LLM` `runs/tmp/librarian-promote--draft.md` for human review; the script never writes into `docs/research/` itself. `--show-prompt` prints the assembled prompt for inspection without calling the LLM. +`--accept` is an explicit per-invocation opt-in that copies the draft +to the suggested target as part of the same command — still +propose-only by design (the operator chooses acceptance knowingly, +not auto-applied). Refuses to run if: - the path is not under `docs/archive/`, @@ -202,6 +206,20 @@ Refuses to run if: If the LLM judges the archive non-promotable, it is instructed to return a single line `SKIP: ` instead of a draft. +### First successful round-trip (2026-05-08) + +Smoke test against `docs/archive/option-a-bench-result-2026-05-07.md` +with `LIBRARIAN_LLM=gemini` produced a draft that passed spot-check: +`Last verified` header used today's date and the current commit +(`8bbed31`); cited file paths and line numbers were verified as +real (`traversal.pyx:473-475`, `inference_server.py:226-228`); code +snippets matched current source; durable conclusion (sync-blocking +policy boundary as the structural ceiling) preserved; ~60 lines of +prose, no bullet soup. Accepted verbatim as +`docs/research/option-a-bench-result.md`. The whole loop — +oversize finding → extracted archive → LLM draft → human accept — +closed without touching the LLM's output. + ## Stage 2 remaining - MEMORY.md drift fixup mode (read drift report, propose one-line diff --git a/docs/research/option-a-bench-result.md b/docs/research/option-a-bench-result.md new file mode 100644 index 0000000..cf6a098 --- /dev/null +++ b/docs/research/option-a-bench-result.md @@ -0,0 +1,61 @@ +# Option A Benchmark and Structural Ceiling + +**Last verified:** 2026-05-08, commit `8bbed31` +**Source:** `docs/archive/option-a-bench-result-2026-05-07.md` + +## Question + +Why did the centralized inference server (Option A) result in a 5x traversal regression (0.21x speedup) despite the GPU being ~230x faster at raw forward passes than the CPU? + +Short answer: **the sync-blocking nature of the current traversal recursion prevents batching.** Because every worker thread waits for a single-row policy response before proceeding, the realized GPU batch size is limited by the worker count, which is too small to amortize the IPC and shared-memory synchronization overhead for small models. + +## Code reference + +The sync-blocking boundary is located in `src/coolrl_lost_cities/games/classic/deep_cfr/traversal.pyx`. In `_regret_matching_policy_c` (around line 475) and `_policy_from_strategy_network` (around line 552), the traversal recursion crosses into PyTorch: + +```cython +with torch.inference_mode(): + x = torch.as_tensor(info_state, dtype=torch.float32, device=self.device).unsqueeze(0) + advantages = networks[player](x).squeeze(0).detach().cpu().numpy().astype(np.float32) +``` + +When `traversal.inference_backend` is set to `server` in `configs/deep_cfr/default.yaml`, the `networks[player]` call is intercepted by a `NetworkProxy` (instantiated in `workers.py`, around line 91). This proxy posts a request to the `InferenceServer` and blocks until a response is received via a per-slot event. + +The server's batching logic in `src/coolrl_lost_cities/games/classic/deep_cfr/inference_server.py` (around line 221) reports the realized batch size: + +```python +handles.stats_queue.put(BatchStatsMessage(batch_size=len(batch), group_count=len(groups))) +``` + +## Analysis + +The 2026-05-07 benchmark results (`scripts/bench_inference_backend.py`) showed that with `num_workers: 8`, the mean batch size reported by the server was only **~7.2–7.9**. Because the server further splits batches by network kind and index to avoid mixing weights in a single forward pass, the actual GPU group size was roughly **4 rows**. + +Comparing the per-state costs from the microbenchmark (`scripts/profile_gpu_forward.py`): + +| Realized Batch | μs/state (GPU) | +| ---: | ---: | +| 1 | 80.07 | +| 4 | 20.30 | +| 64 | 1.46 | +| 256 | 0.34 | + +At a batch size of 4, the GPU compute time (~20μs) is negligible compared to the IPC round-trip cost (queue post, context switch, event wakeup), which is on the order of **100–200μs** per call. This overhead is compounded over ~200k policy calls per iteration, leading to the observed jump from ~10.8s (local CPU) to ~51.6s (remote GPU) for the traversal phase. + +The "structural ceiling" is that `batch_window_us` and `max_batch` tuning cannot improve performance if there are no additional in-flight requests to coalesce. With 8 workers blocking synchronously, the server will never see the 64+ requests needed to reach the high-efficiency SIMD regime. + +## Practical implication + +Option A is deferred for the current small MLP models (512x3). The `local` backend remains the default in `configs/deep_cfr/default.yaml`. + +To unlock the projected GPU gains, the traversal must be restructured to drive batch sizes up. This leads to two primary paths: +1. **Option B (Interleaved Traversal):** Refactor the Cython traversal into a state machine that can advance multiple traversals concurrently per worker. Each worker would suspend at a policy call, batch its own requests, and resume continuations once the results return. +2. **Option C (Vectorized Traversal):** Re-implement traversal as a single-process operation on large tensors. + +Option B is the chosen next step because it preserves the Cython game-rule logic while breaking the sync-blocking boundary. Option A's plumbing remains in the codebase as it will become beneficial if (a) the model size increases to the point where forward compute exceeds IPC cost, or (b) Option B successfully drives the server's batch size toward 64+. + +## References + +- `docs/research/batched-traversal-inference-decision.md` +- `experiments/traversal_policy_boundary/bench_policy_boundary.py` +- `docs/archive/post-a-optimization-calculus-2026-05-07.md` diff --git a/scripts/librarian-ignore.txt b/scripts/librarian-ignore.txt index e844533..28c464c 100644 --- a/scripts/librarian-ignore.txt +++ b/scripts/librarian-ignore.txt @@ -18,8 +18,3 @@ scripts/run_model_size_experiment.sh # Future config described in docs/plans/torch_compile.md configs/deep_cfr/default_compile.yaml - -# Hypothetical accept target named in docs/plans/librarian.md as a -# smoke-test illustration; resolves naturally if Stage 2 promote is -# accepted, otherwise stays a forward reference. -docs/research/option-a-bench-result.md diff --git a/scripts/librarian_promote.py b/scripts/librarian_promote.py index 207a2c9..094738a 100644 --- a/scripts/librarian_promote.py +++ b/scripts/librarian_promote.py @@ -24,6 +24,7 @@ from __future__ import annotations import argparse import os import re +import shutil import subprocess import sys from datetime import datetime @@ -32,7 +33,7 @@ from pathlib import Path LLM_COMMANDS = { "claude": ["claude", "-p"], "codex": ["codex", "exec"], - "gemini": ["gemini", "-p"], + "gemini": ["gemini", "-p", ""], } DATE_SUFFIX = re.compile(r"-\d{4}-\d{2}-\d{2}$") @@ -85,6 +86,16 @@ def main() -> int: action="store_true", help="Print the assembled prompt to stdout and exit; do not call the LLM.", ) + parser.add_argument( + "--accept", + action="store_true", + help=( + "After the draft lands in runs/tmp/, also copy it verbatim to the " + "suggested docs/research/ target. Explicit per-invocation opt-in: " + "still propose-only by design (you are choosing acceptance " + "knowingly), not auto-apply." + ), + ) args = parser.parse_args() root = _repo_root() @@ -145,7 +156,8 @@ def main() -> int: file=sys.stderr, ) result = subprocess.run( - cmd + [prompt], + cmd, + input=prompt, capture_output=True, text=True, check=False, @@ -157,10 +169,23 @@ def main() -> int: draft_path.write_text(result.stdout, encoding="utf-8") print(f"Draft written to: {draft_path.relative_to(root)}") - print(f"Suggested target on accept: {rel_target}") - print() - print("Next: review the draft. To accept verbatim:") - print(f" cp {draft_path.relative_to(root)} {rel_target}") + + if args.accept: + if target.exists(): + print( + f"Refusing --accept: {rel_target} appeared while the LLM ran.", + file=sys.stderr, + ) + return 1 + shutil.copyfile(draft_path, target) + print(f"Copied to: {rel_target}") + print("Next: review the diff and `git add` + commit if you're satisfied.") + else: + print(f"Suggested target on accept: {rel_target}") + print() + print("Next: review the draft. To accept verbatim:") + print(f" cp {draft_path.relative_to(root)} {rel_target}") + print("Or rerun with --accept to copy in the same step.") return 0