Skip to content

perf: cache L in Constant adapter, skip Σ re-decomposition (closes #68) - #69

Merged
thomaspinder merged 3 commits into
mainfrom
fix/constant-cache-cholesky-factor
May 18, 2026
Merged

thomaspinder merged 3 commits into
mainfrom
fix/constant-cache-cholesky-factor

Conversation

@thomaspinder

Copy link
Copy Markdown
Collaborator

Summary

  • Constant.build_pymc_latent now registers L as a pm.Deterministic alongside Sigma
  • Constant.cholesky_at reads posterior["L"] directly instead of calling np.linalg.cholesky(Σ) on every invocation; this propagates to cholesky_path and forecast_cholesky_path which delegate to it
  • Posterior gains one new variable (L); existing consumers (Sigma, B, intercept, sigma_sd, tril_offdiag) are byte-for-byte unchanged

Closes #68.

Test plan

  • pytest tests/test_volatility.py -v — 20/20 pass, including both @pytest.mark.slow TestPosteriorEquivalence MCMC tests
  • pytest -m "not slow" -q — 227 passed, 22 deselected, no regressions
  • ruff check src/ tests/ — clean
  • ruff format --check src/ tests/ — clean
  • ty check — clean
  • New TestConstantCholeskyAt::test_reads_cached_L_not_decomposed_sigma pins the "reads cached L, never re-decomposes Σ" contract by feeding deliberately-inconsistent L/Σ to the adapter
  • TestPosteriorEquivalence::test_explicit_constant_matches_string_default extended to include L in the byte-for-byte equality check across volatility= default / "constant" / Constant()

🤖 Generated with Claude Code

thomaspinder and others added 2 commits May 18, 2026 20:02
Register the Cholesky factor L of Σ as a pm.Deterministic alongside
the existing Sigma deterministic. Loop 1 of the #68 fix: posterior
now carries L directly so downstream cholesky_at can read it without
re-decomposing Σ on every call (next commit).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…loses #68)

The Constant adapter's cholesky_at now reads posterior["L"] directly,
which the previous commit exposed as a pm.Deterministic. Removes the
batched np.linalg.cholesky call from every cholesky_at invocation and
from the cholesky_path/forecast_cholesky_path broadcasts that delegate
to it.

Updates:
- synthetic_idata_2v fixture now carries L alongside Sigma so
  hand-rolled posteriors mirror real Constant-adapter output
- TestPosteriorEquivalence byte-for-byte set extended to include L
- New TestConstantCholeskyAt::test_reads_cached_L_not_decomposed_sigma
  pins the contract by using inconsistent L vs. Σ in the posterior

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov-commenter

codecov-commenter commented May 18, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.4%. Comparing base (6e8aad4) to head (8ff239a).

Additional details and impacted files
@@          Coverage Diff          @@
##            main     #69   +/-   ##
=====================================
  Coverage   92.3%   92.4%           
=====================================
  Files         26      26           
  Lines       1127    1141   +14     
  Branches     107     111    +4     
=====================================
+ Hits        1041    1055   +14     
  Misses        65      65           
  Partials      21      21           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@thomaspinder
thomaspinder merged commit 33d4caf into main May 18, 2026
14 checks passed
@thomaspinder
thomaspinder deleted the fix/constant-cache-cholesky-factor branch May 18, 2026 19:11
thomaspinder added a commit that referenced this pull request May 18, 2026
Wraps up the spring-cleaning stack. Individually small fixes that
cluster naturally into one style PR.

- src/impulso/volatility.py: `Constant.cholesky_path` /
  `forecast_cholesky_path` docstrings reference `L` (the actual
  posterior key after #69) instead of `Sigma`; cross-ref
  `Constant.cholesky_at` as the canonical accessor.
- src/impulso/sv/data.py: `_MIN_OBS` is now `ClassVar[int] = 24` —
  makes the class-attribute intent explicit instead of relying on
  Pydantic's implicit underscore handling.
- src/impulso/fitted.py: `FittedVAR.has_exog` drops `@computed_field`
  (now a plain `@property`) so it stops appearing in `model_dump()`.
- src/impulso/sv/dynamics.py: `RandomWalk.build_latent_path` /
  `AR1.build_latent_path` swap `Any` for `"pt.TensorVariable"` forward
  refs that match the SVDynamics Protocol declaration; drop the
  now-unused `Any` import.
- src/impulso/results.py: delete `VARResultBase.__repr__` override —
  Pydantic's default produces a useful compact repr because every
  `idata` field has `Field(repr=False)`.
- src/impulso/**/*.py: docstring sweep — double backticks (RST) → single
  backticks (Markdown / mkdocstrings); no more `:meth:` / `:class:`
  cross-refs (replaced with plain backticked names in earlier passes).
- CLAUDE.md: codify the docstring convention under "Code Conventions".
- src/impulso/identified.py: delete
  `forecast_error_variance_decomposition`; `fevd` is the canonical
  name. Docstring + .qmd / .md cross-references updated.
- pyproject.toml + uv.lock: bump `0.0.5 → 0.0.6` to ship the cumulative
  audit cleanup (#73 + #75 + #77 + this PR).

Closes audit findings #5, #8, #12, #13, #14, #15, #16
(reports/audit-2026-05-18.md).

Breaking (pre-v0.1): callers of
`IdentifiedVAR.forecast_error_variance_decomposition` must use
`IdentifiedVAR.fevd` (no signature change). The long name was always
an alias; per repo policy pre-v0.1 API breaks are in scope.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
thomaspinder added a commit that referenced this pull request May 18, 2026
Wraps up the spring-cleaning stack. Individually small fixes that
cluster naturally into one style PR.

- src/impulso/volatility.py: `Constant.cholesky_path` /
  `forecast_cholesky_path` docstrings reference `L` (the actual
  posterior key after #69) instead of `Sigma`; cross-ref
  `Constant.cholesky_at` as the canonical accessor.
- src/impulso/sv/data.py: `_MIN_OBS` is now `ClassVar[int] = 24` —
  makes the class-attribute intent explicit instead of relying on
  Pydantic's implicit underscore handling.
- src/impulso/fitted.py: `FittedVAR.has_exog` drops `@computed_field`
  (now a plain `@property`) so it stops appearing in `model_dump()`.
- src/impulso/sv/dynamics.py: `RandomWalk.build_latent_path` /
  `AR1.build_latent_path` swap `Any` for `"pt.TensorVariable"` forward
  refs that match the SVDynamics Protocol declaration; drop the
  now-unused `Any` import.
- src/impulso/results.py: delete `VARResultBase.__repr__` override —
  Pydantic's default produces a useful compact repr because every
  `idata` field has `Field(repr=False)`.
- src/impulso/**/*.py: docstring sweep — double backticks (RST) → single
  backticks (Markdown / mkdocstrings); no more `:meth:` / `:class:`
  cross-refs (replaced with plain backticked names in earlier passes).
- CLAUDE.md: codify the docstring convention under "Code Conventions".
- src/impulso/identified.py: delete
  `forecast_error_variance_decomposition`; `fevd` is the canonical
  name. Docstring + .qmd / .md cross-references updated.
- pyproject.toml + uv.lock: bump `0.0.5 → 0.0.6` to ship the cumulative
  audit cleanup (#73 + #75 + #77 + this PR).

Closes audit findings #5, #8, #12, #13, #14, #15, #16
(reports/audit-2026-05-18.md).

Breaking (pre-v0.1): callers of
`IdentifiedVAR.forecast_error_variance_decomposition` must use
`IdentifiedVAR.fevd` (no signature change). The long name was always
an alias; per repo policy pre-v0.1 API breaks are in scope.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

This branch was previously deployed

1 inactive deployment
docs-preview — 8ff239ad Deployed May 18, 2026 by thomaspinder via deploy-docs-preview #149
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.

Constant.cholesky_at re-decomposes Σ on every call — cache L

2 participants