Skip to content

Add VFF Inducing Structure (re-opened from #2) - #3

Merged
bwengals merged 9 commits into
pymc-devs:mainfrom
thomaspinder:vff-implementation-v2
May 19, 2026
Merged

Add VFF Inducing Structure (re-opened from #2)#3
bwengals merged 9 commits into
pymc-devs:mainfrom
thomaspinder:vff-implementation-v2

Conversation

@thomaspinder

Copy link
Copy Markdown
Contributor

Re-opens #2 after the force-push to main broke shared history. Same 7 commits cherry-picked onto the new upstream/main, plus one preservation commit for the VFF demo content (see below).

Summary

  • Adds FourierFeatures1D inter-domain inducing variable (Matérn12/32/52) and the corresponding Kuu_solve / Kuu_sqrt_solve / Kuu_logdet structured-solve API on InducingVariables.
  • Refactors base_conditional into conditional_whitened / conditional_unwhitened so SVGP can pass structured solves without materialising Kmm.
  • Adds gauss_kl_structured for the unwhitened KL with a structured prior covariance.
  • Adds VFF tests + e2e + boundary-domain regression tests + KL/inducing-fourier oracle fixtures.

Conflict resolutions during cherry-pick

Histories were disjoint after the force-push (no merge-base), so this was a clean cherry-pick. The conflicts I resolved (all reviewable in the diff):

  • ptgp/conditionals.py — kept the new two-helper structure, moved the new _DEFAULT_JITTER logic into the back-compat base_conditional wrapper so existing callers still get jitter.
  • ptgp/inducing.py — accepted upstream's greedy_variance_init (now returns (Points, GreedyVarianceDiagnostics)).
  • tests/test_inducing.py — accepted upstream's tuple-unpacking of random_subsample_init.
  • ptgp/gp/svgp.py — kept upstream's variational_params docstring + added the "or structured inducing variables" hint.
  • ptgp/optim/training.py — accepted upstream's new init / init_rng / frozen_vars kwargs.
  • notebooks/demo.ipynb — accepted upstream's version (it had the API renames I'd be conflicting with). To preserve the VFF demo + Wasserstein-distance comparison content, I added it as notebooks/vff_demo.ipynb (last commit) with a header note flagging that the cells need API porting before re-execution.

Test plan

  • CI runs on this branch
  • Manual: pytest tests/ with the assumption-system PyTensor + v6 PyMC envs
  • Port vff_demo.ipynb to the new variational_params / sigma= API (follow-up)

Closes #2 in spirit (cherry-picked, not history-shared).

thomaspinder and others added 9 commits May 7, 2026 04:58
Two scale-unlock features for SVGP on big-N data, planned as independent
PRs: 1D-Matérn variational Fourier features via a new InducingVariables
subclass with a Woodbury solve_Kuu bridge, and an exact closed-form
variational expectation for Poisson with exp link.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Four findings addressed:

- PR 2 (Poisson): the original plan proposed adding an analytic VE that
  already exists in ptgp/likelihoods/poisson.py. Rescoped to hardening
  (end-to-end SVGP test, gradient correctness, graph-level regression
  assertion). Dropped the invlink-callable→link-string API change, which
  would have been a silent breaking change for users passing custom
  invlinks.

- PR 1 (VFF) domain validation: specified runtime checks for training
  and prediction, a default refusal to extrapolate, and an opt-in
  allow_extrapolation escape hatch with explicit tests.

- PR 1 (VFF) Kuu contract: split K_uu (dense) from structured_Kuu
  (optional typed DiagPlusLowRank). solve_Kuu is the only dispatch
  point, so a buggy consumer calling K_uu on a VFF object gets a slow
  but correct result, not a shape-incompatible structured tuple.

- Algorithmic-path tests: both PRs gain CI-asserted graph-introspection
  tests that verify the fast path actually runs in the compiled graph,
  rather than relying on wall-clock notebooks.

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

Copy link
Copy Markdown
Collaborator

Really stoked to add this!

I'll fix these myself post-merge (they come from us being on a newer pytensor commit):

  • pt.specify_assumptionspt.assume (renamed upstream)
  • sys.setrecursionlimit(50000) in the test files, no longer needed on the current pytensor branch
  • Duplicate diag method in ptgp/kernels/base.py, the old abstract one needs to be removed now that you've added the fallback

Question:
demo.ipynb now has VFF content added (which is great), but vff_demo.ipynb looks very similar. What's the intended difference between them? Should we keep both?

Followup:
I'm going to make an issue about how to handle the InducingVariables abstraction going forward (thinking about VFF + the spherical harmonics implementation that's coming).

@thomaspinder

Copy link
Copy Markdown
Contributor Author

Hey Bill, demo.ipynb was not running e2e. I think vff_demo can actually be removed. Originally demo.ipynb was not running e2e and i didn't want to introduce scope creep by changing the notebook and introducing VFF. If you're happy, then I'll remove vff_demo from this PR?

@bwengals

Copy link
Copy Markdown
Collaborator

Oh your call, if its a dup then best to remove. I've just been using demo.ipynb as scratch or an example use case I can interactively double check / mess with / look at how the api looks in practice while messing around. If you want to have vff as a seperate nb totally ok, probably better to split it up actually. My plan is to delete it once there are proper examples and docs.

@bwengals
bwengals merged commit 183a7ff into pymc-devs:main May 19, 2026
@bwengals

Copy link
Copy Markdown
Collaborator

thanks dude! sorry for the hassle again

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