Add Bayesian posterior sampling to Analysis and ParameterAnalysis - #238
Merged
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>
Extends the sampling introduced for Analysis1d to the remaining two Analysis classes, using the mixin hooks added with it. No new sampling machinery: each class supplies its fitter, its data, and its chain parameters, and everything else is shared. Analysis gains sample_posterior(fit_method=...), mirroring fit(): - 'independent' gives each Q index its own chain, delegating to the Analysis1d objects, and returns one result per Q (or a single result when a Q_index is given). - 'simultaneous' runs one chain over every Q at once through a MultiFitter, refreshing each per-Q convolver against its masked energy grid first, exactly as the simultaneous fit does. ParameterAnalysis samples the binding models. Its fit() built the MultiFitter inline, so the per-target data, functions, and models are now resolved by a shared _build_fit_inputs() that both paths use, which also guarantees fitting and sampling see the same targets in the same order with the same unit conversions. Parameter labels needed rethinking. A multi-Q analysis holds one copy of each parameter per Q, all sharing a name, so a summary showed several identical rows and a name could not pick a parameter out. Labels are now produced by an overridable parameter_label(): Analysis qualifies by Q index, ParameterAnalysis by binding model, and both only when the bare name is actually ambiguous, so single-Q and single-binding cases keep their short names. The summary and bounds tables size themselves to the longest label rather than truncating. Also fixes Analysis.fit's docstring, which promised a single FitResults for a simultaneous fit. MultiFitter splits its combined result back up by dataset, so a list has always been returned. Tutorial 1 gains a Bayesian section on the two-step diffusion fit, where the posterior turns out to be about twelve times tighter than the reported least-squares uncertainties. That gap is real and worth explaining: the width fit has a reduced chi-squared near 150, so lmfit inflates its uncertainties by the square root of that, while the sampler takes the stated uncertainties at face value. Sampling the full simultaneous diffusion model was measured at over ten minutes, so the tutorial uses the ParameterAnalysis step instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
henrikjacobsenfys
added
[scope] enhancement
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #238 +/- ##
===========================================
+ Coverage 98.13% 98.69% +0.55%
===========================================
Files 57 57
Lines 4816 5193 +377
Branches 830 903 +73
===========================================
+ Hits 4726 5125 +399
+ Misses 48 31 -17
+ Partials 42 37 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
The summary table already reported each parameter's unit, but the plots did not, so a diffusion coefficient came out as a bare number. Units are now threaded through to plot_trace and plot_corner, and the posterior predictive plot gets axis labels taken from the analysis' own energy and intensity units. Details that needed care: - Matplotlib parks a shared exponent at the end of the axis, on top of the axis label. It is now folded into the label, sharing one set of parentheses with the unit, so a diffusion coefficient reads "diffusion_coefficient (1e-8 m^2/s)" rather than stacking two parentheticals or overlapping. - Dimensionless and empty units are skipped. A polynomial coefficient labelled "dimensionless" is noise. - The top-left panel of a corner plot is a histogram, so its vertical axis counts draws rather than carrying a parameter. It is now labelled "counts" instead of being left blank, which read as an omission. - Corner tick counts are capped, since four labelled ticks per panel is as much as a small panel can carry legibly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two fixes found by writing the tests codecov asked for. ParameterAnalysis qualified an ambiguous parameter with the owning model's display_name, but for several models -- the diffusion models among them -- display_name is the class name, so two models constructed as name='Diffusion A' and name='Diffusion B' both came back as "BrownianTranslationalDiffusion" and the label did not disambiguate anything. It now uses the model's name, matching the choice to report parameters under their name rather than their display name, and falls back to the unique name only when the names collide too. The rest is test coverage for branches that were reachable but untested: the label fallbacks, the BUMPS outlier crash being re-raised as a degeneracy hint, a chain column that matches no parameter, loading a chain through its sidecar, the mixin's unimplemented hooks, and the scientific-notation exponent being folded into an axis label. 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>
…tegration tests Two problems found while reviewing the previous commits. Caching the MultiFitter on ParameterAnalysis introduced a regression. A FitBinding can be edited in place -- binding.targets = ... -- which ParameterAnalysis cannot observe. Changing the number of targets left the cached fitter holding one fit function against two datasets, and fit() died with "FitError: list index out of range". It rebuilt every call before, so this worked previously. The targets the fitter was built for are now recorded and compared, which is enough to catch an edit that cannot be observed directly. The integration tests then failed in CI on macOS, inside BUMPS' outlier removal, on an identifiable model. That matters beyond the test: the error message claimed the crash means degenerate parameters, and this shows short chains do it too. The message now names both causes, and the integration tests switch the outlier removal off, as they already do for the burn-point trimming. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six issues found reviewing the previous commits. The sidecar could be written with the wrong labels. A subset run built the name map inside the block that holds the other parameters fixed, where nothing looks ambiguous, so a multi-Q chain recorded unqualified names that no longer matched on reload. The map is now built outside that block, where the free set is the user's real one. extend_sampling() accepted a different parameter subset. BUMPS resumes from a stored chain whose width is fixed, so that could only fail deep inside the sampler; it is now refused up front. The IndexError relabelling was unconditional, so an IndexError from this package would have been reported as a BUMPS modelling problem. It now only applies when the traceback passes through bumps. Labelling a chain was quadratic in the parameter count: collecting the parameters and scanning for their owner both happened per parameter, and each walks every sub-model. 75 parameters took 0.39 s, and every summary and plot pays it. The parameters are now collected once per pass, and Analysis keeps an owner index alongside its analysis list. The same case now measures at 0.00 s. Asking an Analysis for a summary after sampling independently reported that nothing had been sampled, moments after it had. It now says where the chains actually are. Applying bounds many orders of magnitude wider than the parameter is still allowed -- it is what the fit implied -- but no longer silent, so a scripted apply() cannot hide a degeneracy the table would have shown. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three lines the review fixes added were not reachable from the unit tests. Two are now covered: extending after a run that died before storing results, where the chain-shape guard has nothing to compare against, and a parameter shared across every Q index, which is left out of the owner map because no single Q identifies it. The third was the non-finite check in the absurd-width test, and it was redundant rather than untested: an infinite width already compares greater than any threshold, and the zero-scale case returns before it. Removed, so the behaviour is unchanged and there is no dead branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sampling with fit_method='independent' left the results only on the Analysis1d objects, so the Analysis that produced them could not report on them. It now gathers them, but only where gathering is sound. posterior_summary() collects every Q into one table, labelled by Q index, and set_parameters_to_posterior_median() applies each chain to its own Q. Both are per-parameter marginal operations, and a marginal is well defined within its own chain, so combining them across separate chains says nothing that was not sampled. plot_corner() deliberately does not aggregate. Independent sampling draws each Q separately, so no draw pairs a parameter at one Q with a parameter at another, and a corner plot built from them would show correlations that are an artefact of how the sampling was run rather than anything measured. It says so and points at the per-Q corner plots, which are real. plot_trace() likewise, the chains being separate runs of different lengths rather than one trace. posterior_results exposes the per-Q chains directly, and a simultaneous chain still takes precedence over stale per-Q ones. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Independent chains share no draws, so there is no joint distribution across Q to plot, and combining them would show correlations that came from how the sampling was run rather than from the data. Refusing outright was correct but unhelpful: the correlations within each Q are real and worth looking at. Analysis.plot_corner() now shows one Q at a time. Pass Q_index for a particular one, or leave it out in a notebook for a slider across the Q values that were sampled. A simultaneous chain is unaffected; it already covers every Q in one figure. Outside a notebook the error names the sampled Q indices rather than only saying no. The slider is built with append_display_data rather than the Output widget's context manager. The context manager is the obvious choice and captures nothing under some kernels, which would have shipped a slider with a permanently blank panel beside it. Verified by executing a notebook against a real kernel, and the test asserts the panel actually holds a figure, since an empty panel is the regression that matters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The slider was described in the tutorial's caveats but never demonstrated: every notebook call to plot_corner() went through the single-chain path, because the Bayesian tutorial used Analysis1d and tutorial 1 used ParameterAnalysis, neither of which has a Q dimension. So the only things exercising it were the unit tests. The tutorial now builds the full multi-Q Analysis, samples a few Q values, gathers them with posterior_summary(), and shows the slider. It samples Q indices 4, 8 and 12 rather than all sixteen. Sampling every Q measured at 70 s against 16 s for three, and the subset also shows two things worth showing: that sampling is slow enough to be worth trying a few Q values first, and that the slider offers only the Q values that were actually sampled. Verified against a real kernel that the cell emits a widget view, rather than only that the notebook ran without raising. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Matches where plopp puts its slicer controls, which is also where the existing slicerplot_with_residuals puts them via the figure's bottom bar. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
Replaces the BayesianSamplingMixin with a PosteriorSampler that each analysis holds, so sampling, labelling and plotting stop sharing one class. Analysis gets a MultiQPosteriorSampler on top, which keeps the per-Q chains and the Q slider. Labels move to ParameterLabels, built once per call rather than once per parameter, and each analysis supplies the qualifier it needs: Q index for Analysis, binding model name for ParameterAnalysis. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The section headers still pointed at a class that no longer exists, and MultiQPosteriorSampler was reachable only through Analysis.bayesian. 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)
The sampling tests labelled the action WHEN and had no THEN, so a reader could not see where the arrangement stopped and the call under test began. Setup is WHEN, the action is THEN, the assertions are EXPECT, and steps that genuinely collapse onto one statement carry one combined marker instead. Comments only; no test changed what it does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Same pass as on the single-Q tests: setup is WHEN, the action is THEN, the assertions are EXPECT, and a step that collapses onto one statement carries one combined marker. Comments only; no test changed what it does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Tests were split by feature rather than by the file they exercise, so posterior_sampling.py had no test file of its own and Analysis1d had two. The sampler's tests now live in test_posterior_sampling.py under one TestPosteriorSampler, with the old class names as section banners, and the four tests that are really about Analysis1d's cached fitter move into TestAnalysis1d. No test changed what it does; the same 31 + 4 tests run as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Analysis and ParameterAnalysis each had a second test file, and the sampler had none of its own. The sampler's tests, whichever analysis drives them, now live in test_posterior_sampling.py under TestPosteriorSampler and TestMultiQPosteriorSampler; the fitter, chain parameter and label tests move into TestAnalysis and TestParameterAnalysis. Old class names became section banners. The multi-Q and ParameterAnalysis helpers keep distinct names in the merged file, since their signatures differ from the single-Q ones. The same 1660 tests run as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- extend() now verifies the chain holds the same parameters, not just the same number, and refuses to resume after a failed run or after the model or data changed - Parameter objects passed to sample(parameters=...) are validated against the free set the same way strings are - sampling with no free parameters and degenerate (min >= max) bounds raise clear errors before reaching BUMPS - parameters_at_bounds keys by unique_name so same-named per-Q parameters no longer collide, and guards empty draws - suggest_bounds flags non-finite fitted uncertainties for attention - save() refuses to write an empty label sidecar; loading one warns like a missing sidecar - colliding display labels get positional suffixes in the sidecar so save/load resolves each column to its own parameter - plot_posterior_predictive omits error bars when the data carries no variances (new Experiment.has_variances) - posterior plots validate draws/logp up front, name NaN columns, and share x-limits per corner column - document that sampling runs are not seedable Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # src/easydynamics/analysis/posterior_sampling.py # tests/unit/easydynamics/analysis/test_posterior_sampling.py # tests/unit/easydynamics/utils/test_posterior_plotting.py
- sampling one Q index independently now clears a stale simultaneous chain, so summary(), set_parameters_to_median() and plot_corner() report the run the user just made instead of the old one - extend() and save() after an independent run explain that the chains live on the per-Q analyses instead of resuming or saving the stale simultaneous chain; a genuinely failed run keeps its own message - Q_index arguments are validated like every Analysis method, so a negative index raises instead of silently wrapping - the gathered summary resolves each per-Q chain through its own saved labels, so chains loaded from disk keep names and units - warnings are attributed to the caller on both the single-Q and multi-Q paths, and the corner-plot slider forwards plot kwargs - the multi-Q integration tests share one independent sampling run, assert the straight line is actually recovered, and the extend test no longer mutates the shared fixture Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- plot_marginal(parameter) renders one parameter's posterior histogram with the median and the 16/84 percentile interval summary() reports, resolving labels the same way sample(parameters=...) does - plot_correlations() renders the Pearson correlation matrix of the chain with annotated cells, a diverging colormap and masked cells for constant columns - sample(progress=True) and extend(progress=True) report sampling progress through the Sampler's progress_callback, closing the line with an explicit done marker because BUMPS' own step estimate assumes the wrong chain count - the 95 percent predictive band needed no change: credible_interval already exists on plot_posterior_predictive Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
After independent per-Q sampling the multi-Q sampler now presents a Q slider instead of refusing: - plot_posterior_predictive builds the per-Q data, median and credible band into a scipp DataGroup and renders it through plopp exactly like plot_data_and_model; plopp cannot shade a band on sliced lines, so the slider view draws labelled band edges while the Q_index path keeps the shaded band - plot_trace, plot_marginal and plot_correlations take Q_index for a single figure, show a slider in a notebook, and otherwise name the sampled Q indices - the matplotlib sliders render every figure once up front and only swap PNG bytes on a move, so dragging tracks smoothly with continuous updates instead of re-rendering per change - per-Q energy grids are NaN-padded onto the common grid through the finite mask, so masked points draw as gaps Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The tutorial now demonstrates plot_marginal and plot_correlations from the sampled chain, progress=True on the sampling call, the 95 percent predictive band option, the Q slider that every posterior plot offers over independent chains, and notes that runs are not seedable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The progress reporter closes through try/finally instead of a bare re-raise, and the plotting validation errors are documented in the form the docstring linter expects. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Q-slider cells go through the plopp slicer, which refuses the inline backend; every plopp-using tutorial already runs %matplotlib widget. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
henrikjacobsenfys
added a commit
that referenced
this pull request
Aug 17, 2026
* Reach the whole library through one namespace
Review feedback: the import style was inconsistent enough that a reader
had to scroll back to the imports cell to find out where a name came
from. Surveying it, the tutorials used four styles, and the last two
existed only because there was no other way to reach those names:
import easydynamics as edyn 32 uses
import easydynamics.sample_model as sm 151 uses
from easydynamics.convolution import Convolution forced
from easydynamics.utils.utils import hbar forced
easydynamics.__all__ held six names, so Analysis1d, Convolution,
detailed_balance_factor and hbar could only be had by importing the
module that defines them. The inconsistency was structural rather than
careless, and no amount of tidying the notebooks alone would have fixed
it.
Everything public is now re-exported from easydynamics, 37 names, so
`import easydynamics as edyn` reaches all of it. The sub-packages stay
importable and the internal layout is untouched: only the front door is
flat. Flat is comfortable at this size, there were no name collisions,
and the sample_model grouping was already imprecise, holding
InstrumentModel, ResolutionModel and BackgroundModel.
The tutorials and the docstring examples that render into the API
reference now use that one style throughout. A test keeps the front door
in step with the sub-packages and the notebooks in step with the
convention, which is also written down in CONTRIBUTING.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Point the front door at the composed sampler
The flat namespace still re-exported the mixin that the refactor
removed, and not the sampler classes that replaced it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Unwrap the security-issue line again
Prettier 3.9, which CI installs, measures the shield emoji differently
from the older release cached here and wants the line whole.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Mark setup, action and expectation apart in the namespace tests
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Add Bayesian posterior sampling to Analysis and ParameterAnalysis (#238)
* Add Bayesian posterior sampling to Analysis1d
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>
* Add Bayesian posterior sampling to Analysis and ParameterAnalysis
Extends the sampling introduced for Analysis1d to the remaining two
Analysis classes, using the mixin hooks added with it. No new sampling
machinery: each class supplies its fitter, its data, and its chain
parameters, and everything else is shared.
Analysis gains sample_posterior(fit_method=...), mirroring fit():
- 'independent' gives each Q index its own chain, delegating to the
Analysis1d objects, and returns one result per Q (or a single result
when a Q_index is given).
- 'simultaneous' runs one chain over every Q at once through a
MultiFitter, refreshing each per-Q convolver against its masked energy
grid first, exactly as the simultaneous fit does.
ParameterAnalysis samples the binding models. Its fit() built the
MultiFitter inline, so the per-target data, functions, and models are
now resolved by a shared _build_fit_inputs() that both paths use, which
also guarantees fitting and sampling see the same targets in the same
order with the same unit conversions.
Parameter labels needed rethinking. A multi-Q analysis holds one copy of
each parameter per Q, all sharing a name, so a summary showed several
identical rows and a name could not pick a parameter out. Labels are now
produced by an overridable parameter_label(): Analysis qualifies by Q
index, ParameterAnalysis by binding model, and both only when the bare
name is actually ambiguous, so single-Q and single-binding cases keep
their short names. The summary and bounds tables size themselves to the
longest label rather than truncating.
Also fixes Analysis.fit's docstring, which promised a single FitResults
for a simultaneous fit. MultiFitter splits its combined result back up
by dataset, so a list has always been returned.
Tutorial 1 gains a Bayesian section on the two-step diffusion fit, where
the posterior turns out to be about twelve times tighter than the
reported least-squares uncertainties. That gap is real and worth
explaining: the width fit has a reduced chi-squared near 150, so lmfit
inflates its uncertainties by the square root of that, while the sampler
takes the stated uncertainties at face value. Sampling the full
simultaneous diffusion model was measured at over ten minutes, so the
tutorial uses the ParameterAnalysis step instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Label the posterior plot axes with units and quantities
The summary table already reported each parameter's unit, but the plots
did not, so a diffusion coefficient came out as a bare number. Units are
now threaded through to plot_trace and plot_corner, and the posterior
predictive plot gets axis labels taken from the analysis' own energy and
intensity units.
Details that needed care:
- Matplotlib parks a shared exponent at the end of the axis, on top of
the axis label. It is now folded into the label, sharing one set of
parentheses with the unit, so a diffusion coefficient reads
"diffusion_coefficient (1e-8 m^2/s)" rather than stacking two
parentheticals or overlapping.
- Dimensionless and empty units are skipped. A polynomial coefficient
labelled "dimensionless" is noise.
- The top-left panel of a corner plot is a histogram, so its vertical
axis counts draws rather than carrying a parameter. It is now labelled
"counts" instead of being left blank, which read as an omission.
- Corner tick counts are capped, since four labelled ticks per panel is
as much as a small panel can carry legibly.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Qualify parameter labels by model name, and cover the remaining branches
Two fixes found by writing the tests codecov asked for.
ParameterAnalysis qualified an ambiguous parameter with the owning
model's display_name, but for several models -- the diffusion models
among them -- display_name is the class name, so two models constructed
as name='Diffusion A' and name='Diffusion B' both came back as
"BrownianTranslationalDiffusion" and the label did not disambiguate
anything. It now uses the model's name, matching the choice to report
parameters under their name rather than their display name, and falls
back to the unique name only when the names collide too.
The rest is test coverage for branches that were reachable but untested:
the label fallbacks, the BUMPS outlier crash being re-raised as a
degeneracy hint, a chain column that matches no parameter, loading a
chain through its sidecar, the mixin's unimplemented hooks, and the
scientific-notation exponent being folded into an axis label.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Warm the tutorial data cache before running notebooks in parallel
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>
* Rebuild the fitter when a binding changes shape, and stabilise the integration tests
Two problems found while reviewing the previous commits.
Caching the MultiFitter on ParameterAnalysis introduced a regression. A
FitBinding can be edited in place -- binding.targets = ... -- which
ParameterAnalysis cannot observe. Changing the number of targets left the
cached fitter holding one fit function against two datasets, and fit()
died with "FitError: list index out of range". It rebuilt every call
before, so this worked previously. The targets the fitter was built for
are now recorded and compared, which is enough to catch an edit that
cannot be observed directly.
The integration tests then failed in CI on macOS, inside BUMPS' outlier
removal, on an identifiable model. That matters beyond the test: the
error message claimed the crash means degenerate parameters, and this
shows short chains do it too. The message now names both causes, and the
integration tests switch the outlier removal off, as they already do for
the burn-point trimming.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Address the review findings on the sampling API
Six issues found reviewing the previous commits.
The sidecar could be written with the wrong labels. A subset run built
the name map inside the block that holds the other parameters fixed,
where nothing looks ambiguous, so a multi-Q chain recorded unqualified
names that no longer matched on reload. The map is now built outside that
block, where the free set is the user's real one.
extend_sampling() accepted a different parameter subset. BUMPS resumes
from a stored chain whose width is fixed, so that could only fail deep
inside the sampler; it is now refused up front.
The IndexError relabelling was unconditional, so an IndexError from this
package would have been reported as a BUMPS modelling problem. It now
only applies when the traceback passes through bumps.
Labelling a chain was quadratic in the parameter count: collecting the
parameters and scanning for their owner both happened per parameter, and
each walks every sub-model. 75 parameters took 0.39 s, and every summary
and plot pays it. The parameters are now collected once per pass, and
Analysis keeps an owner index alongside its analysis list. The same case
now measures at 0.00 s.
Asking an Analysis for a summary after sampling independently reported
that nothing had been sampled, moments after it had. It now says where
the chains actually are.
Applying bounds many orders of magnitude wider than the parameter is
still allowed -- it is what the fit implied -- but no longer silent, so a
scripted apply() cannot hide a degeneracy the table would have shown.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Cover the review fixes, and drop a redundant guard
Three lines the review fixes added were not reachable from the unit
tests. Two are now covered: extending after a run that died before
storing results, where the chain-shape guard has nothing to compare
against, and a parameter shared across every Q index, which is left out
of the owner map because no single Q identifies it.
The third was the non-finite check in the absurd-width test, and it was
redundant rather than untested: an infinite width already compares
greater than any threshold, and the zero-scale case returns before it.
Removed, so the behaviour is unchanged and there is no dead branch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Gather the per-Q chains on Analysis after independent sampling
Sampling with fit_method='independent' left the results only on the
Analysis1d objects, so the Analysis that produced them could not report
on them. It now gathers them, but only where gathering is sound.
posterior_summary() collects every Q into one table, labelled by Q index,
and set_parameters_to_posterior_median() applies each chain to its own Q.
Both are per-parameter marginal operations, and a marginal is well
defined within its own chain, so combining them across separate chains
says nothing that was not sampled.
plot_corner() deliberately does not aggregate. Independent sampling draws
each Q separately, so no draw pairs a parameter at one Q with a parameter
at another, and a corner plot built from them would show correlations
that are an artefact of how the sampling was run rather than anything
measured. It says so and points at the per-Q corner plots, which are
real. plot_trace() likewise, the chains being separate runs of different
lengths rather than one trace.
posterior_results exposes the per-Q chains directly, and a simultaneous
chain still takes precedence over stale per-Q ones.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Step through the per-Q corner plots with a slider
Independent chains share no draws, so there is no joint distribution
across Q to plot, and combining them would show correlations that came
from how the sampling was run rather than from the data. Refusing
outright was correct but unhelpful: the correlations within each Q are
real and worth looking at.
Analysis.plot_corner() now shows one Q at a time. Pass Q_index for a
particular one, or leave it out in a notebook for a slider across the Q
values that were sampled. A simultaneous chain is unaffected; it already
covers every Q in one figure. Outside a notebook the error names the
sampled Q indices rather than only saying no.
The slider is built with append_display_data rather than the Output
widget's context manager. The context manager is the obvious choice and
captures nothing under some kernels, which would have shipped a slider
with a permanently blank panel beside it. Verified by executing a
notebook against a real kernel, and the test asserts the panel actually
holds a figure, since an empty panel is the regression that matters.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Show the per-Q corner slider in the Bayesian tutorial
The slider was described in the tutorial's caveats but never
demonstrated: every notebook call to plot_corner() went through the
single-chain path, because the Bayesian tutorial used Analysis1d and
tutorial 1 used ParameterAnalysis, neither of which has a Q dimension.
So the only things exercising it were the unit tests.
The tutorial now builds the full multi-Q Analysis, samples a few Q
values, gathers them with posterior_summary(), and shows the slider.
It samples Q indices 4, 8 and 12 rather than all sixteen. Sampling every
Q measured at 70 s against 16 s for three, and the subset also shows two
things worth showing: that sampling is slow enough to be worth trying a
few Q values first, and that the slider offers only the Q values that
were actually sampled.
Verified against a real kernel that the cell emits a widget view, rather
than only that the notebook ran without raising.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Put the corner slider under the figure
Matches where plopp puts its slicer controls, which is also where the
existing slicerplot_with_residuals puts them via the figure's bottom bar.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Compose the posterior sampler instead of mixing it in
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>
* Export the multi-Q sampler and drop the mixin's name
The section headers still pointed at a class that no longer exists, and
MultiQPosteriorSampler was reachable only through Analysis.bayesian.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Warm the tutorial data cache before running notebooks in parallel
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)
* Mark setup, action and expectation apart in the new tests
The sampling tests labelled the action WHEN and had no THEN, so a reader
could not see where the arrangement stopped and the call under test
began. Setup is WHEN, the action is THEN, the assertions are EXPECT, and
steps that genuinely collapse onto one statement carry one combined
marker instead.
Comments only; no test changed what it does.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Mark setup, action and expectation apart in the multi-Q tests
Same pass as on the single-Q tests: setup is WHEN, the action is THEN,
the assertions are EXPECT, and a step that collapses onto one statement
carries one combined marker.
Comments only; no test changed what it does.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Give the sampler its own test file
Tests were split by feature rather than by the file they exercise, so
posterior_sampling.py had no test file of its own and Analysis1d had
two. The sampler's tests now live in test_posterior_sampling.py under
one TestPosteriorSampler, with the old class names as section banners,
and the four tests that are really about Analysis1d's cached fitter move
into TestAnalysis1d.
No test changed what it does; the same 31 + 4 tests run as before.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Put each test in the file of the class it exercises
Analysis and ParameterAnalysis each had a second test file, and the
sampler had none of its own. The sampler's tests, whichever analysis
drives them, now live in test_posterior_sampling.py under
TestPosteriorSampler and TestMultiQPosteriorSampler; the fitter, chain
parameter and label tests move into TestAnalysis and
TestParameterAnalysis. Old class names became section banners.
The multi-Q and ParameterAnalysis helpers keep distinct names in the
merged file, since their signatures differ from the single-Q ones.
The same 1660 tests run as before.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Refuse silent chain corruption and harden the posterior sampler
- extend() now verifies the chain holds the same parameters, not just
the same number, and refuses to resume after a failed run or after
the model or data changed
- Parameter objects passed to sample(parameters=...) are validated
against the free set the same way strings are
- sampling with no free parameters and degenerate (min >= max) bounds
raise clear errors before reaching BUMPS
- parameters_at_bounds keys by unique_name so same-named per-Q
parameters no longer collide, and guards empty draws
- suggest_bounds flags non-finite fitted uncertainties for attention
- save() refuses to write an empty label sidecar; loading one warns
like a missing sidecar
- colliding display labels get positional suffixes in the sidecar so
save/load resolves each column to its own parameter
- plot_posterior_predictive omits error bars when the data carries no
variances (new Experiment.has_variances)
- posterior plots validate draws/logp up front, name NaN columns, and
share x-limits per corner column
- document that sampling runs are not seedable
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Keep the multi-Q sampler pointed at the chain the user actually ran
- sampling one Q index independently now clears a stale simultaneous
chain, so summary(), set_parameters_to_median() and plot_corner()
report the run the user just made instead of the old one
- extend() and save() after an independent run explain that the chains
live on the per-Q analyses instead of resuming or saving the stale
simultaneous chain; a genuinely failed run keeps its own message
- Q_index arguments are validated like every Analysis method, so a
negative index raises instead of silently wrapping
- the gathered summary resolves each per-Q chain through its own saved
labels, so chains loaded from disk keep names and units
- warnings are attributed to the caller on both the single-Q and
multi-Q paths, and the corner-plot slider forwards plot kwargs
- the multi-Q integration tests share one independent sampling run,
assert the straight line is actually recovered, and the extend test
no longer mutates the shared fixture
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Add marginal posteriors, correlation heatmaps and sampling progress
- plot_marginal(parameter) renders one parameter's posterior histogram
with the median and the 16/84 percentile interval summary() reports,
resolving labels the same way sample(parameters=...) does
- plot_correlations() renders the Pearson correlation matrix of the
chain with annotated cells, a diverging colormap and masked cells for
constant columns
- sample(progress=True) and extend(progress=True) report sampling
progress through the Sampler's progress_callback, closing the line
with an explicit done marker because BUMPS' own step estimate assumes
the wrong chain count
- the 95 percent predictive band needed no change: credible_interval
already exists on plot_posterior_predictive
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Give every posterior plot a Q slider over independent chains
After independent per-Q sampling the multi-Q sampler now presents a Q
slider instead of refusing:
- plot_posterior_predictive builds the per-Q data, median and credible
band into a scipp DataGroup and renders it through plopp exactly like
plot_data_and_model; plopp cannot shade a band on sliced lines, so
the slider view draws labelled band edges while the Q_index path
keeps the shaded band
- plot_trace, plot_marginal and plot_correlations take Q_index for a
single figure, show a slider in a notebook, and otherwise name the
sampled Q indices
- the matplotlib sliders render every figure once up front and only
swap PNG bytes on a move, so dragging tracks smoothly with
continuous updates instead of re-rendering per change
- per-Q energy grids are NaN-padded onto the common grid through the
finite mask, so masked points draw as gaps
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Write the progress line through sys.stdout
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Show the new posterior plots in the Bayesian tutorial
The tutorial now demonstrates plot_marginal and plot_correlations from
the sampled chain, progress=True on the sampling call, the 95 percent
predictive band option, the Q slider that every posterior plot offers
over independent chains, and notes that runs are not seedable.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Apply the formatting fixes
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Satisfy the docstring and formatting checks
The progress reporter closes through try/finally instead of a bare
re-raise, and the plotting validation errors are documented in the
form the docstring linter expects.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Document propagated exceptions the way the docstring linter expects
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Give the Bayesian tutorial the widget backend its sliders need
The Q-slider cells go through the plopp slicer, which refuses the
inline backend; every plopp-using tutorial already runs %matplotlib
widget.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Extends the Bayesian sampling from #237 to the remaining two Analysis classes.
Stacked on #237 — please merge that first; this PR targets the
bayesianbranch, and I will retarget it atdeveloponce #237 lands.No new sampling machinery. Each class hands the same
PosteriorSamplerits three callables — the chain parameters, the data to fit against, and a labeller — and everything else is shared, which was the point of building #237 around composition.Analysis
analysis.bayesianis aMultiQPosteriorSampler, which adds the per-Q handling on top of the shared sampler.sample(fit_method=...)mirrorsfit():The simultaneous path refreshes each per-Q convolver against its masked energy grid before sampling, exactly as
_fit_all_Q_simultaneouslydoes, so the sampler sees the model evaluations the fit would have made. This was the open question at the end of #237, and it works: 9 parameters across 3 Q values sample in ~14s.After independent sampling,
summary()andset_parameters_to_median()gather the per-Q chains into one table, labelled by Q index. Each row is a marginal, and a marginal is well defined inside its own chain, so collecting them claims nothing that was not sampled.Corner plots are the exception: separate chains share no draws, so nothing pairs a parameter at one Q with a parameter at another, and a combined figure would show correlations that came from how the sampling was run.
plot_corner()therefore steps through them —plot_corner(Q_index=8)for one, or a slider over the sampled Q values in a notebook.ParameterAnalysis
Samples the binding models.
fit()built itsMultiFitterinline, so the per-target data, functions, and models are now resolved by a shared_build_fit_inputs()used by both paths — which also guarantees fitting and sampling see the same targets, in the same order, with the same unit conversions.Parameter labels
This needed rethinking for multi-Q. A multi-Q analysis holds one copy of each parameter per Q, all sharing a name — three parameters called
Gaussian width. A summary showed three identical rows, and a name could not be used to select a parameter.Each class now supplies a qualifier to
ParameterLabels:Analysisuses the Q index,ParameterAnalysisthe binding model name, and both only when the bare name is actually ambiguous, so single-Q and single-binding cases keep their short names. The summary and bounds tables size themselves to the longest label instead of truncating at a fixed width.Drive-by fix
Analysis.fit's docstring promised "a single FitResults object if fitting simultaneously".MultiFittersplits its combined result back up per dataset, so a list has always been returned. Docstring corrected to match the behaviour.Tutorial
Tutorial 1 gains a Bayesian section on the two-step diffusion fit, and it surfaces something worth documenting: the posterior comes out about twelve times tighter than the reported least-squares uncertainties.
That gap is real, not a bug. The width fit has a reduced χ² of ~150, so
lmfitinflates its reported uncertainties by √(reduced χ²) ≈ 12 on the assumption that a poor fit means understated input uncertainties. DREAM makes no such adjustment. Neither is simply right, and the gap is itself a signal that the two-step model is not capturing the data — which is exactly what the tutorial goes on to address by fitting everything simultaneously.I originally intended to sample the full simultaneous diffusion model, but measured it at over ten minutes — too slow for a tutorial and for CI. The
ParameterAnalysisstep gives the same lesson in ~25s.bayesian.ipynbgains a multi-Q section that samples three Q values and drives the corner slider (16s, against ~70s for all sixteen).Testing
suggest_boundsrefuses to invent a scale: a polynomial coefficient sitting at exactly zero with vanishing uncertainty is flagged rather than bounded, andabsolute_flooris what resolves it.pixi run fixandpixi run checkclean.🤖 Generated with Claude Code