CliMA / CliMA/EnsembleKalmanProcesses.jl

[math-audit-2026-07-09] Summary of math-audit issues

Open
#584 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Julia
Stars
125
Forks
24
Avg merge
1d 18h
Merged PRs (30d)
5

Description

## Issue

A lot of bugs/silent behaviours found, and new fixes/guardrails/potential-improvements found during the review:

## Unscented variants (UKI / UTKI)
Sigma-point construction, prior-precision scaling, and the modified-transform contract need to match the cited literature's conventions exactly.
- `impose_prior=true` biased the converged estimate toward the prior by double-counting its precision.
- The sigma-point SVD fallback factored the wrong matrix and could throw on a collapsed covariance.
- UTKI crashed when constructed without a prior, and silently diverged from UKI when the non-modified unscented transform was selected.
- A failed *center* sigma point still had its contribution zeroed out, and the very first analysis step transiently pairs an uninflated prior covariance with doubled noise.
- The prediction step for one iteration and its analysis use inconsistent timesteps under adaptive scheduling.
- Negative center weights and momentum-reconstructed covariances are unguarded and can go indefinite, routing into the (fixed) broken SVD fallback.
- Assorted dead code, a mislabeled comment, and failsafe closures capturing the wrong process reference.

## Kalman-inversion variants (EKI, Sparse/QP EKI, ETKI, GNKI)
Timestep scaling, prior-as-extra-observation handling, and update-group subsetting need to be applied consistently across every inversion variant.
- Stochastic EKI never perturbed the imposed-prior block, causing systematic ensemble under-dispersion.
- The Sparse QP formulation ignored the scheduler's timestep in its data-misfit weighting.
- SparseInversion and GNKI both silently ignored update groups instead of properly subsetting.
- GNKI's stationary covariance was off by a scaling factor from the true posterior.
- A threaded loop in SparseInversion draws from a shared RNG, causing a data race.
- ETKI uses the wrong (conditional rather than marginal) prior precision under update groups, and localizers are passed a stale ensemble size after member failures.
- Covariance-normalization convention (1/N vs. 1/(N−1)) and an absolute regularization nugget drift inconsistently across process types.
- SparseInversion computes a vanilla Kalman update that is immediately discarded, and a regularization guard applied to a sliced copy was a no-op.

## Core `EnsembleKalmanProcess` (inflation, indexing, diagnostics)
Index arithmetic, inflation ordering, and diagnostic formulas in the shared core need internal consistency and guards against degenerate states.
- Minibatch-to-update-group index extension assumes all observations are equal length, silently misaligning data otherwise.
- Inflation validates its stability condition only after already mutating the ensemble store, and separately crashes on a fresh process with no timestep history yet.
- Multiplicative inflation is a silent no-op unless a scale parameter is also passed, and additive inflation takes an unguarded square root of the covariance.
- A getter silently adds a regularization nugget to the sample covariance, contradicting its own docstring.
- Two loss/error metrics normalize by different factors, so the meaning of the reported error changes depending on whether a prior was supplied.
- The CRPS diagnostic returns NaN on ensemble collapse and contains a dead code branch.
- NaN imputation deterministically shrinks the variance of imputed rows (currently pinned as intended behavior, worth revisiting).
- A dead negativity check with a copy-pasted message, and a NaN-column deduplication step that can miss genuinely duplicate columns.

## EKS / Accelerators / Learning-rate schedulers
Momentum, timestep, and noise-covariance code needs to guard degenerate inputs and stay consistent with its underlying continuous-time derivation under nonuniform steps.
- The data-misfit-controller scheduler divides by unguarded statistics, which can push the timestep to infinity and propagate NaNs.
- Nesterov-type acceleration is applied to the Langevin sampler, biasing the invariant distribution it's meant to preserve, and its momentum recursion assumes uniform step sizes that don't hold under the default scheduler.
- The scheduler's docstring formula is off by a factor of two and mislabels a variable.
- The sampler ignores update-group indices (fails loudly rather than silently); assorted dead code (an unused covariance computation, duplicate branches) and a missing momentum-coefficient validation. ## Localizers / update groups Localization kernels need positive-semidefinite-preserving, zero-variance-safe, correctly documented formulas, and update-group indices need validation.
- One localizer's correlation-standard-deviation formula was wrong for nonzero correlations, and the whole family divided by sample standard deviation with no zero-variance guard.
- A documented cutoff correlation doesn't match the localizer's actual effective cutoff, and two localizers' elementwise scalings aren't guaranteed positive-semidefinite.
- A dropout-based localizer draws from the global RNG rather than the seeded one and doesn't rescale for the dropout probability, causing undocumented shrinkage.
- One localizer throws or silently degenerates at very small ensemble sizes.
- Update groups never check for duplicate indices, which can make the observation covariance singular.
- A dead field, an undocumented assumption that parameters and outputs share a grid ordering, a misstated docstring, and dead tolerance-checking logic.

## Observations / minibatching
Observation construction, inverse-covariance handling, and minibatch/epoch bookkeeping need to promote and validate inputs consistently, and preserve user-specified sampling behavior.
- A user-supplied minibatch epoch pool is discarded after the first epoch, silently reverting to uniform sampling.
- Scalar inverse-covariances aren't promoted the way scalar covariances are, causing a crash on first use.
- One inverse-covariance code path is unrunnable, and mathematically wrong even if the immediate error were fixed.
- A minibatcher can produce an empty epoch (leading to a downstream division by zero) when the requested minibatch size exceeds the epoch size.
- Pseudoinverse results are documented as exact inverses.
- A misnamed fallback method, an invalid error construction, and a copy-pasted docstring.

## ParameterDistributions / FunctionParameterDistributions
Constraint transforms, moment formulas, and Dict/vector sample interfaces need bounds/size validation and correctly oriented Jacobians.
- Dict-interface transforms can return uninitialized memory when sample rows exceed the distribution's dimensions.
- One constraint's Jacobian sign and its moment-integration bounds are reversed, giving negated moments.
- Bounded transforms saturate for large inputs, so a finite round trip can return infinity.
- A Gaussian-constraint constructor never validates that its constrained standard deviation is positive.
- Single-sample distributions return NaN for their covariance and variance.
- Mean/variance shapes are inconsistent across different distribution compositions.
- Several comments contradict the actual (correct) sample and constraint conventions used elsewhere in the code.

## full audit:
[review.md](https://github.com/user-attachments/files/31442805/review.md)

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with review.md and identify one independently scoped finding; the issue lists affected areas but does not name source files or tests. Trace the selected behavior in the corresponding Julia implementation, add or update focused coverage, and verify that the specific audit finding is resolved without taking on the broader list.

Written by the indexing model from the issue text.

Assessment

Tech stack
julia
Domain
data, machine-learning
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.