Fix units of model_base

docs/docs/tutorials/delta_lorentz.ipynb

@@ -75,8 +75,8 @@
    "metadata": {},
    "outputs": [],
    "source": [
-    "diffusion_model._A_0_list[3].value = 0.02  # noqa\n",
-    "diffusion_model._lorentzian_width_list[5].value = 0.1  # noqa"
+    "diffusion_model._A_0_list[3].value = 0.02  # ruff: ignore[private-member-access]\n",
+    "diffusion_model._lorentzian_width_list[5].value = 0.1  # ruff: ignore[private-member-access]"
    ]
   },
   {

src/easydynamics/analysis/analysis.py

@@ -200,7 +200,7 @@ def rebin(
             If rebinning changes Q and ``confirm`` is not ``True``.
         """
         old_Q = np.asarray(self.Q.values) if self.Q is not None else None
-        old_binned_data = self.experiment._binned_data  # noqa: SLF001
+        old_binned_data = self.experiment._binned_data  # ruff: ignore[private-member-access]
 
         self.experiment.rebin(dimensions)
         new_Q = np.asarray(self.Q.values) if self.Q is not None else None
@@ -212,7 +212,7 @@ def rebin(
         )
 
         if q_changed and not confirm:
-            self.experiment._binned_data = old_binned_data  # noqa: SLF001
+            self.experiment._binned_data = old_binned_data  # ruff: ignore[private-member-access]
             raise ValueError(
                 'Rebinning changed Q values, which requires clearing Q from sample_model and '
                 'instrument_model (including resolution and background sub-models). '
@@ -756,7 +756,7 @@ def _fit_all_Q_simultaneously(self) -> FitResults:
         ys = []
         ws = []
 
-        # TODO: consider using scipp built-in masking instead of numpy boolean masks,  # noqa: FIX002 TD002 TD003
+        # TODO: consider using scipp built-in masking instead of numpy boolean masks,  # ruff: ignore[line-contains-todo, missing-todo-author, missing-todo-link]
 
         for analysis1d in self.analysis_list:
             x, y, weight, mask = self.experiment.extract_x_y_weights_only_finite(
@@ -812,7 +812,11 @@ def _create_model_array(self, energy: sc.Variable | None = None) -> sc.DataArray
         """
         if energy is None:
             energy = self.energy
-        model = sc.array(dims=['Q', 'energy'], values=self.calculate(energy=energy))
+        model = sc.array(
+            dims=['Q', 'energy'],
+            values=self.calculate(energy=energy),
+            unit=self.sample_model.y_unit,
+        )
         return sc.DataArray(
             data=model,
             coords={'Q': self.Q, 'energy': energy},
@@ -859,7 +863,7 @@ def _create_components_dataset(
             energy = self.energy
 
         datasets = [
-            analysis1d._create_components_dataset_single_Q(  # noqa: SLF001
+            analysis1d._create_components_dataset_single_Q(  # ruff: ignore[private-member-access]
                 add_background=add_background, energy=energy
             )
             for analysis1d in self.analysis_list

src/easydynamics/analysis/analysis1d.py

@@ -270,7 +270,7 @@ def fit(self) -> FitResults:
     def as_fit_function(
         self,
         _x: np.ndarray | sc.Variable | None = None,
-        **kwargs: dict[str, Any],  # noqa: ARG002
+        **kwargs: dict[str, Any],  # ruff: ignore[unused-method-argument]
     ) -> callable:
         """
         Return self._calculate as a fit function.
@@ -294,7 +294,7 @@ def as_fit_function(
 
         def fit_function(
             _x: np.ndarray | sc.Variable | None = None,
-            **kwargs: dict[str, Any],  # noqa: ARG001
+            **kwargs: dict[str, Any],  # ruff: ignore[unused-function-argument]
         ) -> np.ndarray:
             """Fit function."""
             return self._calculate()
@@ -736,6 +736,8 @@ def _build_convolution(
             convolution_settings=self.convolution_settings,
             temperature=self.temperature,
             detailed_balance_settings=self.detailed_balance_settings,
+            x_unit=self.sample_model.x_unit,
+            y_unit=self.sample_model.y_unit,
         )
 
     #############
@@ -854,7 +856,7 @@ def _to_scipp_array(
         if energy is None:
             energy = self._masked_energy
         return sc.DataArray(
-            data=sc.array(dims=['energy'], values=values),
+            data=sc.array(dims=['energy'], values=values, unit=self.sample_model.y_unit),
             coords={
                 'energy': energy,
                 'Q': self.Q[self.Q_index],

src/easydynamics/base_classes/easydynamics_list.py

@@ -206,7 +206,7 @@ def _validate_type(self, value: object) -> None:
         if not isinstance(value, tuple(self._protected_types)):
             allowed = ', '.join(t.__name__ for t in self._protected_types)
             raise TypeError(
-                f'Value must be an instance of type: {allowed}. Got {type(value).__name__} instead.'  # noqa: E501
+                f'Value must be an instance of type: {allowed}. Got {type(value).__name__} instead.'  # ruff: ignore[line-too-long]
             )
 
     # ------------------------------------------------------------------

src/easydynamics/convolution/convolution.py

@@ -249,9 +249,9 @@ def _build_convolution_plan(self) -> None:
         Separate sample model components into analytical pairs, delta functions, and the rest.
         """
 
-        analytical_sample_components = ComponentCollection()
-        delta_sample_components = ComponentCollection()
-        numerical_sample_components = ComponentCollection()
+        analytical_sample_components = ComponentCollection(x_unit=self.x_unit, y_unit=self.y_unit)
+        delta_sample_components = ComponentCollection(x_unit=self.x_unit, y_unit=self.y_unit)
+        numerical_sample_components = ComponentCollection(x_unit=self.x_unit, y_unit=self.y_unit)
 
         for sample_component in self._sample_components:
             # If delta function, put in delta sample model and go to the
@@ -341,9 +341,9 @@ def convert_y_unit(self, unit: str) -> None:
         # The sub-convolvers share this convolver's component objects, which were already
         # converted by super(); only their y-unit labels need updating.
         if getattr(self, '_analytical_convolver', None) is not None:
-            self._analytical_convolver._relabel_y_unit(self.y_unit)  # noqa: SLF001
+            self._analytical_convolver._relabel_y_unit(self.y_unit)  # ruff: ignore[private-member-access]
         if getattr(self, '_numerical_convolver', None) is not None:
-            self._numerical_convolver._relabel_y_unit(self.y_unit)  # noqa: SLF001
+            self._numerical_convolver._relabel_y_unit(self.y_unit)  # ruff: ignore[private-member-access]
 
     # Update some setters so the internal sample models are updated
     def __setattr__(self, name: str, value: any) -> None:

src/easydynamics/convolution/convolution_base.py

@@ -96,20 +96,28 @@ def __init__(
             isinstance(sample_components, (ComponentCollection, ModelComponent))
         ):
             raise TypeError(
-                f'`sample_components` is an instance of {type(sample_components).__name__}, but must be a ComponentCollection or ModelComponent.'  # noqa: E501
+                f'`sample_components` is an instance of {type(sample_components).__name__}, but must be a ComponentCollection or ModelComponent.'  # ruff: ignore[line-too-long]
             )
         if isinstance(sample_components, ModelComponent):
-            sample_components = ComponentCollection(components=[sample_components])
+            sample_components = ComponentCollection(
+                components=[sample_components],
+                x_unit=sample_components.x_unit,
+                y_unit=sample_components.y_unit,
+            )
         self._sample_components = sample_components
 
         if resolution_components is not None and not (
             isinstance(resolution_components, (ComponentCollection, ModelComponent))
         ):
             raise TypeError(
-                f'`resolution_components` is an instance of {type(resolution_components).__name__}, but must be a ComponentCollection or ModelComponent.'  # noqa: E501
+                f'`resolution_components` is an instance of {type(resolution_components).__name__}, but must be a ComponentCollection or ModelComponent.'  # ruff: ignore[line-too-long]
             )
         if isinstance(resolution_components, ModelComponent):
-            resolution_components = ComponentCollection(components=[resolution_components])
+            resolution_components = ComponentCollection(
+                components=[resolution_components],
+                x_unit=resolution_components.x_unit,
+                y_unit=resolution_components.y_unit,
+            )
         self._resolution_components = resolution_components
 
     @property
@@ -314,11 +322,15 @@ def sample_components(self, sample_components: ComponentCollection | ModelCompon
         """
         if not isinstance(sample_components, (ComponentCollection, ModelComponent)):
             raise TypeError(
-                f'`sample_components` is an instance of {type(sample_components).__name__}, but must be a ComponentCollection or ModelComponent.'  # noqa: E501
+                f'`sample_components` is an instance of {type(sample_components).__name__}, but must be a ComponentCollection or ModelComponent.'  # ruff: ignore[line-too-long]
             )
 
         if isinstance(sample_components, ModelComponent):
-            sample_components = ComponentCollection(components=[sample_components])
+            sample_components = ComponentCollection(
+                components=[sample_components],
+                x_unit=sample_components.x_unit,
+                y_unit=sample_components.y_unit,
+            )
         self._sample_components = sample_components
 
     @property
@@ -353,9 +365,13 @@ def resolution_components(
         """
         if not isinstance(resolution_components, (ComponentCollection, ModelComponent)):
             raise TypeError(
-                f'`resolution_components` is an instance of {type(resolution_components).__name__}, but must be a ComponentCollection or ModelComponent.'  # noqa: E501
+                f'`resolution_components` is an instance of {type(resolution_components).__name__}, but must be a ComponentCollection or ModelComponent.'  # ruff: ignore[line-too-long]
             )
 
         if isinstance(resolution_components, ModelComponent):
-            resolution_components = ComponentCollection(components=[resolution_components])
+            resolution_components = ComponentCollection(
+                components=[resolution_components],
+                x_unit=resolution_components.x_unit,
+                y_unit=resolution_components.y_unit,
+            )
         self._resolution_components = resolution_components

src/easydynamics/convolution/numerical_convolution_base.py

@@ -139,11 +139,11 @@ def _convolution_plan_is_current(self) -> bool:
         seen_version = getattr(self, '_plan_seen_version', None)
         if seen_version is None:
             return False
-        return self.convolution_settings._plan_valid_for(seen_version)  # noqa: SLF001
+        return self.convolution_settings._plan_valid_for(seen_version)  # ruff: ignore[private-member-access]
 
     def _mark_convolution_plan_current(self) -> None:
         """Record that this convolver's plan matches its current state and settings."""
-        self._plan_seen_version = self.convolution_settings._plan_version  # noqa: SLF001
+        self._plan_seen_version = self.convolution_settings._plan_version  # ruff: ignore[private-member-access]
 
     @property
     def convolution_settings(self) -> ConvolutionSettings:
@@ -358,7 +358,7 @@ def _create_energy_grid(
             is_uniform = np.allclose(energy_diff, energy_diff[0])
             if not is_uniform:
                 raise ValueError(
-                    'Input array `energy` must be uniformly spaced if upsample_factor is not given.'  # noqa: E501
+                    'Input array `energy` must be uniformly spaced if upsample_factor is not given.'  # ruff: ignore[line-too-long]
                 )
             energy_dense = self.energy.values
 

src/easydynamics/sample_model/components/expression_component.py

@@ -474,13 +474,13 @@ def _relabel_parameter_unit(self, name: str, unit: str | sc.Unit) -> None:
 
         # Parameter.unit is read-only and convert_unit rescales the value, so a pure relabel
         # has to swap the underlying scipp scalars (value and bounds) directly.
-        param._scalar = sc.scalar(  # noqa: SLF001
+        param._scalar = sc.scalar(  # ruff: ignore[private-member-access]
             param.value,
             unit=new_unit,
-            variance=param._scalar.variance,  # noqa: SLF001
+            variance=param._scalar.variance,  # ruff: ignore[private-member-access]
         )
-        param._min = sc.scalar(param._min.value, unit=new_unit)  # noqa: SLF001
-        param._max = sc.scalar(param._max.value, unit=new_unit)  # noqa: SLF001
+        param._min = sc.scalar(param._min.value, unit=new_unit)  # ruff: ignore[private-member-access]
+        param._max = sc.scalar(param._max.value, unit=new_unit)  # ruff: ignore[private-member-access]
 
     @classmethod
     def _create_physical_constant(cls, name: str) -> DescriptorNumber:

src/easydynamics/sample_model/diffusion_model/delta_lorentz.py

@@ -534,7 +534,7 @@ def create_component_collections(
                 y_unit=self.y_unit,
             )
             if self._allow_Q_variation['lorentzian_width'] is True:
-                lorz_component._width = self._lorentzian_width_list[i]  # noqa: SLF001
+                lorz_component._width = self._lorentzian_width_list[i]  # ruff: ignore[private-member-access]
 
             # If the width is allowed to vary with Q it is independent.
             # If the width is not allowed to vary with Q it must be made

src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py

@@ -317,7 +317,7 @@ def _assert_convertible_to_mev(unit: str | sc.Unit) -> None:
             test.convert_unit('meV')
         except Exception as e:
             raise UnitError(
-                f'Invalid unit: {unit}. Unit must be a string or scipp Unit and convertible to meV.'  # noqa: E501
+                f'Invalid unit: {unit}. Unit must be a string or scipp Unit and convertible to meV.'  # ruff: ignore[line-too-long]
             ) from e
 
     def convert_x_unit(self, unit: str | sc.Unit) -> None:

src/easydynamics/sample_model/model_base.py

@@ -73,7 +73,7 @@ def __init__(
                 f'got {type(components).__name__}'
             )
 
-        self._components = ComponentCollection()
+        self._components = ComponentCollection(x_unit=self.x_unit, y_unit=self.y_unit)
         self._component_collections: list[ComponentCollection] = []
         self._component_collections_is_dirty = True
         if isinstance(components, (ModelComponent, ComponentCollection)):
@@ -313,7 +313,9 @@ def _convert_axis_unit(self, unit: str | sc.Unit, axis: str) -> None:
 
         method = f'convert_{axis}_unit'
         old_unit = self.x_unit if axis == 'x' else self.y_unit
-        children = [*self.components, *self._component_collections]
+        # Convert the template collection as a whole (not its unpacked components) so its own
+        # unit attribute is updated too; regenerated per-Q collections copy that attribute.
+        children = [self._components, *self._component_collections]
         convert_units_with_rollback([
             (getattr(child, method), unit, old_unit) for child in children
         ])

src/easydynamics/sample_model/sample_model.py

@@ -181,7 +181,7 @@ def append_diffusion_model(self, diffusion_model: DiffusionModelBase) -> None:
         """
         if not isinstance(diffusion_model, DiffusionModelBase):
             raise TypeError(
-                f'diffusion_model must be a DiffusionModelBase, got {type(diffusion_model).__name__}'  # noqa: E501
+                f'diffusion_model must be a DiffusionModelBase, got {type(diffusion_model).__name__}'  # ruff: ignore[line-too-long]
             )
         diffusion_model.Q = self.Q
         self._diffusion_models.append(diffusion_model)
@@ -355,7 +355,7 @@ def temperature_unit(self, _value: str | sc.Unit) -> None:
 
         raise AttributeError(
             f'Temperature_unit is read-only. Use convert_temperature_unit to change the unit '
-            f'between allowed types or create a new {self.__class__.__name__} with the desired unit.'  # noqa: E501
+            f'between allowed types or create a new {self.__class__.__name__} with the desired unit.'  # ruff: ignore[line-too-long]
         )
 
     def convert_temperature_unit(self, unit: str | sc.Unit) -> None:

tests/unit/easydynamics/analysis/test_analysis.py

@@ -931,6 +931,30 @@ def test_create_model_array(self, analysis):
             np.array([[1.0, 2.0, 3.0], [4.0, 5.0, 6.0], [7.0, 8.0, 9.0]]),
         )
 
+    def test_create_model_array_and_residuals_with_counts_data(self):
+        "Regression: model arrays must carry the sample model y_unit so counts data works"
+        # WHEN
+        Q = sc.array(dims=['Q'], values=[1.0, 2.0], unit='1/Angstrom')
+        energy = sc.linspace('energy', -5.0, 5.0, num=20, unit='meV')
+        values = np.ones((2, 20))
+        data_array = sc.DataArray(
+            data=sc.array(dims=['Q', 'energy'], values=values, variances=values, unit='counts'),
+            coords={'Q': Q, 'energy': energy},
+        )
+        analysis = Analysis(
+            experiment=Experiment(data=data_array),
+            sample_model=SampleModel(y_unit='counts', components=Gaussian(y_unit='counts')),
+            instrument_model=InstrumentModel(),
+        )
+
+        # THEN
+        model_array = analysis._create_model_array()
+        residuals = analysis._create_residuals_array()
+
+        # EXPECT
+        assert model_array.unit == sc.Unit('counts')
+        assert residuals.unit == sc.Unit('counts')
+
     def test_create_residuals_array(self, analysis):
         # WHEN
         # Mock the _create_model_array method to return a specific model array

tests/unit/easydynamics/analysis/test_analysis1d.py

@@ -13,6 +13,7 @@
 from easydynamics.analysis.analysis1d import Analysis1d
 from easydynamics.experiment import Experiment
 from easydynamics.sample_model import InstrumentModel
+from easydynamics.sample_model import ResolutionModel
 from easydynamics.sample_model import SampleModel
 from easydynamics.sample_model.component_collection import ComponentCollection
 from easydynamics.sample_model.components.gaussian import Gaussian
@@ -770,6 +771,52 @@ def test_create_model_array(self, analysis1d):
             np.array([1.0, 2.0, 3.0]),
         )
 
+    @pytest.fixture
+    def analysis1d_counts(self):
+        "Analysis1d with data and all models in counts, as loaded from a real experiment"
+        Q = sc.array(dims=['Q'], values=[1.0], unit='1/Angstrom')
+        energy = sc.linspace('energy', -5.0, 5.0, num=20, unit='meV')
+        values = np.ones((1, 20))
+        data_array = sc.DataArray(
+            data=sc.array(dims=['Q', 'energy'], values=values, variances=values, unit='counts'),
+            coords={'Q': Q, 'energy': energy},
+        )
+
+        return Analysis1d(
+            experiment=Experiment(data=data_array),
+            sample_model=SampleModel(y_unit='counts', components=Gaussian(y_unit='counts')),
+            instrument_model=InstrumentModel(
+                resolution_model=ResolutionModel(components=Gaussian(width=0.5))
+            ),
+            Q_index=0,
+        )
+
+    def test_model_array_has_sample_model_y_unit(self, analysis1d_counts):
+        "Regression: model arrays must carry the sample model y_unit, not dimensionless"
+        # WHEN THEN
+        model_array = analysis1d_counts._create_model_array()
+
+        # EXPECT
+        assert model_array.unit == sc.Unit('counts')
+
+    def test_residuals_and_datagroup_with_counts_data(self, analysis1d_counts):
+        "Regression: residuals (data - model) must work when the data is not dimensionless"
+        # WHEN THEN
+        datagroup = analysis1d_counts.data_and_model_to_datagroup(include_residuals=True)
+
+        # EXPECT
+        assert datagroup['Model'].unit == sc.Unit('counts')
+        assert datagroup['Residuals'].unit == sc.Unit('counts')
+
+    def test_convolver_gets_sample_model_units(self, analysis1d_counts):
+        "Regression: the convolver must be built with the sample model's units"
+        # WHEN THEN
+        convolver = analysis1d_counts._create_convolver()
+
+        # EXPECT
+        assert convolver.x_unit == analysis1d_counts.sample_model.x_unit
+        assert convolver.y_unit == 'counts'
+
     @pytest.mark.parametrize(
         'add_background',
         [True, False],