Repository navigation
Conversation
…EGRAINED_APC) The hybrid coordinator disables fine-grained hits for the whole model because KpoolTailManager -- a per-request scratch that opts out of prefix caching -- is included in the compatibility check. One veto snaps every hit to the 3584-token hybrid page, so a warm agent turn recomputes up to 3583 tokens (0.9-4.0 s measured). overlay/patch_apc_fine_grained_hits.py (fail-closed, transactional) excludes only groups whose spec sets participates_in_prefix_caching=False, keeps MLA and mamba in the match, and enforces the real invariant hash_block_size % index_kpool == 0 (verified from the live specs at init; refuses to boot otherwise). KpoolTail stays transient and non-shareable. Composes with patch_hybrid_prefix_hit.py and patch_apc_per_group_retention.py in either order (host test asserts AST-equal composition both ways). Wiring: GLM53_FINEGRAINED_APC (0/1, default 1) validated by the launcher and by the coordinator; artifact guard/order/mounts; Dockerfile COPY/RUN. Measured on 2x DGX Spark GB10 (live): 3x/4x 30k repeats 1.000 hits at ~0.4 s, 2x60k 0.999, 3x5k 0.992; 4-agent x 60k bench turns 1-3 0.886-0.928 with per-turn wall 17.7-23.6 s (before: 0.000, 150-196 s). Host test: tests/test_apc_fine_grained_hits.py.
2e03b51 to
c4c04ae
Compare
plotarmordev
left a comment
There was a problem hiding this comment.
Holding this draft as the single tracked successor for the fine-grained prefix-cache work in #84. Independent source review and direct content comparison confirm that the overlay and design document are byte-identical across the two PRs; this branch carries the mechanism on the post-#130 base. Closing the older duplicate will not discard its measurements or credit.
Please carry forward the outstanding #84 change requests:
- Default off at every layer: .env.example, launcher fallback, and overlay runtime fallback must all use 0. Retain strict 0/1 validation, explicit-empty rejection, and the runtime kill switch.
- Preserve #130's per-group retention behavior and the runtime hash/kpool compatibility check. Keep both-rank wiring and pre-stop validation.
- Validate patch-owned helper/gate regions canonically before accepting an already-patched file; token/sub-string presence alone is insufficient. Anchor drift must fail without modifying the target.
- Remove the connector receipt and the upstream-fixed silent no-op detector requested for removal in #84. Retain meaningful host/in-image regression coverage and rebase onto current main after the independent fixes land.
The reviewed opt-in variant completed 292 observational requests across four conditions: each had the same 63/73 strict checker result and the same ten failures. This found no fine-only checker regression, but is not a fresh-boot BAAB qualification or an internal KV/KDA/CoW/replay numerical oracle. The earlier acceptance-length concerns remain unresolved; fine-grained APC is not approved as a default. Preserve #84's live receipts and A/B results as reference evidence, rather than resetting that review history.
Reviewed — changes/evidence requested; do not merge yetMaintainer source fixes are now pushed to this PR, preserving author history: Incorporated source fixes and author workThe integration preserves the author's Evidence acknowledged; what remains openOur earlier short-tail probe required 31,616 cached tokens but observed 28,672, after the replay clamp and reconciliation; we retained the guard and stopped that acquisition at its failed prerequisite. The author's new 66-request report adds important evidence: the 60-request core BAAB study found no additional cache reuse; a separate six-request AAB diagnostic included an enabled long continuation with 11,904 cached tokens versus 7,168 in its controls. All 66 output checks passed as reported by the author. We have not independently reproduced that study. One enabled supplemental observation establishes a conditional lead, not a repeatable general or default-on benefit; it must stay separate from the core protocol. Specific author changes/evidence requested
Required qualification before marking readyUse the published TheGrill v0.2.0 experimental prerelease, from source Follow the measurement/evidence scope, claim-specific recipe guidance, and report template. Pin the executable, workload/producer, actual backend/runtime and model revisions, and declared settings. Qualify supported request controls before the prospective acquisition; use the full For this PR, please keep it draft and mark it ready only after the requested changes and separate correctness/numerical checks pass, and the relevant TheGrill 0.2.0 reports complete and satisfy the predeclared PR-specific acceptance gates. Then request maintainer re-review of the exact final head. Exit 0 or TheGrill publication is not blanket backend support or persisted-model-KV/eviction proof; unsupported claims need separate real-backend evidence or must remain explicitly unqualified. This is the review gate for this PR, not a claim that every project must adopt TheGrill. Disposition: reviewed; changes/evidence requested; merge hold. Tooling availability is now resolved, so only |
…EGRAINED_APC) The hybrid coordinator disables fine-grained hits for the whole model because KpoolTailManager -- a per-request scratch that opts out of prefix caching -- is included in the compatibility check. One veto snaps every hit to the 3584-token hybrid page, so a warm agent turn recomputes up to 3583 tokens (0.9-4.0 s measured). overlay/patch_apc_fine_grained_hits.py (fail-closed, transactional) excludes only groups whose spec sets participates_in_prefix_caching=False, keeps MLA and mamba in the match, and enforces the real invariant hash_block_size % index_kpool == 0 (verified from the live specs at init; refuses to boot otherwise). KpoolTail stays transient and non-shareable. Composes with patch_hybrid_prefix_hit.py and patch_apc_per_group_retention.py in either order (host test asserts AST-equal composition both ways). Wiring: GLM53_FINEGRAINED_APC (0/1, default 1) validated by the launcher and by the coordinator; artifact guard/order/mounts; Dockerfile COPY/RUN. Measured on 2x DGX Spark GB10 (live): 3x/4x 30k repeats 1.000 hits at ~0.4 s, 2x60k 0.999, 3x5k 0.992; 4-agent x 60k bench turns 1-3 0.886-0.928 with per-turn wall 17.7-23.6 s (before: 0.000, 150-196 s). Host test: tests/test_apc_fine_grained_hits.py.
c4c04ae to
18daea4
Compare
|
Pushed the rebased branch and review fixes. The PR remains an opt-in proposal, with Commit map
How the review requests were addressedAll implementation corrections below are in
Focused live verification, completed 2026-09-15 JSTEach label identifies a separate test run, starting from a fresh boot of the inference service across both CX7-connected DGX Sparks together:
The actual execution order was:
This is the BAAB (on/off/off/on) sequence referenced below. In the core results table, A1+A2 pools the two disabled runs and B1+B2 pools the two enabled runs. The runs used isolated containers, launcher staging, API endpoint, and compile caches. Fixed TP=2, EXL3/TR3 4bpw target, DFlash2 k=7 and draft TP=2, adaptive-k off, FP8 KV, context 131,072, batch 7,168, max sequences 4, utilization 0.85, global retention unset, SWA retention 0, replay window 2,048. Only the fine-grained flag differs across the core arms. This does not qualify the full 850K configuration. Each boot received the same 15 serialized requests: three roughly 9K–12K-token documents, each tested cold, repeated, with two fixed branches, and returned to the original prompt. Temperature 0, seed 171, thinking disabled, maximum output 256. Formatting/checksum checks are deterministic, and generated output is not fed back into the branch prompts.
Acceptance length is The replay clamp explains why an enabled 64-token alignment is insufficient evidence of an actual fine hit. For example, a 9,005-token repeat logged a candidate of 8,960, clamped to 6,912, but the completed request recorded zero cached tokens. Roughly 12K short-tail requests ultimately reused 7,168 tokens. Intermediate clamp targets are not hit receipts. This agrees with the distinction raised in the earlier bounded qualification comment. After B1 exposed this behavior, a separate fill/long-continuation pair was added for A1, A2, and B2 only. It is an AAB (off/off/on) diagnostic, excluded from the core BAAB results: an 11,966-token fill followed by a 15,493-token continuation, with enough new input to exceed the replay window.
11,904 is divisible by 64 but not by 3,584. The enabled observation reused 4,736 additional tokens, reducing fresh prefill from 8,325 to 3,589 tokens and TTFT by 54.6% relative to the mean controls. All three outputs were byte-identical. This demonstrates a conditional benefit, with only one enabled observation against two controls. When enabling is worthwhileA promising workload to test is a long, unchanged prefix that remains cached, followed by substantial new text—for example, an existing conversation followed by a large tool result. A usable fine-grained boundary must survive the retention and DFlash replay checks; a long conversation alone does not ensure a benefit. Keep Leave Verification receipts and limits
Conclusion: retain opt-in default |
plotarmordev
left a comment
There was a problem hiding this comment.
Changes/evidence requested on 4ee3f3d04fb7a61961af36ecb177a848d6e9ddff. Maintainer fixes are pushed and scoped CPU checks passed; this is not approval of the remaining runtime claims. Specific remaining changes, acknowledged positive/negative evidence, and exact readiness gates are in the updated maintainer handoff.
Use TheGrill v0.2.0 experimental prerelease with its published CI-built artifact checksums/receipts, not staging hashes. Keep the PR draft until its separate correctness/numerical checks and relevant predeclared performance gates are satisfied; then request exact-head re-review. Tooling availability is resolved, but the merge hold remains. No additional GPU qualification is claimed by this review.
plotarmordev
left a comment
There was a problem hiding this comment.
Hold update
Public head checked: 4ee3f3d04fb7a61961af36ecb177a848d6e9ddff.
Why it is held: The cache feature has a conditional benefit, not a general win. In our bounded integrated comparison, long append reused 12,032 rather than 10,752 tokens; medium append reused 24,000 rather than 21,504; short append fell to 7,168 rather than 10,752. The reference answers were correct. This is not proof of internal state or numerical safety.
What is already addressed: Default-off behavior, strict flags, both-rank wiring, and replay/retention safeguards were accepted in the source review. Your separate core BAAB study and exploratory long-continuation result remain distinct evidence; we are not relabeling one as the other.
Next action — me, @plotarmordev: Settle the supported scope and competing implementations (#125/#195), then pin the numerical/state-safety and performance checks. The outstanding qualification request is not cleared by the cache gains above.
Author action now: No immediate rebase or new hardware run. Keep the option off by default and retain the short/no-benefit cases. Once the plan is agreed, compare balanced on/off runs with completed-request counters and answer checks; separately check the claimed replay, eviction and state-safety behavior. Do not weaken safety guards to make a hit count pass.
Please keep the hold until the conditions above are met. When a new version is ready for review, include its commit ID, the checks actually run, their results, and anything not tested. Keep failed or incomplete runs in the report. Share only sanitized results—not private prompts, credentials, or machine configuration.
Maintainer priority triage — 2026-09-26Reviewed head: P2, retained for a maintainer scope/qualification decision—not for merging duplicate lookup code. Main #251 now implements participating-group fine-grained lookup plus replay/state handling, so the original opt-in implementation is no longer an independent feature to land as-is. Outstanding state-safety/quality questions are not dismissed by that overlap or by closing the lean duplicate #125. Next owner: @plotarmordev decides whether to move the remaining qualification work into a distinct current-main task and archive this PR, or define a genuinely non-overlapping delta. That explicit disposition and preserved safety gates unblock further work. No author rebase/benchmark request now. This is prioritization only; no merge or implementation work is authorized by it. |
|
Archiving this PR, per the 2026-09-26 maintainer triage. Main #251 now implements participating-group fine-grained lookup with replay/state handling, so this PR's lookup code would duplicate it. The unresolved state-safety and qualification questions, and your evidence, move to #300 so they aren't lost. Thank you @mmizutani for the mechanism, the safety corrections and the 66-request study; they shaped what landed. |
Description and Motivation
The hybrid coordinator includes
KpoolTailManagerin its fine-grained lookup veto even though that scratch group opts out of prefix caching. This PR lets participating cache groups use 64-token lookup boundaries when the runtime layout supports them, instead of always falling back to the 3,584-token hybrid page.GLM53_FINEGRAINED_APC=0is the example, launcher, and runtime default. Set1and restart to opt in. Current evidence demonstrates a conditional benefit, but does not qualify default enablement or resolve earlier draft-acceptance concerns.hash_block_size % index_kpool == 0. Participating coarse-only managers retain the fallback; unsafe or unverifiable scratch layouts refuse initialization when fine hits would otherwise be enabled.0/1validation, explicit-empty rejection, kill switch, pre-stop artifact checks, both-rank wiring, and Dockerfile integration. Caller overrides use main's existing generic mechanism.Based on upstream main
d8ad1831182412dfc7323d90f4ad91b8d636451e.When enabling is worthwhile
A promising workload to test is a long, unchanged prefix that remains cached, followed by substantial new text—for example, an existing conversation followed by a large tool result. A usable fine-grained boundary must survive the retention and DFlash replay checks; a long conversation alone does not ensure a benefit.
Keep
GLM53_FINEGRAINED_APC=1only after repeated on/off comparisons on representative requests show a worthwhile improvement: more actual cached tokens and reduced time to first output and/or total response time, without regressions in answer correctness or draft acceptance. Check completed-request cache counters and timings; an enabled 64-token alignment or an intermediate replay-clamp target is insufficient evidence.Leave
0for mostly unrelated/cold requests or workloads without a demonstrated benefit. The core short-follow-up tests gained no additional reuse. The successful long-continuation probe had enough new input to exceed the tested 2,048-token replay window, but 2,048 is not a universal enablement threshold, and one enabled observation does not qualify long agentic workflows generally.Related Issues
Successor to #84; preserves its A/B evidence and acceptance assessment. Addresses the implementation requests in the #171 review; broader runtime qualification remains open. Preserves #130.
Testing
Current local checks after diff reduction:
18daea4: identical after excluding documentation. The launcher preamble exactly matches main. Shell syntax, diff whitespace, and relative documentation links passed.The live verification record at
18daea4contains the complete configuration, fixed payload protocol, metrics, and provenance deviations. Two CX7-connected Sparks ran TP=2, DFlash2 k=7/draft TP=2, adaptive-k off, FP8 KV, context 131,072, batch 7,168, max sequences 4, global retention unset, SWA retention 0, and replay window 2,048.These are local observations, not CI results or a general speedup guarantee. The cleanup adds no new GPU measurements. Internal KV/KDA/CoW equivalence, eviction pressure, KV-transfer connectors, long agentic-loop acceptance, and default-on qualification remain unproven by this run.
Checklist:
0with strict validation and a runtime kill switch..env.example, and feature documentation.