Skip to content

Unit overhaul 4/6: Separate unit into x_unit/y_unit and update tutorials#224

Open
henrikjacobsenfys wants to merge 2 commits into
split/02b-q-variablefrom
split/02c-unit-rename
Open

Unit overhaul 4/6: Separate unit into x_unit/y_unit and update tutorials#224
henrikjacobsenfys wants to merge 2 commits into
split/02b-q-variablefrom
split/02c-unit-rename

Conversation

@henrikjacobsenfys

@henrikjacobsenfys henrikjacobsenfys commented Jul 6, 2026

Copy link
Copy Markdown
Member

Replace the single unit on EasyDynamicsModelBase with independent x_unit (axis, default meV) and y_unit (output, default dimensionless), and migrate every subclass in the hierarchy: components, diffusion models, convolution, instrument/resolution/sample/background models, experiment and analysis. convert_unit becomes convert_x_unit/convert_y_unit, implemented via a shared rollback helper, and evaluate gains an output='numpy'|'scipp' selector. Also adds the additive get_fit_targets() model API (consumed by #226).

The base-class rename is irreducibly atomic — it forces all subclasses to change together, so this core can't be split further without a temporary compatibility shim.

Tutorial-notebook updates are distributed across the stack so they run green on every PR, not just at the top — each PR's tutorial-tests job executes all notebooks against that branch. This PR carries only the x_unit/y_unit notebook edits (sample_model and tutorial0_more_advanced); the FitBinding(targets=…) notebook changes land in #226 and the ExpressionComponent ones in #227.

Two independent changes that were previously bundled into this PR have been carved into stacked follow-ups: the FitBinding/FitTarget redesign (#226) and the ExpressionComponent physical-constants/parameter-units feature (#227).

All unit tests pass.


Stack — splitting the monolithic "Unit system overhaul" (#217) into a reviewable series. Merge bottom-up:

  1. Add unit-conversion utilities and FitTarget → develop (Unit overhaul 1/6: Add unit-conversion utilities and FitTarget #221)
  2. Centralize Q-index validation via verify_Q_indexsplit/01-unit-utils (Unit overhaul 2/6: Centralize Q-index validation via verify_Q_index #222)
  3. Store Q as a scipp Variable in 1/angstrom → split/02a-verify-q-index (Unit overhaul 3/6: Store Q as a scipp Variable in 1/angstrom #223)
  4. Separate model unit into x_unit/y_unit + tutorialssplit/02b-q-variable (Unit overhaul 4/6: Separate unit into x_unit/y_unit and update tutorials #224)
  5. Redesign FitBinding around FitTargetssplit/02c-unit-rename (Unit overhaul 5/6: Redesign FitBinding around FitTargets #226)
  6. ExpressionComponent physical constants + parameter unitssplit/02d-fit-binding (Unit overhaul 6/6: ExpressionComponent physical constants and parameter units #227)

🤖 Generated with Claude Code

@henrikjacobsenfys henrikjacobsenfys added [scope] enhancement Adds/improves features (major.MINOR.patch) [priority] medium Normal/default priority labels Jul 6, 2026
@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.48454% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.66%. Comparing base (2379d58) to head (2aa65b8).

Files with missing lines Patch % Lines
src/easydynamics/analysis/analysis1d.py 96.00% 0 Missing and 1 partial ⚠️
src/easydynamics/sample_model/components/mixins.py 87.50% 0 Missing and 1 partial ⚠️
src/easydynamics/sample_model/sample_model.py 95.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@                   Coverage Diff                    @@
##           split/02b-q-variable     #224      +/-   ##
========================================================
+ Coverage                 97.47%   98.66%   +1.18%     
========================================================
  Files                        53       53              
  Lines                      3687     3892     +205     
  Branches                    640      664      +24     
========================================================
+ Hits                       3594     3840     +246     
+ Misses                       62       29      -33     
+ Partials                     31       23       -8     
Flag Coverage Δ
unittests 98.66% <99.48%> (+1.18%) ⬆️

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 96.41% <100.00%> (+1.34%) ⬆️
...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%> (ø)
src/easydynamics/convolution/convolution_base.py 100.00% <100.00%> (ø)
.../easydynamics/convolution/numerical_convolution.py 100.00% <100.00%> (+4.16%) ⬆️
...dynamics/convolution/numerical_convolution_base.py 97.56% <100.00%> (+0.19%) ⬆️
src/easydynamics/experiment/experiment.py 98.88% <100.00%> (+1.22%) ⬆️
src/easydynamics/sample_model/background_model.py 100.00% <ø> (ø)
.../easydynamics/sample_model/component_collection.py 100.00% <100.00%> (+0.86%) ⬆️
... and 20 more

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@henrikjacobsenfys henrikjacobsenfys changed the title Unit overhaul 4/5: Separate model unit into x_unit and y_unit Unit overhaul 4/4: Separate unit into x_unit/y_unit and update tutorials Jul 6, 2026
@henrikjacobsenfys henrikjacobsenfys changed the title Unit overhaul 4/4: Separate unit into x_unit/y_unit and update tutorials Unit overhaul 4/6: Separate unit into x_unit/y_unit and update tutorials Jul 6, 2026
Replace the single `unit` on EasyDynamicsModelBase with independent
`x_unit` (axis, default meV) and `y_unit` (output, default dimensionless),
and migrate every subclass in the hierarchy. `convert_unit` becomes
`convert_x_unit`/`convert_y_unit`, and `evaluate` gains an
`output='numpy'|'scipp'` selector. Includes tutorial-notebook updates.

Core of the unit-system overhaul; the FitBinding/FitTarget redesign and
ExpressionComponent physical constants are split into follow-up PRs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[priority] medium Normal/default priority [scope] enhancement Adds/improves features (major.MINOR.patch)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant