Add Bayesian posterior sampling to Analysis1d - #237
Open
henrikjacobsenfys wants to merge 3 commits into
Open
Conversation
Expose the EasyScience Fitter on Analysis1d and add MCMC posterior sampling on top of it, using the BUMPS DREAM sampler introduced in easyscience 2.5.1 (easyscience.fitting.Sampler). Least-squares fitting reports a single point with a curvature-derived uncertainty, which is only trustworthy when parameters are uncorrelated and roughly Gaussian. Sampling maps the whole posterior instead, so correlated and skewed parameters get honest credible intervals. The sampling machinery lives in a mixin with three hooks (build the fitter, bind the data, list the chain parameters) so that Analysis and ParameterAnalysis can reuse it. ParameterAnalysis is not an AnalysisBase and builds a MultiFitter over binding models rather than over itself, so a shared base class would not have worked. Notable details: - fit() now uses a cached Fitter instead of building one per call, and the cache is invalidated through the existing dirty-flag pattern. - Bounds are the prior in DREAM, so sampling refuses to run with any infinite bound. suggest_bounds() proposes finite ones from the fitted values and uncertainties; it is advisory until .apply() is called and never loosens a bound that is already finite, so physical limits survive. A zero-width suggestion is flagged rather than invented. - Sampling restores parameter values afterwards, since BUMPS leaves them wherever the last likelihood evaluation put them. - Chains are reported under Parameter.name, not the internal unique_name. Those names are per-session, so save_chain() writes a sidecar mapping them to stable names and load_chain() uses it; loading without one warns rather than mislabelling the columns. - After sampling, a warning fires when the posterior has piled up against a bound, which catches both bounds that are too tight and degenerate parameters that drift until a bound stops them. - BUMPS crashes with a bare IndexError inside its own outlier removal when chains scatter, which in practice means a degenerate model. That is re-raised with the likely cause and a workaround. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #237 +/- ##
==========================================
Coverage ? 98.18%
==========================================
Files ? 57
Lines ? 4628
Branches ? 795
==========================================
Hits ? 4544
Misses ? 47
Partials ? 37
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This was referenced Aug 13, 2026
Review feedback: bayesian_sampling.py had a lot in it that belonged elsewhere, and it was unclear why it was a mixin at all. It was a mixin because ParameterAnalysis is not an AnalysisBase and fits its binding models rather than itself, so a shared base class does not work. That was a reason, not a good one: it injected some forty methods into every Analysis class. The sampler is now composed. An Analysis exposes one `bayesian` property, and hands the sampler the few things that differ between the Analysis classes -- the data, the free parameters, their labels, and a hook to refresh cached computation -- so PosteriorSampler needs no knowledge of how any Analysis is built, and no Analysis inherits sampling machinery it does not use. Labelling moves to posterior_labels.py. Building it once for a fixed set of parameters also removes the quadratic cost the old code needed a scoped cache to avoid: the counts and lookups are computed in the constructor rather than per column. Plotting stays in posterior_plotting.py, where it already lived. The sampler keeps three short delegates so a chain can still be plotted from the object holding it, but none of the drawing happens there. The public API becomes analysis.bayesian.sample() and friends, and the explicit suggest_bounds().apply() step stays: in DREAM the bounds are the prior, and an unbounded parameter gives a confident-looking interval set by nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The notebook tests run with '-n auto', and five of the notebooks fetch vanadium_data_example.h5 through pooch. On a cold cache the workers race: one is still writing the file into the cache while another opens it, which fails on Windows with "PermissionError: Permission denied". This failed twice in a row on windows-latest, always on that file, always with the other sixteen notebooks passing. The race is pre-existing, but adding a fifth notebook that wants the same file, and lengthening tutorial 1, made it reliable rather than rare. Fetching every tutorial data file once, before the parallel run starts, leaves the workers with nothing to do but read, which is safe. The prefetch reads the URLs and hashes out of the notebooks themselves, so it cannot drift from what they actually download, and it never fails the run: a file it cannot fetch is left to the notebook that needs it, which reports the problem with far more context. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 46d745a)
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.
Adds Bayesian MCMC posterior sampling to
Analysis1d, built on the BUMPS DREAM sampler that arrived in easyscience 2.5.1 (easyscience.fitting.Sampler).Least-squares fitting reports one point with a curvature-derived uncertainty, which is only trustworthy when parameters are uncorrelated and roughly Gaussian — often not the case in QENS. Sampling maps the whole posterior instead, so correlated and skewed parameters get honest credible intervals, and the correlations themselves become visible.
This is the first of two PRs.
AnalysisandParameterAnalysisfollow in a second one.API
Everything to do with sampling hangs off one
bayesianproperty:Structure
analysis.bayesianis aPosteriorSamplerthe analysis holds, not a mixin it inherits. The sampler knows nothing about analyses: it is handed three callables — the chain parameters, the(x, y, weights)to fit against, and a labeller — plus an optional hook to bring cached state up to date before a run. That is the whole contract, and it is what lets PR 2 reuse the same class forParameterAnalysis, which is not anAnalysisBaseat all.The pieces that are not sampling live on their own:
posterior_labels.py—ParameterLabelsturns parameters into the names and units everything reports under, built once per call rather than once per parameter.posterior.py— bounds suggestions, the summary table, pile-up detection.utils/posterior_plotting.py— the figures, as plain functions over arrays.fit()now uses a cachedFitterinstead of constructing one per call, invalidated through the existing dirty-flag pattern. Behaviour is unchanged — the full pre-existing suite passes untouched.Design notes
Bounds are the prior. In DREAM the bounds are the uniform prior, so sampling refuses to run while any free parameter is unbounded.
suggest_bounds()proposes bounds from the fitted values and uncertainties, and is advisory until.apply()is called. It only ever fills an infinite side, so physical limits (a non-negative area) survive; and because too narrow a bound truncates the posterior and understates uncertainty, the defaults are deliberately generous (10σ plus 20% of the value). The relative pad covers minimizers that report zero uncertainty; when there is genuinely no scale information the parameter is flagged rather than given an invented one.Sampling restores your parameters. BUMPS leaves them wherever the last likelihood evaluation put them, which would silently move a fitted model off its fit.
Chains use real parameter names. The sampler labels columns with
unique_name(Parameter_4). Those are per-session, so a saved chain reloaded elsewhere was unreadable;save()now writes a sidecar mapping them to stableParameter.names, and loading without one warns rather than mislabelling columns.Pile-up detection. After sampling, a warning fires when the posterior has piled up against a bound — catching both over-tight bounds and degenerate parameters that drift until a bound stops them. Threshold calibrated against a measured clipped chain (0.135 occupancy) versus healthy ones (0.000).
Degenerate models. BUMPS crashes with a bare
IndexErrorinside its own outlier removal when chains scatter, which in practice means the model is not identifiable. That is re-raised naming the likely cause and thesampler_kwargs={'outliers': 'none'}workaround.Testing
bayesian.ipynbtutorial executes end to end undernbmake.pixi run fixandpixi run checkboth clean.Notes for reviewers
Two pre-existing issues surfaced while building this, both left untouched as out of scope:
normalize_resolution()sets the resolution area to 1 but leaves it free, so the nextfit()moves it and the normalization silently does not hold.component.area.fixed = Trueon an object you constructed has no effect on the analysis — you have to go throughanalysis.get_free_parameters(). This is easy to get wrong.Worth a separate issue: fitting data with no uncertainties. Weights fall back to 1, which is harmless for least-squares but not for a posterior, whose width scales directly with the assumed noise. Treating σ as a free nuisance parameter would fix it, but needs a likelihood carrying the
−N log σterm that BUMPS'Curvedoes not have. Documented as a caveat in the tutorial for now.🤖 Generated with Claude Code