Feature/unit system overhaul

[henrikjacobsenfys]

PR Summary: feature/unit-system-overhaul

This PR does two things simultaneously: it lands a fundamental unit system redesign (the
branch's raison d'être), and it carries in a large body of new functionality from the develop
branch via a merge commit. The two are intertwined — the develop features needed to be updated
to speak the new unit API.

---

Part 1 — New functionality from develop (merged in)

These were already reviewed on the develop branch. They arrive here because
feature/unit-system-overhaul branched from an older state of develop, and the merge pulled
everything forward.

Module	What it adds
analysis/	Analysis (2D multi-Q), Analysis1d (single-Q), AnalysisBase (shared fit/plot logic), FitBinding, ParameterAnalysis
experiment/	Experiment — loads HDF5 data, rebins, masks NaNs
base_classes/	EasyDynamicsBase, EasyDynamicsList, EasyDynamicsModelBase, NameMixin
sample_model/diffusion_model/	JumpTranslationalDiffusion, DeltaLorentz
sample_model/components/	ExpressionComponent, Exponential
settings/	ConvolutionSettings, DetailedBalanceSettings

---

Part 2 — Unit system overhaul (core branch work)

The design change

Every model class previously carried a single unit field representing the energy-axis unit.
This is now split:

• x_unit — the unit of the independent variable (energy axis, typically 'meV')
• y_unit — the unit of the output / intensity (typically 'dimensionless', but physical
  when data is calibrated, e.g. '1/meV')

The consequence is that the area parameter of any peak function now has unit
x_unit * y_unit. If y_unit='dimensionless', the area is in energy units as before. If
y_unit='1/meV', the area is dimensionless (the integral of a normalised peak), which is the
physically correct representation.

What changed in every class

• Constructor parameter: unit= → x_unit=, y_unit='dimensionless' (new)
• Properties: unit → x_unit, plus new y_unit
• Conversion methods: convert_unit() → convert_x_unit() + convert_y_unit()
• evaluate(x) → evaluate(x, output='numpy') — passing output='scipp' returns a
  sc.Variable carrying y_unit, enabling unit-safe downstream computation

How unit handling during evaluate changed

Old behaviour (_prepare_x_for_evaluate): if the caller passed a scipp x with a
different compatible unit (e.g. x in eV when the component is in meV), the method called
self.convert_unit(x.unit.name), permanently mutating the component and all its
parameters. A UserWarning was issued. If units were incompatible, a UnitError was raised
— but only after the mutation had already started.

New behaviour: _prepare_x_for_evaluate validates compatibility and returns a 3-tuple
(x_values, detected_unit, dim). It never touches the model. Individual evaluate()
implementations call _resolve_param_value(param, target_unit) to get a transiently converted
float value for the computation. The model's parameters and x_unit are never changed by a
call to evaluate().

---

Part 3 — All bugs fixed (with regression tests)

Bug 1 — evaluate() permanently mutated model state when x had a different unit

Commit: 8dfeb08a (overhaul units)
File: src/easydynamics/sample_model/components/model_component.py
What happened: Passing a scipp x in eV to a Gaussian initialised in meV would call
self.convert_unit('eV'), permanently converting all parameters. Repeated calls with
different-unit inputs would keep re-converting in a random walk.
Fix: _prepare_x_for_evaluate now returns (values, detected_unit, dim) without touching
the model. _resolve_param_value does a transient scipp scalar conversion at call time.
Regression tests:

• tests/unit/easydynamics/sample_model/components/test_model_component.py —
  test_prepare_x_for_evaluate_with_different_unit_no_warn: checks that x_unit and parameter
  values are unchanged after calling _prepare_x_for_evaluate with a compatible-but-different
  unit
• tests/unit/easydynamics/sample_model/components/test_model_component.py —
  test_resolve_param_value_converts_without_mutating: confirms the float result is correct
  (1.0 meV → 0.001 eV) and the parameter's .value and .unit are unchanged
• tests/unit/easydynamics/sample_model/components/test_model_component.py —
  test_evaluate_with_compatible_unit_gives_correct_result: end-to-end check — Gaussian in meV
  evaluated with scipp x in eV gives numerically identical output to an equivalent Gaussian in
  eV

---

Bug 2 — ComponentCollection.evaluate(output='scipp') raised TypeError on multi-component collections

Commit: 3f59e3e2 (Claude 7th review)
File: src/easydynamics/sample_model/component_collection.py
What happened: sum(component.evaluate(x, output='scipp') for component in self) seeds
with Python's integer 0. The first 0 + sc.Variable raises TypeError because scipp does
not support adding integers to Variables.
Fix: Seed the sum from the first component: first = next(gen); return sum(gen, first).
Regression test:

• tests/unit/easydynamics/sample_model/test_component_collection.py —
  test_evaluate_scipp_output_multi_component_does_not_raise

---

Bug 3 — evaluate(output='scipp') with a DataArray input produced wrong output dimension name

Commit: 3f59e3e2 (Claude 7th review)
File: src/easydynamics/sample_model/components/model_component.py
What happened: A sc.DataArray can have a coord key (e.g. 'energy') that differs from
the coordinate Variable's internal dimension name (e.g. 'x'). The old code extracted the
coord key as dim, but then immediately overwrote it with the Variable's .dims[0] when
processing the Variable. The returned sc.Variable had dimension 'x' instead of 'energy'.
Fix: A dim_from_dataarray flag prevents the Variable branch from overwriting dim when
the DataArray branch already set it.
Regression test:

• tests/unit/easydynamics/sample_model/components/test_model_component.py —
  test_evaluate_preserves_dataarray_coord_key_as_dim

---

Bug 4 — _prepare_x_for_evaluate sorted x, desynchronising output values from input positions

Commit: e90c9b1e (Claude 6th review)
File: src/easydynamics/sample_model/components/model_component.py
What happened: The function returned np.sort(x_in). When evaluate(output='scipp')
wrapped the result in a sc.Variable, the values were in sorted order but the dim coordinate
of the caller's x was in its original order — a silent ordering mismatch.
Fix: Return x_in unsorted. Callers that need a monotonic axis sort before passing x in.

---

Bug 5 — __repr__ methods referenced the removed self.unit / self._unit attribute

Commit: 47584394 (Fix post-merge issues)
Files:

• src/easydynamics/convolution/analytical_convolution.py
• src/easydynamics/convolution/convolution.py
• src/easydynamics/convolution/numerical_convolution.py
• src/easydynamics/sample_model/background_model.py
• src/easydynamics/sample_model/resolution_model.py

What happened: The develop merge introduced __repr__ methods using self.unit /
self._unit. After the overhaul renamed these to x_unit, calling repr() on any of these
objects raised AttributeError.
Fix: All five __repr__ methods updated to self.x_unit.
Regression tests: Existing test_repr methods in the corresponding test files, which all
assert 'x_unit=meV' appears in the output.

---

Bug 6 — Analysis1d.calculate() mutated self._convolver, corrupting the fit-time convolver

Commit: e90c9b1e (Claude 6th review)
File: src/easydynamics/analysis/analysis1d.py
What happened: calculate(energy=custom_grid) wrote
self._convolver = self._create_convolver(energy=custom_grid) and then set
self._convolver_is_dirty = True. This destroyed the convolver that the subsequent fit()
call would have used, forcing an unnecessary rebuild — and if fit() was called before the
rebuild completed, it could use the plot-path energy grid.
Fix: calculate() creates a local convolver and passes it explicitly to
_calculate(convolver=convolver). self._convolver is never touched by calculate().
Regression tests:

• tests/unit/easydynamics/analysis/test_analysis1d.py —
  test_calculate_does_not_dirty_convolver

---

Bug 7 — _on_Q_index_changed crashed when Q_index was None

Commit: e90c9b1e (Claude 6th review)
File: src/easydynamics/analysis/analysis1d.py
What happened: When Q_index was cleared to None, the callback still called
self.experiment.get_masked_energy(Q_index=None), raising TypeError inside the experiment.
Fix: Early return with self._masked_energy = None; self._convolver_is_dirty = True when
self._Q_index is None.
Regression test:

• tests/unit/easydynamics/analysis/test_analysis1d.py — test_on_Q_index_changed

---

Bug 8 — _verify_Q_index permitted any non-negative index when self.Q was None

Commit: f02845e2 (Claude 5th review)
File: src/easydynamics/analysis/analysis_base.py
What happened:
if Q_index < 0 or (self.Q is not None and Q_index >= len(self.Q)) — when self.Q was None
the second condition was skipped, allowing any non-negative index to pass validation even with
no Q set.
Fix: Split into two separate guards: if Q_index < 0: raise ... then
if self.Q is None or Q_index >= len(self.Q): raise ...
Regression test:

• tests/unit/easydynamics/analysis/test_analysis_base.py — _verify_Q_index tests with
  Q=None

---

Bug 9 — plot_parameters(names=[]) plotted all parameters instead of nothing

Commit: f02845e2 (Claude 5th review)
File: src/easydynamics/analysis/analysis.py
What happened: if not names: is True for an empty list, so names was replaced with
all parameter names from the dataset. The caller's explicit empty list was silently ignored.
Fix: if names is None: — only auto-populate when None was passed, not when the caller
passed [].

---

Bug 10 — plot_parameters mutated shared Parameter objects while harmonising units

Commit: f02845e2 (Claude 5th review)
File: src/easydynamics/analysis/analysis.py
What happened: When parameters from different Q indices had different units, the code called
p.convert_unit(units[name]) directly on the Parameter object shared with the model. This
permanently changed the unit of a live model parameter as a side-effect of plotting.
Fix: p = copy(p) before calling convert_unit, so only the local copy is mutated.

---

Bug 11 — ConvolutionSettings copy in _create_analysis_list only copied three fields

Commit: f02845e2 (Claude 5th review)
Files: src/easydynamics/analysis/analysis.py,
src/easydynamics/settings/convolution_settings.py
What happened: Each Analysis1d got its own ConvolutionSettings built by manually
copying upsample_factor, extension_factor, and suppress_warnings. Any field added to
ConvolutionSettings in the future would silently be omitted.
Fix: Added ConvolutionSettings.__copy__ and changed the call to
copy(self.convolution_settings).
Regression test:

• tests/unit/easydynamics/settings/test_convolution_settings.py — copy test

---

Bug 12 — ConvolutionSettings.extension_factor = None did not invalidate the plan

Commit: f02845e2 (Claude 5th review)
File: src/easydynamics/settings/convolution_settings.py
What happened: The setter's numeric validation path raised TypeError before reaching the
plan-invalidation call. None is a valid value meaning "auto-determine", but it was rejected
as non-numeric.
Fix: Added an early-return branch:
if factor is None: self._extension_factor = factor; self.convolution_plan_is_valid = False; return.

---

Bug 13 — fix_energy_offset / free_energy_offset did not update the template parameter when called without Q_index

Commit: f02845e2 (Claude 5th review)
File: src/easydynamics/sample_model/instrument_model.py
What happened: When Q_index=None, the method iterated over self._energy_offsets (the
per-Q list) but never updated self._energy_offset (the template). New Q values added later
would inherit the old fixed/free state.
Fix: self._energy_offset.fixed = fixed added before the loop.

---

Bug 14 — Energy offset subtracted without unit conversion when offset unit ≠ energy grid unit

Commit: e90c9b1e (Claude 6th review)
File: src/easydynamics/convolution/numerical_convolution.py
What happened: self.energy_offset.value was subtracted directly from the energy grid even
when the offset was stored in a different unit (e.g. offset in eV, grid in meV). The
subtraction was numerically wrong by a factor of 1000.
Fix: Convert the offset to the energy grid's unit via sc.to_unit if they differ.
Regression test:

• tests/unit/easydynamics/convolution/test_numerical_convolution.py —
  test_convolution_energy_offset_different_unit

---

Bug 15 — Detailed balance correction applied to wrong energy when using energy offsets

Commit: 3f59e3e2 (Claude 7th review)
File: src/easydynamics/convolution/numerical_convolution.py
What happened: The detailed balance factor was computed on energy_dense - offset_value,
but the sample evaluation used energy_dense - energy_even_length_offset - offset_value. The
asymmetry means the detailed balance correction was applied on a shifted grid, introducing a
systematic error when the energy offset was non-zero.
Fix: Both computations now use the same energy expression:
energy_dense - energy_even_length_offset - offset_value.
Regression test:

• tests/unit/easydynamics/convolution/test_numerical_convolution.py

---

Bug 16 — plot_data_and_model and residuals in Analysis1d included NaN data points

Commit: 3f59e3e2 (Claude 7th review)
File: src/easydynamics/analysis/analysis1d.py
What happened: The data slice experiment.binned_data['Q', self.Q_index] includes energy
bins with NaN intensity (masked or empty bins from the experiment). The model array for the
same Q has finite values at all energy points. Including NaN data in plot_data_and_model
produces broken traces; including them in residuals makes the subtraction produce all-NaN
results.
Fix: Apply experiment.get_finite_energy_mask(Q_index=self.Q_index) to the data slice in
both methods. A new Experiment.get_finite_energy_mask helper was also added.
Regression tests:

• tests/unit/easydynamics/analysis/test_analysis1d.py —
  test_plot_data_and_model_masks_nan_data, test_residuals_masks_nan_data

---

Bug 17 — ParameterAnalysis._get_q_values_and_weights rejected NaN variances that should be silently filtered

Commit: e90c9b1e (Claude 6th review)
File: src/easydynamics/analysis/parameter_analysis.py
What happened: When a parameter was absent for some Q values (e.g. a free parameter only
at certain Q), parameters_to_dataset fills np.nan. The old code treated
~np.isfinite(variances) as an error and raised ValueError, preventing Q-trend analysis for
any dataset with missing parameters.
Fix: Distinguish nan_mask (absent data — silently filter rows) from other
non-finite/non-positive variances (true errors). Only rows where the variance is NaN are
dropped; everything else raises.
Regression tests:

• tests/unit/easydynamics/analysis/test_parameter_analysis.py — NaN variance handling tests

---

Bug 18 — ModelBase.convert_x_unit rollback called convert_x_unit(None) when old_unit was None

Commit: 3f59e3e2 (Claude 7th review)
File: src/easydynamics/sample_model/model_base.py
What happened: If the model was initialised with x_unit=None and a conversion failed,
the rollback attempted component.convert_x_unit(None), which raised TypeError inside the
rollback handler — hiding the original exception.
Fix: Guard if old_unit is not None: around the rollback loop.

---

Bug 19 — ResolutionModel.from_sample_model dirty-flag race condition discarded installed collections

Commit: 3f59e3e2 (Claude 7th review)
File: src/easydynamics/sample_model/resolution_model.py
What happened: EasyScience parameter callbacks triggered during __init__ or copy could
set _component_collections_is_dirty = True on the freshly created ResolutionModel. The
next call to get_component_collection() would then rebuild the collections from scratch,
discarding the ones carefully installed by from_sample_model. This was especially likely to
happen after normalize_area() or fix_all_parameters() triggered parameter-change callbacks.
Fix: Explicitly reset _component_collections_is_dirty = False after installing
collections, and again after the normalize_area() / fix_all_parameters() calls.

---

Bug 20 — DeltaLorentz.calculate_width silently failed when called before Q was set

Commit: 3f59e3e2 (Claude 7th review)
File: src/easydynamics/sample_model/diffusion_model/delta_lorentz.py
What happened: _lorentzian_width_list was empty before Q was set. Calling
calculate_width() in that state raised StopIteration or returned a wrong value depending on
the internal path.
Fix: Explicit if not self._lorentzian_width_list: raise ValueError(...) with a clear
message.
Regression test:

• tests/unit/easydynamics/sample_model/diffusion_model/test_delta_lorentz.py

---

Bug 21 — _invalidate_plan_on_change type annotation was dict[str, object] instead of set[str]

Commit: f02845e2 (Claude 5th review)
File: src/easydynamics/convolution/convolution.py
What happened: The class variable was annotated as ClassVar[dict[str, object]] but is
actually a set. The wrong annotation did not cause a runtime error but could mislead a reader
or confuse a type checker.
Fix: Corrected to ClassVar[set[str]].

---

[codecov]

Codecov Report

❌ Patch coverage is 91.05960% with 54 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.35%. Comparing base (ee07ccf) to head (4e5512d).

Files with missing lines	Patch %	Lines
src/easydynamics/analysis/analysis1d.py	68.18%	4 Missing and 3 partials ⚠️
src/easydynamics/convolution/convolution_base.py	78.94%	1 Missing and 3 partials ⚠️
src/easydynamics/experiment/experiment.py	42.85%	2 Missing and 2 partials ⚠️
...ple_model/components/damped_harmonic_oscillator.py	92.30%	4 Missing ⚠️
...dynamics/sample_model/components/delta_function.py	92.59%	4 Missing ⚠️
...c/easydynamics/sample_model/components/gaussian.py	92.15%	4 Missing ⚠️
...easydynamics/sample_model/components/lorentzian.py	92.30%	4 Missing ⚠️
src/easydynamics/sample_model/components/voigt.py	92.59%	4 Missing ⚠️
src/easydynamics/analysis/parameter_analysis.py	66.66%	2 Missing and 1 partial ⚠️
...asydynamics/sample_model/components/exponential.py	93.61%	3 Missing ⚠️
... and 7 more

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #215      +/-   ##
===========================================
- Coverage    97.89%   97.35%   -0.54%     
===========================================
  Files           52       52              
  Lines         3653     4008     +355     
  Branches       650      692      +42     
===========================================
+ Hits          3576     3902     +326     
- Misses          45       69      +24     
- Partials        32       37       +5     

Flag	Coverage Δ
unittests	97.35% <91.05%> (-0.54%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/easydynamics/analysis/analysis.py	95.96% <100.00%> (+0.91%)	⬆️
src/easydynamics/analysis/analysis_base.py	100.00% <100.00%> (ø)
...asydynamics/base_classes/easydynamics_modelbase.py	100.00% <100.00%> (ø)
...easydynamics/convolution/analytical_convolution.py	98.76% <ø> (ø)
src/easydynamics/convolution/convolution.py	100.00% <100.00%> (ø)
.../easydynamics/convolution/numerical_convolution.py	100.00% <100.00%> (+4.16%)	⬆️
...dynamics/convolution/numerical_convolution_base.py	97.39% <100.00%> (+0.02%)	⬆️
src/easydynamics/sample_model/background_model.py	100.00% <ø> (ø)
...easydynamics/sample_model/components/polynomial.py	100.00% <100.00%> (ø)
...iffusion_model/brownian_translational_diffusion.py	100.00% <100.00%> (ø)
... and 23 more

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.