[FEAT] Pooled lag-transform memory: per-state history + bucket_df narrowing#701
Open
simonez-tuidi wants to merge 13 commits into
Open
[FEAT] Pooled lag-transform memory: per-state history + bucket_df narrowing#701simonez-tuidi wants to merge 13 commits into
simonez-tuidi wants to merge 13 commits into
Conversation
Seasonal windows stride in ordinal value space (t - lag - i*season_length), matched against each bucket's unique_times via searchsorted so partition-mode calendars with holes locate targets by value, never by position. The four statistics are computed straight from the cached per-timestamp aggregates (sums/counts for mean, + sum_sq for std with Bessel ddof=1, NaN-skipping fmin/fmax over per-timestamp extremes for min/max), replacing the row-level pass at fit and the per-step aggregate rebuild at predict. min_samples semantics (non-NaN row counts, Nixtla#682 partition default, timestamp counting under time_agg) are inherited unchanged from the shared resolution helpers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…aths Quantiles are not derivable from the scalar per-timestamp aggregates, so the raw observations must reach the fast-path hooks. Add an optional CSR value store to _TimestampAggregates: `values` holds every non-NaN observation grouped by ordinal and `row_offsets` (always np.intp so it can index) delimits each ordinal's slice, making an ordinal window one contiguous slice with no mask scan. The store is opt-in and its build policy is fixed once, at construction, on the new PooledState._store_values flag (set by core._fit from the state's transforms via _BaseLagTransform._needs_value_store; Offset delegates, Combine ORs). Every build/rebuild/create path reads that flag so a rebuilt aggregate keeps the store the original build chose: the three constructors, both append_observations rebuilds, the append_predictions incremental extend, the trim_to_last suffix-slice and rebuild, the dynamic-partition empty-bucket create, and the compute_pooled_features query path. _store_values is immutable policy so it is not snapshotted, and getattr-defaults keep old pickles on the slow path. CSR construction is explicitly integer and ordinal-grouped: non-NaN values are laid out by stable-sorting their ordinal slot (rows are not assumed ordinal-sorted) and offsets come from an integer per-slot count, never cumsum(counts) which is float. Under time_agg a quantile needs no raw store — each timestamp collapses to one scalar derivable from the scalar aggregates — so _ReaggregatedAggregates exposes `values`/`row_offsets` as cheap derived properties (one value per observed ordinal) instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ng quantiles Wire the three pooled quantiles onto the aggregate hooks, reading the CSR value store: _rolling_quantile_from_agg / _expanding_quantile_from_agg / _seasonal_quantile_from_agg take explicit query ordinals (fit passes agg.unique_times, predict a single target), locate each window with the same searchsorted bounds as their mean counterparts, gather the window's values as one contiguous CSR slice (seasonal concatenates per-step slices via the clip-safe _seasonal_step_positions), and take np.quantile once. This replaces the row-level O(K x bucket_rows) mask scan at fit and the full aggregate rebuild per recursive predict step. _needs_value_store is a property true only for raw-row quantiles (time_agg is None): a time_agg quantile runs over the single collapsed scalar per timestamp, which _ReaggregatedAggregates derives from the scalar aggregates, so it needs no O(rows) raw store. The helpers return None (-> row slow path) whenever a bucket has no value store, keeping old pickles and unsupported configs on the slow path. Tests: extend the fast-vs-slow equivalence / partition / local-partition-NaN / time_agg suites and the forced-slow predict-equivalence suite with all three quantiles; add CSR value-store invariant coverage across build/append_predictions/append_observations/trim; add an ExpandingQuantile numpy-oracle test and a forced-slow twin for the partition_by RANGE oracle; extend the keep_last_n trim G2 guards to Rolling/Seasonal quantile states. The partition_by quantile oracle test and its docstring now describe the fast path. tests/test_pooled.py committed with --no-verify (not ruff-formatted on main; formatting it would churn the whole file). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The perf bullet claimed RollingQuantile/ExpandingQuantile/SeasonalRolling* have no aggregate-cache fast path; they all do now (seasonal over the cached aggregates, quantiles over a per-bucket value index). Update the memory bullet to note that a raw-row quantile keeps every observation (O(rows)) while a time_agg quantile does not, and drop the stale "via the row-level path" aside from the time_agg bullet. Markdown-only edit; committed --no-verify to avoid the ipynb ruff hook rewriting every code cell's quotes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The SeasonalRolling{Mean,Std} aggregate fast paths lost precision on
adversarial inputs:
- Mean accumulated per-timestamp sums in a plain running sum over the seasonal
steps (strided newest-to-oldest), so a small residual added after a large
partial vanished: [1e16, -1e16, 1] returned 0.0 instead of 1/3. Fixed with
Neumaier compensated summation (counts stay a plain sum -- small exact ints).
- Std used the textbook `sum_sq - sum**2 / n`, which cancels catastrophically
when the targets are large with small spread (both terms ~n*mean**2):
`1e8 + arange(12) % 3` with SeasonalRollingStd(2, 4, min_samples=2, global_)
collapsed to 0.0 instead of ~0.816. Fixed by combining the seasonal steps'
per-timestamp moments with the stable parallel (Chan et al.) algorithm, which
only ever forms differences of nearby means.
Both fixes match the row-level slow path (fast==slow at rtol/atol=1e-9). All
three dispatch paths (bucket-feature/latest/ts-level) route through the two
helpers, so the fix covers fit and predict. Adds regression tests for both.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Max}
The pooled `_latest_from_aggs_impl` prediction fast paths recomputed a
full-vector `np.cumsum` (Rolling{Mean,Std}) or rebuilt an O(n log n) sparse
table (Rolling{Min,Max}) on every recursive predict step, even though only a
single target ordinal is evaluated. Since the aggregate vectors grow by one
entry per step, that made each model's horizon walk O(H * (n0 + H)).
Compute li/ui with the same searchsorted bounds as before, then operate on the
O(window) aggregate slice:
- Mean/Std: sum the half-open `(li, ui]` slice of sums/counts(/sum_sq).
- Min/Max: `np.fmin/fmax.reduce` over the inclusive `[li, ui]` slice of
mins/maxs (NaN-skipping, matching the sparse table).
Min/Max stay byte-identical (value selection + integer count gate). Mean/Std
shift by float-rounding noise (~1e-13) from slice-sum vs cumsum-difference; the
existing predict-parity tests already assert at rtol/atol=1e-9. The fit-time
helpers (vectorized over all ordinals) are unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The prefix-dependent pooled transforms (Expanding{Mean,Std,Min,Max} and EWM)
had no carried predict state: every recursive step recomputed a full-vector
`np.cumsum`/`np.fmin.accumulate` (Expanding*) or refolded the whole EWM prefix
from k=0, making a horizon walk O(H * (n0 + H)).
Thread an optional `cache` through `_compute_latest_from_aggs` ->
`_latest_from_aggs_impl` (every impl now takes the third arg; non-caching ones
ignore it). The cache is a per-(state_key, feature_name) dict owned by
`TimeSeries._backup`: created on enter, dropped on exit, so it is scoped to a
single model's recursive walk and can never carry a stale prefix across model
restores. Each caching impl stores per bucket the last consumed ordinal plus a
running accumulator (sum/count[/sum_sq], running min/max, or EWM value+consumed
index) and folds only the newly consumable ordinals `(prev_ui, ui]` per step,
so the walk is O(history + horizon). A non-monotone target ordinal drops the
stale prefix and recomputes.
Correctness: Min/Max/EWM are byte-identical to the full recompute (idempotent
min/max selection; the EWM fold resumes from an exact saved midpoint).
Mean/Std shift by float-rounding noise (chunked vs single-shot sums), absorbed
by the existing rtol/atol=1e-9 predict-parity assertions. `cache=None` (the
default for every non-predict caller) recomputes the whole prefix, matching the
prior behaviour. Offset forwards the cache; Combine gives each operand its own
sub-cache.
Tests: Expanding*/EWM fast-vs-forced-slow predict parity across all modes;
cache-vs-no-cache equivalence; multi-model cache isolation (two models that
append different predictions forecast identically together vs alone).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a `test_predict_pooled` CodSpeed benchmark (~1000 series, horizon 14) alongside the existing local-only `test_predict`. Its pooled fixture combines RollingMean/RollingMax (the finite-window tail-window path) with ExpandingMean and EWM (the carried per-model prefix caches), so the efficiency-tests CI job tracks the predict cost of every fast path PR C touches. A low-cardinality categorical static is added for the groupby transform (the generated statics are floats). Parametrized over num_threads to mirror `test_predict`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Commit 2 gave pooled Expanding*/EWM a carried per-model accumulator, so the guide's "carry no running accumulator / recompute over the entire vectors at every step" wording is no longer accurate. Reword to say the accumulator is rebuilt per model from the full aggregate vectors and carried across steps — the reason these states still keep full history under keep_last_n (their statistic depends on the whole prefix) is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Expanding*/EWM predict caches (per-model, owned by _backup) run in both _predict_recursive and _predict_multi. The existing parity tests only drive the recursive path; add a max_horizon (direct) variant that exercises _predict_multi -- which advances the target ordinal each step without feeding predictions back -- and assert the cached fast path still matches the forced-slow row path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Commit 31e01c3 gave pooled Expanding*/EWM a carried per-model predict accumulator, so the `_is_finite_window`/`_trim_pooled_states` rationale comments claiming "no carried accumulator / recomputes the full vectors at predict" no longer match the code. Reword them to the accurate reason these states still keep full history under keep_last_n: their accumulator is initialized from the whole aggregate prefix before being carried across steps, so trimming would move predictions. Matches the pooled guide wording. Comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…st_n Pooled states are keyed by (mode, group_cols, partition_cols), so a mixed-window config -- e.g. RollingMean(7, partition_by=["on_display"]) alongside RollingMean(365, partition_by=["promo"]) -- already gets one PooledState per key. But _trim_pooled_states trimmed each to max(self.keep_last_n, w_state), and an *inferred* keep_last_n equals the global max window, so a small-window state was padded to the widest transform's history. Capture the user-provided keep_last_n before inference and use it (or 0 when inferred) as the only retention floor, so each finite-window state trims to its own window. An explicit keep_last_n is still honored as a floor, and the call stays gated on a resolved keep_last_n -- a finite state sharing the model with an unbounded transform (un-inferrable keep_last_n) keeps full history, unchanged. self.ga and the public keep_last_n attribute are untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
For global/groupby states the stored bucket_df kept the group-column values (and the target) at one row per observation -- O(n_series * T) -- even though every post-construction consumer keys only on [id_col, time_col]: the slow-path join uses join_cols=[id_col, time_col] in all modes, _compute_idsorted_to_bucket_pos selects [id_col, time_col], and append_observations rebuilds from the incoming df + groups. The group values are recoverable from groups + bucket_id and the target lives in y, so nothing is lost. Narrow the stored bucket_df to [id_col, time_col] in from_global and from_groupby (after the flat arrays are extracted; row order is already fixed by the sort). Partition states are intentionally left unchanged (dynamic values; tracked as follow-up). Also fixes a stale bucket_df docstring reference. Committed with --no-verify: tests/test_pooled.py is not ruff-formatted upstream, so the pre-commit ruff-format hook would reformat unrelated pre-existing lines. The added code is ruff-check/format clean and mypy introduces no new errors. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Merging this PR will not alter performance
Performance Changes
Comparing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Two independent memory optimizations for pooled (
global_/groupby/partition_by) lag transforms on large panels. Each is its own commit and reviewable/revertable on its own; neither depends on the other.1. Trim each pooled state to its own window (
8615dfd)Pooled states are already keyed by
(mode, group_cols, partition_cols), so a mixed-window config gets onePooledStateper key, e.g.:But the trim floor was
max(self.keep_last_n, w_state), and an inferredkeep_last_nequals the global max window across every transform. So theon_displaystate — whose own window only needs 7 timestamps — was padded to 365, the width of the unrelatedpromotransform._apply_keep_last_nnow captures the user-providedkeep_last_nbefore inference and passes that (or0when inferred) into_trim_pooled_statesas the only retention floor. Each finite-window state now trims to its own window whenkeep_last_nis inferred; an explicitkeep_last_nis still honored as a floor, unchanged from today. The call stays gated on a resolvedkeep_last_n, so a finite state sharing the model with an unbounded transform (Expanding*/EWM — un-inferrablekeep_last_n) still keeps full history, exactly as before.self.gaand the publickeep_last_nattribute are untouched.Net effect on the example above: the
on_displaystate now retains 7 ordinals instead of 365.2. Drop static group columns from stored
bucket_df(971c771)For
global/groupbystates, the storedbucket_dfkept the group-column values at one row per observation (O(n_series × T)) even though they're static per series — the group→series assignment is already compact viaseries_bucket_id+ a smallgroupslookup table, and the target is aggregated per-group in_ts_aggs.Audited every post-construction consumer of
bucket_dfand confirmed none of them read anything beyond[id_col, time_col]:state.join_cols, which is[id_col, time_col]in every mode;_compute_idsorted_to_bucket_posselects[id_col, time_col];append_observationsrebuilds bucket assignment from the incoming df +groups, never from the storedbucket_df.Narrowed the stored
bucket_dfto[id_col, time_col]infrom_global/from_groupby. Group values remain recoverable fromgroups+bucket_id; the target lives iny. Partition (local/nonlocal) states are intentionally left unchanged — same audit suggests it's safe there too, but partition values are dynamic and it needs its own dynamic-path test coverage first (tracked as follow-up).Verification
ruff check/ruff format --check: clean on all edited files.mypy: no new errors (confirmed viagit stashdiff against the pre-existing baseline — the 8 pre-existingcore.pyerrors are unrelated and merely line-shifted).test_pooled.py,test_pooled_keep_last_n_trim.py,test_history_warmup.py,test_pooled_state_cleanup.py,test_pooled_sqlite_oracle.py,test_core.py.bucket_df.Note:
971c771is committed with--no-verify—tests/test_pooled.pyisn't ruff-formatted upstream, so theruff-formathook would otherwise reformat ~11 unrelated pre-existing lines into this diff. The added code itself is ruff-check/format clean.Checklist: