fix(kernels): parameterise the spectral measure by lengthscale and dimension - #702
Merged
Merged
Conversation
…mension `StationaryKernel.spectral_density` returned a standardised distribution — `Normal(0, 1)` for RBF, `StudentT(2v, 0, 1)` for Matern — that ignored the lengthscale and was hard-coded to one dimension. Every lengthscale produced the same curve, so `kernel.spectral_density.log_prob(w)` was wrong for any direct consumer (closes #612). RFF was not affected: `BasisFunctionComputation.compute_features` divided the sampled frequencies by the lengthscale and `scaling()` applied the variance, so the approximation compensated downstream. That compensation is what hid the bug, and it is now unnecessary. The measure is D-dimensional and carries diag(l)^-1 as its scale, honouring ARD lengthscales, and satisfies Bochner's theorem k(t) = s^2 E_p(w)[e^{i w.t}]. The variance stays out of it: sigma^2 is the measure's total mass, which normalisation divides out, and it re-enters as the explicit prefactor. Because scaling by a diagonal matrix is exactly elementwise division, sampled frequencies are bit-identical to before — verified across dimensions and lengthscales in `test_rff_effective_frequencies_unchanged`. The isotropic-vs- tensor-product branch in `RFF.__init__` collapses, since MVN and MVT now both arrive correctly shaped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bj9k5fnAZ8JzD4Rg3HMDMj
thomaspinder
temporarily deployed
to
docs-preview
July 26, 2026 13:45 — with
GitHub Actions
Inactive
3 tasks
thomaspinder
enabled auto-merge (squash)
July 26, 2026 14:16
thomaspinder
temporarily deployed
to
docs-preview
July 26, 2026 15:03 — with
GitHub Actions
Inactive
thomaspinder
temporarily deployed
to
docs-preview
July 26, 2026 16:02 — with
GitHub Actions
Inactive
3 tasks
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.
Checklist
uv run poe formatbefore committing.Description
Fixes the spectral density reported by @ahwillia in #612.
StationaryKernel.spectral_densityreturned a standardised distribution —Normal(0, 1)for RBF,StudentT(2ν, 0, 1)for Matérn — that ignored the lengthscale and was hard-coded to one dimension. Every lengthscale gave the same curve, which is exactly the superimposed plot in the issue.Why RFF still worked
BasisFunctionComputation.compute_featuresdivided the sampled frequencies by the lengthscale, andscaling()applied the variance:So the approximation compensated downstream and was numerically correct. That compensation is what kept the bug latent — the only in-repo consumer of
spectral_densitywas the one place that cancelled the error out. Anyone using the property directly got a wrong answer silently.The fix
The measure is now
D-dimensional and carriesdiag(ℓ)⁻¹as its scale (ARD lengthscales included), and the compensating division is removed fromcompute_features. It satisfies Bochner's theoremThe variance deliberately stays out of the measure: σ² is its total mass, which normalisation divides out, and it re-enters as the explicit prefactor above. A numpyro
Distributioncannot represent the unnormalised S(ω) —log_probis normalised by construction — so the docstring pins the relation to R&W §4.2.1 instead:I verified that formula reproduces R&W's closed form for the RBF in 1-D and 3-D.
Compatibility
RFF Gram and cross-covariance values are bit-identical to before. Scaling by a diagonal matrix is exactly elementwise division, so the frequencies are unchanged —
test_rff_effective_frequencies_unchangedassertsjnp.array_equalagainst the old two-step route across dimensions and lengthscales. The lengthscale simply moved from the feature map into the measure it is drawn from.Two API changes, both noted in the CHANGELOG:
spectral_densitynow returnsMultivariateNormal/MultivariateStudentTwithevent_shape == (D,). Callers doing.sample(key, (M, D))should use.sample(key, (M,)).frequencies=now treats them as the frequencies ω directly. They were previously divided by the lengthscale, silently rescaling user-supplied values — arguably a second latent bug.Tests
New
tests/test_kernels/test_spectral_density.py(92 cases). The load-bearing one is a Monte-Carlo Bochner check across RBF/Matérn12/32/52 × D ∈ {1,2,3} × ℓ ∈ {0.5,1,2} × σ² ∈ {1,3} — it pins the parameterisation without reference to any Fourier normalisation convention, so it constrains the measure independently of how it's implemented. Plus the reporter's reproducer, dimensionality, ARD anisotropy, and variance-independence.uv run poe all-tests: 2582 passed. (The one failure in my working tree istest_root_goodpicking up an untracked local scratch.mdin the repo root — unrelated, and not present in CI.)Follow-ups this unblocks
S(ω)accessor; I kept this PR to the reported bug.Issue Number: #612
🤖 Generated with Claude Code
https://claude.ai/code/session_01Bj9k5fnAZ8JzD4Rg3HMDMj