Skip to content

feat: adaptive conformal interval calibration (ACI) - #33

Draft
gkneighb wants to merge 2 commits into
llm-d:mainfrom
gkneighb:feat/aci-conformal
Draft

feat: adaptive conformal interval calibration (ACI)#33
gkneighb wants to merge 2 commits into
llm-d:mainfrom
gkneighb:feat/aci-conformal

Conversation

@gkneighb

Copy link
Copy Markdown
Member

What does this PR do?

Adds opt-in Adaptive Conformal Inference (Gibbs & Candes 2021) that keeps p90 interval coverage valid between retrains. A one-sided upper offset c_t is adapted online in the continuous-coverage loop from recent conformity scores s = y - qhat(x) and added to the served quantile in predict(). The score buffer is rebased on retrain (stale scores against the old qhat would otherwise cause transient over-coverage). alpha_t and c_t are exposed in /metrics, doubling as a drift signal. New aci.py module; ~6 small hooks in training_server.py; disabled by default (ACI_ENABLED=false).

Why is this change needed?

From the #19 discussion with @Whatsonyourmind: the static interval under-covers during the detection-to-retrain lag. ACI holds coverage through the lag with one scalar (gamma), composing with — not replacing — the calibration-triggered retraining (which still sharpens the point model). Validated two ways: an offline demo (coverage held ~90% through the lag vs 45% static, at 3.2ms added width; gamma=0.02 sweet spot) and @Whatsonyourmind's independent reproduction of the same result.

How was this tested?

  • Unit tests added/updated — tests/test_aci.py (8 tests: update direction, [0,1] clip, offset quantile, rebase, maxlen)
  • Integration/e2e tests added/updated — live-server smoke confirmed all hooks: offset grows under drift, /predict reflects it, /metrics exposes alpha_t/c_t, and the retrain-rebase resets the buffer (offset_ms -> 0, alpha_t -> target exactly when a retrain fires — no stale over-coverage)
  • Manual testing performed

Checklist

  • Commits are signed off (git commit -s) per DCO
  • Code follows project contributing guidelines
  • Tests pass locally (make test) — 8 passed, 40 skipped
  • Linters pass (make lint)
  • Documentation updated (env vars; follow-up)

Related Issues

Follow-up to #19. Stacked on the continuous-coverage PR — please review/merge that first; this PR's diff is ACI-only against that base.

Notes for review (@Whatsonyourmind)

Two spots I'd value your eyes on, both flagged in the thread:

  1. Batched alpha_t update. The server steps alpha_t once per coverage-eval interval (vs the per-request prototype), so gamma is per-interval — does the cadence look right against continuous_coverage_loop's timing?
  2. /metrics coverage is the static (pre-offset) value — useful as the drift signal, but it doesn't reflect the ACI-served coverage. Exposing ACI-adjusted coverage too could be a follow-up; happy to add it here if you'd prefer.

@Whatsonyourmind

Copy link
Copy Markdown

Dug into this — it's clean, and the hooks are in the right places. The rebase landing right after last_retrain_time is set is exactly the spot, the (y_true - y_pred) score threading through _calculate_metrics_on_test is tidy, and opt-in / default-off means zero risk to the served path. The update_batch order (evaluate miss-rate against the current offset, then fold the scores in) is also right.

On your two questions:

1. Batched alpha_t cadence — this is the one I'd change. The per-step math is correct, but stepping alpha_t once per eval interval with the same gamma you tuned per-request makes the effective learning rate gamma / N, where N = conformity scores per interval. The γ≈0.02 sweet spot came from the per-request prototype, so in the server it adapts ~N× too slowly. Quantified against your mechanics (drift active, N=200 scores/interval, target 0.1):

alpha_t over coverage-eval intervals:
  per-request    gamma=0.02   : 0.1 → ~0 within the first interval
  batched/intvl  gamma=0.02   : 0.1 → 0.091 → 0.082 → … → ~0 only after ~11 intervals
  batched/intvl  gamma=0.02·N : 0.1 → ~0 in one interval  (matches per-request)

So at 0.02 the batched alpha_t barely moves — ~11 eval intervals (~2200 requests) to do what the prototype did in ~10. Three ways out: scale ACI_GAMMA by ~scores-per-interval; or apply the update as N per-score substeps (≈ equivalent to first order); or keep one-step-per-interval but re-tune gamma to the interval cadence and re-label — the "0.02 sweet spot" is a per-request number and doesn't transfer as-is.

Caveat so this doesn't read as alarming: coverage still recovers either way, because the score-buffer quantile (c_t) recalibrates as post-drift scores enter the buffer — that's the first-order mechanism and it's intact (it's why your smoke test held coverage). The consequence of the slow alpha_t isn't broken coverage; it's that gamma/alpha_t are nearly inert as a knob in the server, so the fine correction on top of the buffer isn't really happening and a gamma sweep won't move much until it's rescaled. If you want alpha_t to earn its place (and the γ sweep to be meaningful), the rescale is the fix.

2. /metrics static vs ACI-adjusted coverage — expose both. Your instinct is right that static coverage is the better drift signal (its deviation from target is exactly the detector). But for ops you also want to confirm ACI is doing its job — that the adjusted coverage is back at target. They answer different questions: static = "is there drift?", adjusted = "are we meeting the SLO despite it?". Cheap to emit both alongside alpha_t/c_t; a dashboard wants the pair.

Everything else looks right — rebase, score threading, opt-in. Once #32 lands and you take this out of draft, happy to do a line-level pass. The gamma rescale is the only thing I'd treat as semi-blocking; the rest is polish.

gkneighb added a commit to gkneighb/llm-d-latency-predictor that referenced this pull request Jun 18, 2026
The batched update stepped alpha_t once per coverage-eval interval, scaling
the effective learning rate by 1/N (N = scores per interval) and leaving the
per-request-tuned gamma nearly inert at the server cadence. Step per score
instead, so gamma carries the same meaning as in the per-request formulation.

Addresses review feedback on llm-d#33.

Signed-off-by: Greg Neighbors <26003+gkneighb@users.noreply.github.com>
gkneighb added a commit to gkneighb/llm-d-latency-predictor that referenced this pull request Jun 18, 2026
Emit {ttft,tpot}_aci_coverage_percent alongside the static coverage and
alpha_t/c_t. Static coverage's deviation from target is the drift signal;
the ACI-adjusted coverage confirms the served interval meets the SLO.

Addresses review feedback on llm-d#33.

Signed-off-by: Greg Neighbors <26003+gkneighb@users.noreply.github.com>
@gkneighb

Copy link
Copy Markdown
Member Author

Thanks for the careful read -- both addressed:

1. Batched alpha_t cadence (the semi-blocker). Fixed in 0b702b5: update_batch now steps per-score rather than once per interval, so the effective learning rate is independent of N and gamma carries its per-request meaning. Your diagnosis was exactly right -- quantified after the fix, one interval of sustained misses drives alpha_t 0.1 -> ~0 (matching the per-request prototype), where the old per-interval step left it at ~0.082. Added two unit tests that lock in per-score stepping (a batch of N misses must move alpha_t ~N x a single miss) so it can't silently regress.

2. Static vs ACI-adjusted coverage. Added in 974e275: /metrics now emits {ttft,tpot}_aci_coverage_percent alongside the static *_coverage_percent and alpha_t/c_t. Exactly your framing -- static deviation = "is there drift?", adjusted = "are we meeting the SLO despite it?".

Still draft until #32 lands; happy to take the line-level pass whenever. Thanks again -- the cadence catch was the difference between gamma being a real knob and inert.

@Whatsonyourmind

Copy link
Copy Markdown

Confirmed — I reproduced both figures independently by replicating update_batch from 0b702b5:

  • gamma=0.02, one interval of sustained (strictly-increasing) misses: per-score → α_t = 0.0, per-interval → α_t = 0.082 — exactly your numbers. α_t clips at 0 after ceil(0.1/0.018) = 6 consecutive misses, so any interval with ≥6 sustained misses pins it to the per-request prototype's value.
  • N-scaling: with gamma=0.001 (unclipped), a 50-miss batch moves α_t 50× a single miss, vs under the old per-interval step — so test_steps_per_score_not_per_batch's >20× assertion has comfortable headroom, and gamma now carries its per-request meaning. The cadence fix is correct.

One forward note for whoever tunes this in prod (not a change request): the step is asymmetric by construction — down-step gamma·(1−target) = 0.018 on a miss vs up-step gamma·target = 0.002 on a cover, a 9:1 ratio. So ACI widens into drift ~9× faster than it relaxes out of it (≈6 misses to saturate α_t→0, ≈50 covers to climb back to 0.1). That asymmetry is a feature — fast to protect coverage, slow to give it back — but it means after a transient spike subsides the served interval stays conservative for a while, since with α_t low the offset rides the buffer's top quantiles until ACI_BUFFER turns over. Worth sizing ACI_BUFFER to the drift timescale you actually care about rather than larger; rebase() on retrain already short-circuits the stale-buffer case, which is the right call.

The static + ACI-adjusted split in /metrics is exactly the framing I'd want. Happy to do the line-level pass on aci.py + the training_server.py hooks the moment #32 lands and this leaves draft.

gkneighb added a commit to gkneighb/llm-d-latency-predictor that referenced this pull request Jun 18, 2026
Document the asymmetric alpha_t step (fast widen / slow relax) and that
ACI_BUFFER should be sized to the drift timescale of interest. Review
follow-up on llm-d#33.

Signed-off-by: Greg Neighbors <26003+gkneighb@users.noreply.github.com>
@gkneighb

Copy link
Copy Markdown
Member Author

Thanks for reproducing it -- good to have the figures independently confirmed.

On the asymmetry: agreed, it's the intended fast-protect / slow-relax behavior, and the 9:1 is just the target baked in -- down-step gamma*(1-target) vs up-step gamma*target = 0.9:0.1 at p90, so it's inherent to ACI rather than a knob you can turn independently of gamma. Your point about the post-transient tail is the practical consequence worth surfacing, so I folded a doc-note into ACI_BUFFER (c69219b): widen-fast/relax-slow means the offset rides the buffer's top quantiles until turnover, so size the buffer to the drift timescale rather than larger; rebase() covers the retrain case.

Ready for the line-level pass whenever #32 lands and this leaves draft -- appreciate the thorough look.

@Whatsonyourmind

Copy link
Copy Markdown

Nice — the ACI_BUFFER doc-note captures it. One tension worth a line in there, since it lands on the same buffer: the window size is doing double duty — it sets the drift-tracking horizon (your point) and the quantile-estimation stability. Size it purely to a short drift timescale and the top-quantile offset is estimated from very few samples, so coverage gets noisy exactly in the regime where widen-fast is firing most. So the floor isn't "larger = laggier," it's "small enough to track drift, but ≥ the sample count your p90 needs to be a stable quantile" — and rebase() is the right escape hatch when a regime shift makes those two irreconcilable.

Happy to do the line-level pass once #32 lands and this leaves draft — ping me.

@github-actions

Copy link
Copy Markdown

This PR is marked as stale after 21d of inactivity. After an additional 14d of inactivity (7d to become rotten, then 7d more), it will be closed. To prevent this PR from being closed, add a comment or remove the lifecycle/stale label.

@gkneighb

Copy link
Copy Markdown
Member Author

Not stale -- this is intentionally stacked on #32 and waiting for it to merge first (its base commit is in #32). #32 has the owner's review feedback addressed and is awaiting re-review; I'll rebase and undraft this the moment it lands.

/remove-lifecycle stale

gkneighb added a commit to gkneighb/llm-d-latency-predictor that referenced this pull request Aug 3, 2026
Rebased onto the component-directory restructure (llm-d#16) and stacked on
the rebased llm-d#32: aci.py moves to training/aci.py, imported as
training.aci.

One-sided upper ACI (Gibbs & Candes 2021) widens the prediction
interval between retrains so SLO coverage holds through detection lag.
Opt-in via LATENCY_ACI_ENABLED (default false); exposes aci_alpha,
aci_offset_ms, and the ACI-adjusted coverage series in /metrics.

Union-merged cleanly with llm-d#32's live-coverage additions (both metric
families coexist). Squashes the prior four ACI commits; review history
is in PR llm-d#33.

Signed-off-by: Greg Neighbors <26003+gkneighb@users.noreply.github.com>
@gkneighb
gkneighb force-pushed the feat/aci-conformal branch from c69219b to 7ce4522 Compare August 3, 2026 03:44
Rebased onto the component-directory restructure (llm-d#16): the module
moved to training/, and calibration lives at training/calibration.py
imported as training.calibration.

Adds a continuous coverage loop that re-evaluates the loaded model
between scheduled retrains and fires an early retrain when an EMA of
|coverage - target| stays over threshold for k evaluations. The pure
CalibrationTrigger carries the decision logic; live coverage lands in
its own metric family; the detector releases its pending state on a
retrain that concludes without landing a model. Opt-in via
LATENCY_COVERAGE_EVAL_INTERVAL_SEC (default 0 = disabled).

Squashes the prior four commits on this branch; review history is
preserved in PR llm-d#32.

Signed-off-by: Greg Neighbors <26003+gkneighb@users.noreply.github.com>
Rebased onto the component-directory restructure (llm-d#16) and stacked on
the rebased llm-d#32: aci.py moves to training/aci.py, imported as
training.aci.

One-sided upper ACI (Gibbs & Candes 2021) widens the prediction
interval between retrains so SLO coverage holds through detection lag.
Opt-in via LATENCY_ACI_ENABLED (default false); exposes aci_alpha,
aci_offset_ms, and the ACI-adjusted coverage series in /metrics.

Union-merged cleanly with llm-d#32's live-coverage additions (both metric
families coexist). Squashes the prior four ACI commits; review history
is in PR llm-d#33.

Signed-off-by: Greg Neighbors <26003+gkneighb@users.noreply.github.com>
@gkneighb
gkneighb force-pushed the feat/aci-conformal branch from 7ce4522 to e3bac21 Compare August 3, 2026 17:34
@github-actions

Copy link
Copy Markdown

This PR is marked as stale after 21d of inactivity. After an additional 14d of inactivity (7d to become rotten, then 7d more), it will be closed. To prevent this PR from being closed, add a comment or remove the lifecycle/stale label.

@gkneighb

Copy link
Copy Markdown
Member Author

Still active -- intentionally stacked on #32 and waiting for it to merge first. #32 is rebased onto the current layout, CI-green, and has the owner's review feedback addressed; awaiting re-review. I'll rebase and undraft this the moment #32 lands.

/remove-lifecycle stale

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants