Skip to content

Commit 67c842e

Browse files
Split minimizer settings from fit-result outputs (#181)
* Propose minimizer input/output split ADR * Address review 1 findings on minimizer input/output split * Address review 2: demote credible intervals; clarify context table * Address review 3: fix context list for credible-interval fields * Reply to review 4: all findings already addressed in earlier replies * Reply to review 5: both findings already addressed * Reply to review 6: single finding already addressed in reply 3 * Reply to review 7: clean signoff, no findings * Draft minimizer input/output split implementation plan * Address plan review 1: reset paths, defaults, imports, factory * Address plan review 2: migration map, reset hooks, CIF ordering * Address plan review 3: lead P1.11 with no-reordering rule * Rename FitResult to FitResultBase, add reset hooks * Add LeastSquaresFitResult class * Add BayesianFitResult class * Register fit-result family classes with factory * Declare paired _fit_result_class on minimizer bases * Wire fit_result swap and reset paths to paired class * Route LSQ result writers to fit_result * Route Bayesian result writers to fit_result * Remove LSQ output descriptors from minimizer base * Remove Bayesian output descriptors from minimizer base * Serialize fit outputs to _fit_result.* tags * Confirm fit_result paired instance flows through serializer * Add settings-used block to fit.results display * Amend affected ADRs for minimizer input/output split * Update tutorials to read outputs from fit_result * Remove essdiffraction dependency * Promote minimizer-input-output-split ADR * Complete Phase 1 minimizer split review gate * Propose IUCr CIF tag alignment for fit outputs * Add review 4 of input/output split post-Phase 1 * Reply to minimizer input-output review 4 * Complete minimizer input-output Phase 2 * Reply to minimizer input-output review 5 * Reply to minimizer input-output review 6 * Reply to minimizer input-output review 7 * Drop minimizer-input-output-split review/reply files
1 parent 973214a commit 67c842e

45 files changed

Lines changed: 2542 additions & 2010 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

docs/dev/adrs/accepted/analysis-cif-fit-state.md

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ Analysis-owned fit state needs to persist:
2626
- pre-fit scalar snapshots for recovery workflows
2727
- compact status metadata for the latest saved fit projection
2828
- deterministic correlation summaries
29-
- minimizer-specific fit outputs on the active `_minimizer.*` category
29+
- minimizer-specific fit outputs on the paired `_fit_result.*` category
3030
- per-parameter posterior summaries on `_fit_parameter`
3131
- large posterior arrays and plot caches in `analysis/results.h5`
3232

@@ -47,8 +47,7 @@ Persist analysis-owned fit state as explicit analysis categories in
4747

4848
Do not add a dedicated `_fit_state` category or
4949
`_fit_state.schema_version`. Persisted fit state is detected from
50-
`_fit_result`, `_fit_parameter`, `_fit_parameter_correlation`, and
51-
fit-output fields on `_minimizer.*`.
50+
`_fit_result`, `_fit_parameter`, and `_fit_parameter_correlation`.
5251

5352
### Common fit-state categories
5453

@@ -77,7 +76,8 @@ pre-fit scalar snapshots:
7776
- `posterior_gelman_rubin`
7877
- `posterior_effective_sample_size_bulk`
7978

80-
`_fit_result` stores the latest saved fit header:
79+
`_fit_result` stores the latest saved fit header and scalar
80+
family-specific fit outputs:
8181

8282
- `result_kind`
8383
- `success`
@@ -92,9 +92,9 @@ pairs are stored.
9292

9393
### Minimizer fit projection
9494

95-
The active `_minimizer.*` category stores both user-selected solver
96-
inputs and fit-filled outputs. Deterministic minimizer classes store
97-
compact fit output counts:
95+
The active `_minimizer.*` category stores user-selected solver inputs
96+
only. Scalar outputs are written to the paired `_fit_result.*` category.
97+
Deterministic fit-result classes add compact fit output counts:
9898

9999
- `objective_name`
100100
- `objective_value`
@@ -104,17 +104,14 @@ compact fit output counts:
104104
- `degrees_of_freedom`
105105
- `covariance_available`
106106
- `correlation_available`
107-
- `runtime_seconds`
108-
- `iterations_performed`
109107
- `exit_reason`
110108

111109
Do not persist a `_deterministic_parameter_result` category. Final
112110
deterministic parameter values and uncertainties already persist in the
113111
model CIF files, and restored deterministic ordering comes from
114112
`_fit_parameter`.
115113

116-
Bayesian minimizer classes store sampler inputs and fit outputs under
117-
`_minimizer.*`, including:
114+
Bayesian minimizer classes store sampler inputs under `_minimizer.*`:
118115

119116
- `sampling_steps`
120117
- `burn_in_steps`
@@ -123,7 +120,9 @@ Bayesian minimizer classes store sampler inputs and fit outputs under
123120
- `parallel_workers`
124121
- `initialization_method`
125122
- `random_seed`
126-
- `runtime_seconds`
123+
124+
Bayesian fit-result classes store scalar outputs under `_fit_result.*`:
125+
127126
- `point_estimate_name`
128127
- `sampler_completed`
129128
- `credible_interval_inner`
@@ -170,10 +169,10 @@ posterior displays.
170169
Load order is:
171170

172171
1. standard analysis configuration
173-
2. common fit-state categories
174-
3. `_minimizer.*` fit-output fields according to the active
175-
`_minimizer.type`
176-
4. posterior sidecar arrays when a Bayesian result is expected
172+
2. `_minimizer.*` settings according to the active `_minimizer.type`
173+
3. common and family-specific `_fit_result.*` fields on the paired class
174+
4. `_fit_parameter` and `_fit_parameter_correlation`
175+
5. posterior sidecar arrays when a Bayesian result is expected
177176

178177
Persist backend runtime objects, optimizer instances, and raw driver
179178
payloads nowhere in this design.

docs/dev/adrs/accepted/display-ux.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,12 @@ Use these naming rules:
200200
path for `versus`.
201201
- `posterior.*` names are used only when posterior samples are required.
202202

203+
`project.display.fit.results()` also prints a "Settings used" block
204+
above the result tables. The block is sourced from
205+
`analysis.minimizer.*` so the minimizer inputs and paired
206+
`analysis.fit_result.*` outputs are visible from the accepted display
207+
facade without adding a new `Analysis`-level display method.
208+
203209
## Rejected Alternatives
204210

205211
Flat display facade:

docs/dev/adrs/accepted/minimizer-category-consolidation.md

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -55,31 +55,34 @@ samplers.
5555

5656
## Decision
5757

58-
### 1. Unified `minimizer` category replaces all sampler-input and fit-result categories
58+
### 1. Unified `minimizer` category replaces sampler-input categories
5959

6060
Introduce a single switchable category `minimizer` on `Analysis`. Its
6161
concrete class is determined by `Analysis.minimizer_type`. The category
62-
holds both user-writable inputs and fit-filled outputs in one place.
62+
now holds user-writable minimizer inputs only. The later
63+
[`minimizer-input-output-split.md`](minimizer-input-output-split.md) ADR
64+
reverses the fit-output half of this rule: scalar fit outputs live on
65+
the paired `fit_result` category instead of on `minimizer`.
6366

6467
The following categories are removed:
6568

6669
- `bayesian_sampler` — fields move into the Bayesian concrete classes of
6770
`minimizer`.
6871
- `bayesian_result`, `bayesian_convergence` — fields move into the
69-
Bayesian concrete classes of `minimizer` (`runtime_seconds`,
72+
Bayesian concrete classes of `fit_result` (`fitting_time`,
7073
`acceptance_rate_mean`, `gelman_rubin_max`,
7174
`effective_sample_size_min`, `best_log_posterior`, …).
7275
- `deterministic_result` — fields move into the deterministic concrete
73-
classes of `minimizer` (`runtime_seconds`, `iterations_performed`,
74-
`exit_reason`, …).
76+
classes of `fit_result` (`fitting_time`, `iterations`,
77+
`objective_value`, `exit_reason`, …).
7578
- `bayesian_parameter_posterior` — replaced by `Parameter.posterior`
7679
(see §3).
7780
- `bayesian_distribution_cache`, `bayesian_pair_cache`,
7881
`bayesian_predictive_dataset` — replaced by HDF5 sidecar (see §4).
7982

80-
`fit_result` and `fit_parameter` (analysis-owned bounds, success flag,
81-
reduced chi-square, message, fit time, iterations) remain unchanged as
82-
fit-mode-agnostic header categories.
83+
`fit_parameter` (analysis-owned bounds) remains a fit-state category.
84+
`fit_result` remains the common fit header category and is extended by
85+
the input/output split ADR with family-specific scalar outputs.
8386

8487
### 2. Selectors move to the `Analysis` owner
8588

@@ -388,10 +391,11 @@ category's class-level `_engine_metadata` dict.
388391

389392
### Trade-offs
390393

391-
- `minimizer` is the first category that mixes writable user inputs and
392-
writable fit-filled outputs in the same scope. This is a small new
393-
convention but is the natural generalization of how `Parameter`
394-
already holds both user input and refined value on the same object.
394+
- `minimizer` no longer mixes writable user inputs and fit-filled
395+
outputs in the same scope. That stricter boundary is recorded by
396+
[`minimizer-input-output-split.md`](minimizer-input-output-split.md);
397+
`Parameter` remains the refinement-in-place precedent for model values
398+
rather than minimizer diagnostics.
395399
- The set of `_minimizer.*` tags present in CIF depends on the active
396400
`_fitting.minimizer_type`. Loading a CIF whose tags don't match the
397401
minimizer's allowed set raises (clear validation, not silent
@@ -461,9 +465,10 @@ category count for each new sampler and entrenches the convention break.
461465

462466
### D. Strict input-only `minimizer` plus a separate `fit_result`
463467

464-
Keep the categories single-concept (inputs xor outputs) at the cost of
465-
two-place lookup for related info. Rejected in favour of the
466-
one-category-mixes-both shape (§1, §"Trade-offs") because the existing
467-
`Parameter` model already mixes input and refined value on the same
468-
object, and one-place discoverability is more valuable than strict
469-
purity.
468+
Originally rejected in favour of the one-category-mixes-both shape (§1,
469+
§"Trade-offs"). Reversed by
470+
[`minimizer-input-output-split.md`](minimizer-input-output-split.md)
471+
after implementation showed the `Parameter` analogy does not hold for
472+
minimizer settings versus fit diagnostics. The current design keeps
473+
`minimizer` input-only and moves scalar fit outputs to the paired
474+
`fit_result` category.

0 commit comments

Comments
 (0)