From b714a2a3097ba3f9b96d0ca0d9e58ed9c488306b Mon Sep 17 00:00:00 2001 From: henrikjacobsenfys Date: Thu, 26 Mar 2026 03:54:27 +0100 Subject: [PATCH 1/4] work on normalization --- docs/docs/tutorials/tutorial1_brownian.ipynb | 4 +- src/easydynamics/analysis/analysis1d.py | 84 +++++++++++-------- src/easydynamics/analysis/analysis_base.py | 40 +++++---- .../sample_model/instrument_model.py | 66 ++++++++------- src/easydynamics/sample_model/model_base.py | 53 +++++++----- 5 files changed, 147 insertions(+), 100 deletions(-) diff --git a/docs/docs/tutorials/tutorial1_brownian.ipynb b/docs/docs/tutorials/tutorial1_brownian.ipynb index 1eaed39c..2cdd4276 100644 --- a/docs/docs/tutorials/tutorial1_brownian.ipynb +++ b/docs/docs/tutorials/tutorial1_brownian.ipynb @@ -330,7 +330,7 @@ "metadata": {}, "outputs": [], "source": [ - "diffusion_experiment.plot_data(slicer=True)" + "diffusion_experiment.plot_data(slicer=True,ymax=4)" ] }, { @@ -565,7 +565,7 @@ "outputs": [], "source": [ "diffusion_model_analysis.fit(fit_method='simultaneous')\n", - "diffusion_model_analysis.plot_data_and_model()" + "diffusion_model_analysis.plot_data_and_model(ymax=2)" ] }, { diff --git a/src/easydynamics/analysis/analysis1d.py b/src/easydynamics/analysis/analysis1d.py index ec6c3c8d..e09c7608 100644 --- a/src/easydynamics/analysis/analysis1d.py +++ b/src/easydynamics/analysis/analysis1d.py @@ -30,7 +30,7 @@ class Analysis1d(AnalysisBase): def __init__( self, - display_name: str | None = 'MyAnalysis', + display_name: str | None = "MyAnalysis", unique_name: str | None = None, experiment: Experiment | None = None, sample_model: SampleModel | None = None, @@ -166,7 +166,7 @@ def fit(self) -> FitResults: Analysis. """ if self._experiment is None: - raise ValueError('No experiment is associated with this Analysis.') + raise ValueError("No experiment is associated with this Analysis.") # Create convolver once to reuse during fitting self._convolver = self._create_convolver() @@ -262,13 +262,13 @@ def plot_data_and_model( import plopp as pp if self.experiment.data is None: - raise ValueError('No data to plot. Please load data first.') + raise ValueError("No data to plot. Please load data first.") energy = self._verify_energy(energy) if energy is None: energy = self._masked_energy - data = self.experiment.data['Q', self.Q_index] + data = self.experiment.data["Q", self.Q_index] model_array = self._create_sample_scipp_array(energy=energy) component_dataset = self._create_components_dataset_single_Q( @@ -277,24 +277,26 @@ def plot_data_and_model( # Create a dataset containing the data, model, and individual # components for plotting. - data_and_model = sc.Dataset({ - 'Data': data, - 'Model': model_array, - }) + data_and_model = sc.Dataset( + { + "Data": data, + "Model": model_array, + } + ) data_and_model = sc.merge(data_and_model, component_dataset) plot_kwargs_defaults = { - 'title': self.display_name, - 'linestyle': {'Data': 'none', 'Model': '-'}, - 'marker': {'Data': 'o', 'Model': 'none'}, - 'color': {'Data': 'black', 'Model': 'red'}, - 'markerfacecolor': {'Data': 'none', 'Model': 'none'}, + "title": self.display_name, + "linestyle": {"Data": "none", "Model": "-"}, + "marker": {"Data": "o", "Model": "none"}, + "color": {"Data": "black", "Model": "red"}, + "markerfacecolor": {"Data": "none", "Model": "none"}, } if plot_components: for comp_name in component_dataset: - plot_kwargs_defaults['linestyle'][comp_name] = '--' - plot_kwargs_defaults['marker'][comp_name] = None + plot_kwargs_defaults["linestyle"][comp_name] = "--" + plot_kwargs_defaults["marker"][comp_name] = None # Overwrite defaults with any user-provided kwargs plot_kwargs_defaults.update(kwargs) @@ -320,7 +322,7 @@ def _require_Q_index(self) -> int: ValueError: If the Q index is not set. """ if self._Q_index is None: - raise ValueError('Q_index must be set.') + raise ValueError("Q_index must be set.") return self._Q_index def _on_Q_index_changed(self) -> None: @@ -349,7 +351,9 @@ def _verify_energy(self, energy: sc.Variable | None) -> sc.Variable | None: """ if energy is not None and not isinstance(energy, sc.Variable): - raise TypeError(f'Energy must be a sc.Variable or None. Got {type(energy)}.') + raise TypeError( + f"Energy must be a sc.Variable or None. Got {type(energy)}." + ) return energy def _calculate_energy_with_offset( @@ -376,8 +380,8 @@ def _calculate_energy_with_offset( energy_offset.convert_unit(str(energy.unit)) except Exception as e: raise sc.UnitError( - f'Energy and energy offset must have compatible units. ' - f'Got {energy.unit} and {energy_offset.unit}.' + f"Energy and energy offset must have compatible units. " + f"Got {energy.unit} and {energy_offset.unit}." ) from e energy_with_offset = energy.copy(deep=True) @@ -453,7 +457,9 @@ def _evaluate_components( # performance is not important. # We don't create a convolver if the resolution is empty. - resolution = self.instrument_model.resolution_model.get_component_collection(Q_index) + resolution = self.instrument_model.resolution_model.get_component_collection( + Q_index + ) if resolution.is_empty: return components.evaluate(energy_with_offset) @@ -527,8 +533,10 @@ def _evaluate_background(self, energy: sc.Variable | None = None) -> np.ndarray: np.ndarray: The evaluated background contribution. """ Q_index = self._require_Q_index() - background_components = self.instrument_model.background_model.get_component_collection( - Q_index=Q_index + background_components = ( + self.instrument_model.background_model.get_component_collection( + Q_index=Q_index + ) ) return self._evaluate_components( components=background_components, @@ -589,8 +597,8 @@ def _create_convolver( if sample_components.is_empty: return None - resolution_components = self.instrument_model.resolution_model.get_component_collection( - Q_index + resolution_components = ( + self.instrument_model.resolution_model.get_component_collection(Q_index) ) if resolution_components.is_empty: return None @@ -658,7 +666,9 @@ def _create_background_component_scipp_array( ) return self._to_scipp_array(values=values, energy=energy) - def _create_sample_scipp_array(self, energy: sc.Variable | None = None) -> sc.DataArray: + def _create_sample_scipp_array( + self, energy: sc.Variable | None = None + ) -> sc.DataArray: """Create a scipp DataArray for the full sample model including background. @@ -697,22 +707,28 @@ def _create_components_dataset_single_Q( Q_index=self.Q_index ).components - background_components = self.instrument_model.background_model.get_component_collection( - Q_index=self.Q_index - ).components + background_components = ( + self.instrument_model.background_model.get_component_collection( + Q_index=self.Q_index + ).components + ) if energy is None: energy = self._masked_energy - background = self._evaluate_background(energy=energy) if add_background else None + background = ( + self._evaluate_background(energy=energy) if add_background else None + ) for component in sample_components: scipp_arrays[component.display_name] = self._create_component_scipp_array( component=component, background=background, energy=energy ) for component in background_components: - scipp_arrays[component.display_name] = self._create_background_component_scipp_array( - component=component, energy=energy + scipp_arrays[component.display_name] = ( + self._create_background_component_scipp_array( + component=component, energy=energy + ) ) return sc.Dataset(scipp_arrays) @@ -737,9 +753,9 @@ 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), coords={ - 'energy': energy, - 'Q': self.Q[self.Q_index], + "energy": energy, + "Q": self.Q[self.Q_index], }, ) diff --git a/src/easydynamics/analysis/analysis_base.py b/src/easydynamics/analysis/analysis_base.py index fe79adff..f3c101b7 100644 --- a/src/easydynamics/analysis/analysis_base.py +++ b/src/easydynamics/analysis/analysis_base.py @@ -24,7 +24,7 @@ class AnalysisBase(EasyScienceModelBase): def __init__( self, - display_name: str | None = 'MyAnalysis', + display_name: str | None = "MyAnalysis", unique_name: str | None = None, experiment: Experiment | None = None, sample_model: SampleModel | None = None, @@ -65,21 +65,23 @@ def __init__( elif isinstance(experiment, Experiment): self._experiment = experiment else: - raise TypeError('experiment must be an instance of Experiment or None.') + raise TypeError("experiment must be an instance of Experiment or None.") if sample_model is None: self._sample_model = SampleModel() elif isinstance(sample_model, SampleModel): self._sample_model = sample_model else: - raise TypeError('sample_model must be an instance of SampleModel or None.') + raise TypeError("sample_model must be an instance of SampleModel or None.") if instrument_model is None: self._instrument_model = InstrumentModel() elif isinstance(instrument_model, InstrumentModel): self._instrument_model = instrument_model else: - raise TypeError('instrument_model must be an instance of InstrumentModel or None.') + raise TypeError( + "instrument_model must be an instance of InstrumentModel or None." + ) if extra_parameters is not None: if isinstance(extra_parameters, Parameter): @@ -89,7 +91,9 @@ def __init__( ): self._extra_parameters = extra_parameters else: - raise TypeError('extra_parameters must be a Parameter or a list of Parameters.') + raise TypeError( + "extra_parameters must be a Parameter or a list of Parameters." + ) else: self._extra_parameters = [] @@ -121,7 +125,7 @@ def experiment(self, value: Experiment) -> None: """ if not isinstance(value, Experiment): - raise TypeError('experiment must be an instance of Experiment') + raise TypeError("experiment must be an instance of Experiment") self._experiment = value self._on_experiment_changed() @@ -146,7 +150,7 @@ def sample_model(self, value: SampleModel) -> None: TypeError: if value is not a SampleModel. """ if not isinstance(value, SampleModel): - raise TypeError('sample_model must be an instance of SampleModel') + raise TypeError("sample_model must be an instance of SampleModel") self._sample_model = value self._on_sample_model_changed() @@ -172,7 +176,7 @@ def instrument_model(self, value: InstrumentModel) -> None: TypeError: if value is not an InstrumentModel. """ if not isinstance(value, InstrumentModel): - raise TypeError('instrument_model must be an instance of InstrumentModel') + raise TypeError("instrument_model must be an instance of InstrumentModel") self._instrument_model = value self._on_instrument_model_changed() @@ -199,7 +203,7 @@ def Q(self, value: sc.Variable) -> None: Raises: AttributeError: If trying to set Q. """ - raise AttributeError('Q is a read-only property derived from the Experiment.') + raise AttributeError("Q is a read-only property derived from the Experiment.") @property def energy(self) -> sc.Variable | None: @@ -226,7 +230,9 @@ def energy(self, value: sc.Variable) -> None: AttributeError: If trying to set energy. """ - raise AttributeError('energy is a read-only property derived from the Experiment.') + raise AttributeError( + "energy is a read-only property derived from the Experiment." + ) @property def temperature(self) -> Parameter | None: @@ -253,7 +259,9 @@ def temperature(self, value: np.ndarray | Parameter) -> None: AttributeError: If trying to set temperature. """ - raise AttributeError('temperature is a read-only property derived from the SampleModel.') + raise AttributeError( + "temperature is a read-only property derived from the SampleModel." + ) @property def extra_parameters(self) -> list[Parameter]: @@ -284,7 +292,9 @@ def extra_parameters(self, value: Parameter | list[Parameter]) -> None: elif value is None: self._extra_parameters = [] else: - raise TypeError('extra_parameters must be a Parameter, a list of Parameters, or None.') + raise TypeError( + "extra_parameters must be a Parameter, a list of Parameters, or None." + ) ############# # Other methods @@ -330,7 +340,7 @@ def _verify_Q_index(self, Q_index: int | None) -> int | None: or Q_index < 0 or (self.Q is not None and Q_index >= len(self.Q)) ): - raise IndexError('Q_index must be a valid index for the Q values.') + raise IndexError("Q_index must be a valid index for the Q values.") return Q_index ############# @@ -343,5 +353,5 @@ def __repr__(self) -> str: Returns: str: A string representation of the Analysis. """ - return f' {self.__class__.__name__} (display_name={self.display_name}, \ - unique_name={self.unique_name})' + return f" {self.__class__.__name__} (display_name={self.display_name}, \ + unique_name={self.unique_name})" diff --git a/src/easydynamics/sample_model/instrument_model.py b/src/easydynamics/sample_model/instrument_model.py index c30beeb4..f31fd1f4 100644 --- a/src/easydynamics/sample_model/instrument_model.py +++ b/src/easydynamics/sample_model/instrument_model.py @@ -26,13 +26,13 @@ class InstrumentModel(NewBase): def __init__( self, - display_name: str = 'MyInstrumentModel', + display_name: str = "MyInstrumentModel", unique_name: str | None = None, Q: Q_type | None = None, resolution_model: ResolutionModel | None = None, background_model: BackgroundModel | None = None, energy_offset: Numeric | None = None, - unit: str | sc.Unit = 'meV', + unit: str | sc.Unit = "meV", ) -> None: """Initialize an InstrumentModel. @@ -71,8 +71,8 @@ def __init__( else: if not isinstance(resolution_model, ResolutionModel): raise TypeError( - f'resolution_model must be a ResolutionModel or None, ' - f'got {type(resolution_model).__name__}' + f"resolution_model must be a ResolutionModel or None, " + f"got {type(resolution_model).__name__}" ) self._resolution_model = resolution_model @@ -81,8 +81,8 @@ def __init__( else: if not isinstance(background_model, BackgroundModel): raise TypeError( - f'background_model must be a BackgroundModel or None, ' - f'got {type(background_model).__name__}' + f"background_model must be a BackgroundModel or None, " + f"got {type(background_model).__name__}" ) self._background_model = background_model @@ -90,10 +90,10 @@ def __init__( energy_offset = 0.0 if not isinstance(energy_offset, Numeric): - raise TypeError('energy_offset must be a number or None') + raise TypeError("energy_offset must be a number or None") self._energy_offset = Parameter( - name='energy_offset', + name="energy_offset", value=float(energy_offset), unit=self.unit, fixed=False, @@ -127,7 +127,7 @@ def resolution_model(self, value: ResolutionModel) -> None: """ if not isinstance(value, ResolutionModel): raise TypeError( - f'resolution_model must be a ResolutionModel, got {type(value).__name__}' + f"resolution_model must be a ResolutionModel, got {type(value).__name__}" ) self._resolution_model = value self._on_resolution_model_change() @@ -156,7 +156,7 @@ def background_model(self, value: BackgroundModel) -> None: if not isinstance(value, BackgroundModel): raise TypeError( - f'background_model must be a BackgroundModel, got {type(value).__name__}' + f"background_model must be a BackgroundModel, got {type(value).__name__}" ) self._background_model = value self._on_background_model_change() @@ -198,8 +198,8 @@ def Q(self, value: Q_type | None) -> None: if len(old_Q) != len(new_Q) or not np.allclose(old_Q, new_Q): raise ValueError( - 'New Q values are not similar to the old ones. ' - 'To change Q values, first run clear_Q().' + "New Q values are not similar to the old ones. " + "To change Q values, first run clear_Q()." ) @property @@ -226,8 +226,8 @@ def unit(self, unit_str: str) -> None: AttributeError: Always, as the unit is read-only. """ raise AttributeError( - f'Unit is read-only. Use convert_unit to change the unit between allowed types ' - f'or create a new {self.__class__.__name__} with the desired unit.' + f"Unit is read-only. Use convert_unit to change the unit between allowed types " + f"or create a new {self.__class__.__name__} with the desired unit." ) # noqa: E501 @property @@ -253,7 +253,9 @@ def energy_offset(self, value: Numeric) -> None: TypeError: If value is not a number. """ if not isinstance(value, Numeric): - raise TypeError(f'energy_offset must be a number, got {type(value).__name__}') + raise TypeError( + f"energy_offset must be a number, got {type(value).__name__}" + ) self._energy_offset.value = value self._on_energy_offset_change() @@ -276,7 +278,7 @@ def clear_Q(self, confirm: bool = False) -> None: """ if not confirm: raise ValueError( - 'Clearing Q values requires confirmation. Set confirm=True to proceed.' + "Clearing Q values requires confirmation. Set confirm=True to proceed." ) self._Q = None self.background_model.clear_Q(confirm=True) @@ -295,7 +297,7 @@ def convert_unit(self, unit_str: str | sc.Unit) -> None: """ unit = _validate_unit(unit_str) if unit is None: - raise ValueError('unit_str must be a valid unit string or scipp Unit') + raise ValueError("unit_str must be a valid unit string or scipp Unit") self._background_model.convert_unit(unit) self._resolution_model.convert_unit(unit) @@ -331,10 +333,12 @@ def get_all_variables(self, Q_index: int | None = None) -> list[Parameter]: variables = [self._energy_offsets[i] for i in range(len(self._Q))] else: if not isinstance(Q_index, int): - raise TypeError(f'Q_index must be an int or None, got {type(Q_index).__name__}') + raise TypeError( + f"Q_index must be an int or None, got {type(Q_index).__name__}" + ) if Q_index < 0 or Q_index >= len(self._Q): raise IndexError( - f'Q_index {Q_index} is out of bounds for Q of length {len(self._Q)}' + f"Q_index {Q_index} is out of bounds for Q of length {len(self._Q)}" ) variables = [self._energy_offsets[Q_index]] @@ -351,6 +355,10 @@ def free_resolution_parameters(self) -> None: """Free all parameters in the resolution model.""" self.resolution_model.free_all_parameters() + def normalize_resolution(self) -> None: + """Normalize the resolution model to have area 1.""" + self.resolution_model.normalize_area() + def get_energy_offset_at_Q(self, Q_index: int) -> Parameter: """Get the energy offset Parameter at a specific Q index. @@ -367,10 +375,12 @@ def get_energy_offset_at_Q(self, Q_index: int) -> Parameter: IndexError: If Q_index is out of bounds. """ if self._Q is None: - raise ValueError('No Q values are set in the InstrumentModel.') + raise ValueError("No Q values are set in the InstrumentModel.") if Q_index < 0 or Q_index >= len(self._Q): - raise IndexError(f'Q_index {Q_index} is out of bounds for Q of length {len(self._Q)}') + raise IndexError( + f"Q_index {Q_index} is out of bounds for Q of length {len(self._Q)}" + ) return self._energy_offsets[Q_index] @@ -417,11 +427,11 @@ def __repr__(self) -> str: """ return ( - f'{self.__class__.__name__}(' - f'unique_name={self.unique_name!r}, ' - f'unit={self.unit}, ' - f'Q_len={None if self._Q is None else len(self._Q)}, ' - f'resolution_model={self._resolution_model!r}, ' - f'background_model={self._background_model!r}' - f')' + f"{self.__class__.__name__}(" + f"unique_name={self.unique_name!r}, " + f"unit={self.unit}, " + f"Q_len={None if self._Q is None else len(self._Q)}, " + f"resolution_model={self._resolution_model!r}, " + f"background_model={self._background_model!r}" + f")" ) diff --git a/src/easydynamics/sample_model/model_base.py b/src/easydynamics/sample_model/model_base.py index ef2c6fb1..17d884df 100644 --- a/src/easydynamics/sample_model/model_base.py +++ b/src/easydynamics/sample_model/model_base.py @@ -25,9 +25,9 @@ class ModelBase(EasyScienceModelBase): def __init__( self, - display_name: str = 'MyModelBase', + display_name: str = "MyModelBase", unique_name: str | None = None, - unit: str | sc.Unit | None = 'meV', + unit: str | sc.Unit | None = "meV", components: ModelComponent | ComponentCollection | None = None, Q: Q_type | None = None, ) -> None: @@ -59,8 +59,8 @@ def __init__( components, (ModelComponent, ComponentCollection) ): raise TypeError( - f'Components must be a ModelComponent, a ComponentCollection or None, ' - f'got {type(components).__name__}' + f"Components must be a ModelComponent, a ComponentCollection or None, " + f"got {type(components).__name__}" ) self._components = ComponentCollection() @@ -93,8 +93,8 @@ def evaluate( if not self._component_collections: raise ValueError( - 'No components in the model to evaluate. ' - 'Run generate_component_collections() first' + "No components in the model to evaluate. " + "Run generate_component_collections() first" ) y = [collection.evaluate(x) for collection in self._component_collections] @@ -156,8 +156,8 @@ def unit(self, unit_str: str) -> None: read-only. """ raise AttributeError( - f'Unit is read-only. Use convert_unit to change the unit between allowed types ' - f'or create a new {self.__class__.__name__} with the desired unit.' + f"Unit is read-only. Use convert_unit to change the unit between allowed types " + f"or create a new {self.__class__.__name__} with the desired unit." ) # noqa: E501 def convert_unit(self, unit: str | sc.Unit) -> None: @@ -176,7 +176,9 @@ def convert_unit(self, unit: str | sc.Unit) -> None: old_unit = self._unit if not isinstance(unit, (str, sc.Unit)): - raise TypeError(f'Unit must be a string or sc.Unit, got {type(unit).__name__}') + raise TypeError( + f"Unit must be a string or sc.Unit, got {type(unit).__name__}" + ) try: for component in self.components: component.convert_unit(unit) @@ -213,7 +215,9 @@ def components(self, value: ModelComponent | ComponentCollection | None) -> None ComponentCollection, or None. """ if not isinstance(value, (ModelComponent, ComponentCollection, type(None))): - raise TypeError('Components must be a ModelComponent or a ComponentCollection') + raise TypeError( + "Components must be a ModelComponent or a ComponentCollection" + ) self.clear_components() if value is not None: @@ -255,8 +259,8 @@ def Q(self, value: Q_type | None) -> None: if len(old_Q) != len(new_Q) or not np.allclose(old_Q, new_Q): raise ValueError( - 'New Q values are not similar to the old ones. ' - 'To change Q values, first run clear_Q().' + "New Q values are not similar to the old ones. " + "To change Q values, first run clear_Q()." ) def clear_Q(self, confirm: bool = False) -> None: @@ -271,7 +275,7 @@ def clear_Q(self, confirm: bool = False) -> None: """ if not confirm: raise ValueError( - 'Clearing Q values requires confirmation. Set confirm=True to proceed.' + "Clearing Q values requires confirmation. Set confirm=True to proceed." ) self._Q = None self._on_Q_change() @@ -318,11 +322,13 @@ def get_all_variables(self, Q_index: int | None = None) -> list[Parameter]: ] else: if not isinstance(Q_index, int): - raise TypeError(f'Q_index must be an int or None, got {type(Q_index).__name__}') + raise TypeError( + f"Q_index must be an int or None, got {type(Q_index).__name__}" + ) if Q_index < 0 or Q_index >= len(self._component_collections): raise IndexError( - f'Q_index {Q_index} is out of bounds for component collections ' - f'of length {len(self._component_collections)}' + f"Q_index {Q_index} is out of bounds for component collections " + f"of length {len(self._component_collections)}" ) all_vars = self._component_collections[Q_index].get_all_variables() return all_vars @@ -343,14 +349,19 @@ def get_component_collection(self, Q_index: int) -> ComponentCollection: ComponentCollections. """ if not isinstance(Q_index, int): - raise TypeError(f'Q_index must be an int, got {type(Q_index).__name__}') + raise TypeError(f"Q_index must be an int, got {type(Q_index).__name__}") if Q_index < 0 or Q_index >= len(self._component_collections): raise IndexError( - f'Q_index {Q_index} is out of bounds for component collections ' - f'of length {len(self._component_collections)}' + f"Q_index {Q_index} is out of bounds for component collections " + f"of length {len(self._component_collections)}" ) return self._component_collections[Q_index] + def normalize_area(self) -> None: + """Normalize the area of the model across all Q values.""" + for collection in self._component_collections: + collection.normalize_area() + # ------------------------------------------------------------------ # Private methods # ------------------------------------------------------------------ @@ -385,6 +396,6 @@ def __repr__(self) -> str: str: A string representation of the ModelBase. """ return ( - f'{self.__class__.__name__}(unique_name={self.unique_name}, ' - f'unit={self.unit}), Q = {self.Q}, components = {self.components}' + f"{self.__class__.__name__}(unique_name={self.unique_name}, " + f"unit={self.unit}), Q = {self.Q}, components = {self.components}" ) From 2cbfd4b29ecff65e0296b2b8d5e2d5722a33cc43 Mon Sep 17 00:00:00 2001 From: henrikjacobsenfys Date: Thu, 26 Mar 2026 04:25:57 +0100 Subject: [PATCH 2/4] final touches --- docs/docs/tutorials/tutorial1_brownian.ipynb | 5 +- .../tutorials/tutorial2_nanoparticles.ipynb | 2 + src/easydynamics/analysis/analysis1d.py | 84 ++++++++----------- src/easydynamics/analysis/analysis_base.py | 49 ++++++----- .../sample_model/instrument_model.py | 62 +++++++------- src/easydynamics/sample_model/model_base.py | 48 +++++------ .../analysis/test_analysis_base.py | 15 ++++ 7 files changed, 127 insertions(+), 138 deletions(-) diff --git a/docs/docs/tutorials/tutorial1_brownian.ipynb b/docs/docs/tutorials/tutorial1_brownian.ipynb index 2cdd4276..5562adde 100644 --- a/docs/docs/tutorials/tutorial1_brownian.ipynb +++ b/docs/docs/tutorials/tutorial1_brownian.ipynb @@ -330,7 +330,7 @@ "metadata": {}, "outputs": [], "source": [ - "diffusion_experiment.plot_data(slicer=True,ymax=4)" + "diffusion_experiment.plot_data(slicer=True, ymax=4)" ] }, { @@ -366,7 +366,7 @@ "id": "927b8fb5", "metadata": {}, "source": [ - "We also create a new instrument_model and attach it to our analysis, giving it the resolution model determined in the vanadium analysis. " + "We also create a new instrument_model and attach it to our analysis, giving it the resolution model determined in the vanadium analysis. We further fix all parameters in the resolution model and normalize it." ] }, { @@ -381,6 +381,7 @@ " resolution_model=vanadium_analysis.instrument_model.resolution_model,\n", ")\n", "instrument_model.resolution_model.fix_all_parameters()\n", + "instrument_model.normalize_resolution()\n", "\n", "diffusion_analysis = Analysis(\n", " display_name='Diffusion Analysis',\n", diff --git a/docs/docs/tutorials/tutorial2_nanoparticles.ipynb b/docs/docs/tutorials/tutorial2_nanoparticles.ipynb index 28169b83..e7308dbf 100644 --- a/docs/docs/tutorials/tutorial2_nanoparticles.ipynb +++ b/docs/docs/tutorials/tutorial2_nanoparticles.ipynb @@ -280,6 +280,7 @@ " resolution_model=res_analysis.instrument_model.resolution_model,\n", ")\n", "instrument_model.resolution_model.fix_all_parameters()\n", + "instrument_model.normalize_resolution()\n", "\n", "\n", "analysis = Analysis(\n", @@ -417,6 +418,7 @@ " resolution_model=res_analysis.instrument_model.resolution_model,\n", ")\n", "instrument_model.resolution_model.fix_all_parameters()\n", + "instrument_model.normalize_resolution()\n", "\n", "# Create the analysis object\n", "mag_analysis = Analysis(\n", diff --git a/src/easydynamics/analysis/analysis1d.py b/src/easydynamics/analysis/analysis1d.py index e09c7608..ec6c3c8d 100644 --- a/src/easydynamics/analysis/analysis1d.py +++ b/src/easydynamics/analysis/analysis1d.py @@ -30,7 +30,7 @@ class Analysis1d(AnalysisBase): def __init__( self, - display_name: str | None = "MyAnalysis", + display_name: str | None = 'MyAnalysis', unique_name: str | None = None, experiment: Experiment | None = None, sample_model: SampleModel | None = None, @@ -166,7 +166,7 @@ def fit(self) -> FitResults: Analysis. """ if self._experiment is None: - raise ValueError("No experiment is associated with this Analysis.") + raise ValueError('No experiment is associated with this Analysis.') # Create convolver once to reuse during fitting self._convolver = self._create_convolver() @@ -262,13 +262,13 @@ def plot_data_and_model( import plopp as pp if self.experiment.data is None: - raise ValueError("No data to plot. Please load data first.") + raise ValueError('No data to plot. Please load data first.') energy = self._verify_energy(energy) if energy is None: energy = self._masked_energy - data = self.experiment.data["Q", self.Q_index] + data = self.experiment.data['Q', self.Q_index] model_array = self._create_sample_scipp_array(energy=energy) component_dataset = self._create_components_dataset_single_Q( @@ -277,26 +277,24 @@ def plot_data_and_model( # Create a dataset containing the data, model, and individual # components for plotting. - data_and_model = sc.Dataset( - { - "Data": data, - "Model": model_array, - } - ) + data_and_model = sc.Dataset({ + 'Data': data, + 'Model': model_array, + }) data_and_model = sc.merge(data_and_model, component_dataset) plot_kwargs_defaults = { - "title": self.display_name, - "linestyle": {"Data": "none", "Model": "-"}, - "marker": {"Data": "o", "Model": "none"}, - "color": {"Data": "black", "Model": "red"}, - "markerfacecolor": {"Data": "none", "Model": "none"}, + 'title': self.display_name, + 'linestyle': {'Data': 'none', 'Model': '-'}, + 'marker': {'Data': 'o', 'Model': 'none'}, + 'color': {'Data': 'black', 'Model': 'red'}, + 'markerfacecolor': {'Data': 'none', 'Model': 'none'}, } if plot_components: for comp_name in component_dataset: - plot_kwargs_defaults["linestyle"][comp_name] = "--" - plot_kwargs_defaults["marker"][comp_name] = None + plot_kwargs_defaults['linestyle'][comp_name] = '--' + plot_kwargs_defaults['marker'][comp_name] = None # Overwrite defaults with any user-provided kwargs plot_kwargs_defaults.update(kwargs) @@ -322,7 +320,7 @@ def _require_Q_index(self) -> int: ValueError: If the Q index is not set. """ if self._Q_index is None: - raise ValueError("Q_index must be set.") + raise ValueError('Q_index must be set.') return self._Q_index def _on_Q_index_changed(self) -> None: @@ -351,9 +349,7 @@ def _verify_energy(self, energy: sc.Variable | None) -> sc.Variable | None: """ if energy is not None and not isinstance(energy, sc.Variable): - raise TypeError( - f"Energy must be a sc.Variable or None. Got {type(energy)}." - ) + raise TypeError(f'Energy must be a sc.Variable or None. Got {type(energy)}.') return energy def _calculate_energy_with_offset( @@ -380,8 +376,8 @@ def _calculate_energy_with_offset( energy_offset.convert_unit(str(energy.unit)) except Exception as e: raise sc.UnitError( - f"Energy and energy offset must have compatible units. " - f"Got {energy.unit} and {energy_offset.unit}." + f'Energy and energy offset must have compatible units. ' + f'Got {energy.unit} and {energy_offset.unit}.' ) from e energy_with_offset = energy.copy(deep=True) @@ -457,9 +453,7 @@ def _evaluate_components( # performance is not important. # We don't create a convolver if the resolution is empty. - resolution = self.instrument_model.resolution_model.get_component_collection( - Q_index - ) + resolution = self.instrument_model.resolution_model.get_component_collection(Q_index) if resolution.is_empty: return components.evaluate(energy_with_offset) @@ -533,10 +527,8 @@ def _evaluate_background(self, energy: sc.Variable | None = None) -> np.ndarray: np.ndarray: The evaluated background contribution. """ Q_index = self._require_Q_index() - background_components = ( - self.instrument_model.background_model.get_component_collection( - Q_index=Q_index - ) + background_components = self.instrument_model.background_model.get_component_collection( + Q_index=Q_index ) return self._evaluate_components( components=background_components, @@ -597,8 +589,8 @@ def _create_convolver( if sample_components.is_empty: return None - resolution_components = ( - self.instrument_model.resolution_model.get_component_collection(Q_index) + resolution_components = self.instrument_model.resolution_model.get_component_collection( + Q_index ) if resolution_components.is_empty: return None @@ -666,9 +658,7 @@ def _create_background_component_scipp_array( ) return self._to_scipp_array(values=values, energy=energy) - def _create_sample_scipp_array( - self, energy: sc.Variable | None = None - ) -> sc.DataArray: + def _create_sample_scipp_array(self, energy: sc.Variable | None = None) -> sc.DataArray: """Create a scipp DataArray for the full sample model including background. @@ -707,28 +697,22 @@ def _create_components_dataset_single_Q( Q_index=self.Q_index ).components - background_components = ( - self.instrument_model.background_model.get_component_collection( - Q_index=self.Q_index - ).components - ) + background_components = self.instrument_model.background_model.get_component_collection( + Q_index=self.Q_index + ).components if energy is None: energy = self._masked_energy - background = ( - self._evaluate_background(energy=energy) if add_background else None - ) + background = self._evaluate_background(energy=energy) if add_background else None for component in sample_components: scipp_arrays[component.display_name] = self._create_component_scipp_array( component=component, background=background, energy=energy ) for component in background_components: - scipp_arrays[component.display_name] = ( - self._create_background_component_scipp_array( - component=component, energy=energy - ) + scipp_arrays[component.display_name] = self._create_background_component_scipp_array( + component=component, energy=energy ) return sc.Dataset(scipp_arrays) @@ -753,9 +737,9 @@ 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), coords={ - "energy": energy, - "Q": self.Q[self.Q_index], + 'energy': energy, + 'Q': self.Q[self.Q_index], }, ) diff --git a/src/easydynamics/analysis/analysis_base.py b/src/easydynamics/analysis/analysis_base.py index f3c101b7..58f5d7a6 100644 --- a/src/easydynamics/analysis/analysis_base.py +++ b/src/easydynamics/analysis/analysis_base.py @@ -24,7 +24,7 @@ class AnalysisBase(EasyScienceModelBase): def __init__( self, - display_name: str | None = "MyAnalysis", + display_name: str | None = 'MyAnalysis', unique_name: str | None = None, experiment: Experiment | None = None, sample_model: SampleModel | None = None, @@ -65,23 +65,21 @@ def __init__( elif isinstance(experiment, Experiment): self._experiment = experiment else: - raise TypeError("experiment must be an instance of Experiment or None.") + raise TypeError('experiment must be an instance of Experiment or None.') if sample_model is None: self._sample_model = SampleModel() elif isinstance(sample_model, SampleModel): self._sample_model = sample_model else: - raise TypeError("sample_model must be an instance of SampleModel or None.") + raise TypeError('sample_model must be an instance of SampleModel or None.') if instrument_model is None: self._instrument_model = InstrumentModel() elif isinstance(instrument_model, InstrumentModel): self._instrument_model = instrument_model else: - raise TypeError( - "instrument_model must be an instance of InstrumentModel or None." - ) + raise TypeError('instrument_model must be an instance of InstrumentModel or None.') if extra_parameters is not None: if isinstance(extra_parameters, Parameter): @@ -91,9 +89,7 @@ def __init__( ): self._extra_parameters = extra_parameters else: - raise TypeError( - "extra_parameters must be a Parameter or a list of Parameters." - ) + raise TypeError('extra_parameters must be a Parameter or a list of Parameters.') else: self._extra_parameters = [] @@ -125,7 +121,7 @@ def experiment(self, value: Experiment) -> None: """ if not isinstance(value, Experiment): - raise TypeError("experiment must be an instance of Experiment") + raise TypeError('experiment must be an instance of Experiment') self._experiment = value self._on_experiment_changed() @@ -150,7 +146,7 @@ def sample_model(self, value: SampleModel) -> None: TypeError: if value is not a SampleModel. """ if not isinstance(value, SampleModel): - raise TypeError("sample_model must be an instance of SampleModel") + raise TypeError('sample_model must be an instance of SampleModel') self._sample_model = value self._on_sample_model_changed() @@ -176,7 +172,7 @@ def instrument_model(self, value: InstrumentModel) -> None: TypeError: if value is not an InstrumentModel. """ if not isinstance(value, InstrumentModel): - raise TypeError("instrument_model must be an instance of InstrumentModel") + raise TypeError('instrument_model must be an instance of InstrumentModel') self._instrument_model = value self._on_instrument_model_changed() @@ -203,7 +199,7 @@ def Q(self, value: sc.Variable) -> None: Raises: AttributeError: If trying to set Q. """ - raise AttributeError("Q is a read-only property derived from the Experiment.") + raise AttributeError('Q is a read-only property derived from the Experiment.') @property def energy(self) -> sc.Variable | None: @@ -230,9 +226,7 @@ def energy(self, value: sc.Variable) -> None: AttributeError: If trying to set energy. """ - raise AttributeError( - "energy is a read-only property derived from the Experiment." - ) + raise AttributeError('energy is a read-only property derived from the Experiment.') @property def temperature(self) -> Parameter | None: @@ -259,9 +253,7 @@ def temperature(self, value: np.ndarray | Parameter) -> None: AttributeError: If trying to set temperature. """ - raise AttributeError( - "temperature is a read-only property derived from the SampleModel." - ) + raise AttributeError('temperature is a read-only property derived from the SampleModel.') @property def extra_parameters(self) -> list[Parameter]: @@ -292,14 +284,21 @@ def extra_parameters(self, value: Parameter | list[Parameter]) -> None: elif value is None: self._extra_parameters = [] else: - raise TypeError( - "extra_parameters must be a Parameter, a list of Parameters, or None." - ) + raise TypeError('extra_parameters must be a Parameter, a list of Parameters, or None.') ############# # Other methods ############# + def normalize_resolution(self) -> None: + """Normalize the resolution in the InstrumentModel to ensure + that it integrates to 1. + + This is important for accurate fitting and interpretation of the + results. + """ + self.instrument_model.normalize_resolution() + ############# # Private methods ############# @@ -340,7 +339,7 @@ def _verify_Q_index(self, Q_index: int | None) -> int | None: or Q_index < 0 or (self.Q is not None and Q_index >= len(self.Q)) ): - raise IndexError("Q_index must be a valid index for the Q values.") + raise IndexError('Q_index must be a valid index for the Q values.') return Q_index ############# @@ -353,5 +352,5 @@ def __repr__(self) -> str: Returns: str: A string representation of the Analysis. """ - return f" {self.__class__.__name__} (display_name={self.display_name}, \ - unique_name={self.unique_name})" + return f' {self.__class__.__name__} (display_name={self.display_name}, \ + unique_name={self.unique_name})' diff --git a/src/easydynamics/sample_model/instrument_model.py b/src/easydynamics/sample_model/instrument_model.py index f31fd1f4..3023e43a 100644 --- a/src/easydynamics/sample_model/instrument_model.py +++ b/src/easydynamics/sample_model/instrument_model.py @@ -26,13 +26,13 @@ class InstrumentModel(NewBase): def __init__( self, - display_name: str = "MyInstrumentModel", + display_name: str = 'MyInstrumentModel', unique_name: str | None = None, Q: Q_type | None = None, resolution_model: ResolutionModel | None = None, background_model: BackgroundModel | None = None, energy_offset: Numeric | None = None, - unit: str | sc.Unit = "meV", + unit: str | sc.Unit = 'meV', ) -> None: """Initialize an InstrumentModel. @@ -71,8 +71,8 @@ def __init__( else: if not isinstance(resolution_model, ResolutionModel): raise TypeError( - f"resolution_model must be a ResolutionModel or None, " - f"got {type(resolution_model).__name__}" + f'resolution_model must be a ResolutionModel or None, ' + f'got {type(resolution_model).__name__}' ) self._resolution_model = resolution_model @@ -81,8 +81,8 @@ def __init__( else: if not isinstance(background_model, BackgroundModel): raise TypeError( - f"background_model must be a BackgroundModel or None, " - f"got {type(background_model).__name__}" + f'background_model must be a BackgroundModel or None, ' + f'got {type(background_model).__name__}' ) self._background_model = background_model @@ -90,10 +90,10 @@ def __init__( energy_offset = 0.0 if not isinstance(energy_offset, Numeric): - raise TypeError("energy_offset must be a number or None") + raise TypeError('energy_offset must be a number or None') self._energy_offset = Parameter( - name="energy_offset", + name='energy_offset', value=float(energy_offset), unit=self.unit, fixed=False, @@ -127,7 +127,7 @@ def resolution_model(self, value: ResolutionModel) -> None: """ if not isinstance(value, ResolutionModel): raise TypeError( - f"resolution_model must be a ResolutionModel, got {type(value).__name__}" + f'resolution_model must be a ResolutionModel, got {type(value).__name__}' ) self._resolution_model = value self._on_resolution_model_change() @@ -156,7 +156,7 @@ def background_model(self, value: BackgroundModel) -> None: if not isinstance(value, BackgroundModel): raise TypeError( - f"background_model must be a BackgroundModel, got {type(value).__name__}" + f'background_model must be a BackgroundModel, got {type(value).__name__}' ) self._background_model = value self._on_background_model_change() @@ -198,8 +198,8 @@ def Q(self, value: Q_type | None) -> None: if len(old_Q) != len(new_Q) or not np.allclose(old_Q, new_Q): raise ValueError( - "New Q values are not similar to the old ones. " - "To change Q values, first run clear_Q()." + 'New Q values are not similar to the old ones. ' + 'To change Q values, first run clear_Q().' ) @property @@ -226,8 +226,8 @@ def unit(self, unit_str: str) -> None: AttributeError: Always, as the unit is read-only. """ raise AttributeError( - f"Unit is read-only. Use convert_unit to change the unit between allowed types " - f"or create a new {self.__class__.__name__} with the desired unit." + f'Unit is read-only. Use convert_unit to change the unit between allowed types ' + f'or create a new {self.__class__.__name__} with the desired unit.' ) # noqa: E501 @property @@ -253,9 +253,7 @@ def energy_offset(self, value: Numeric) -> None: TypeError: If value is not a number. """ if not isinstance(value, Numeric): - raise TypeError( - f"energy_offset must be a number, got {type(value).__name__}" - ) + raise TypeError(f'energy_offset must be a number, got {type(value).__name__}') self._energy_offset.value = value self._on_energy_offset_change() @@ -278,7 +276,7 @@ def clear_Q(self, confirm: bool = False) -> None: """ if not confirm: raise ValueError( - "Clearing Q values requires confirmation. Set confirm=True to proceed." + 'Clearing Q values requires confirmation. Set confirm=True to proceed.' ) self._Q = None self.background_model.clear_Q(confirm=True) @@ -297,7 +295,7 @@ def convert_unit(self, unit_str: str | sc.Unit) -> None: """ unit = _validate_unit(unit_str) if unit is None: - raise ValueError("unit_str must be a valid unit string or scipp Unit") + raise ValueError('unit_str must be a valid unit string or scipp Unit') self._background_model.convert_unit(unit) self._resolution_model.convert_unit(unit) @@ -333,12 +331,10 @@ def get_all_variables(self, Q_index: int | None = None) -> list[Parameter]: variables = [self._energy_offsets[i] for i in range(len(self._Q))] else: if not isinstance(Q_index, int): - raise TypeError( - f"Q_index must be an int or None, got {type(Q_index).__name__}" - ) + raise TypeError(f'Q_index must be an int or None, got {type(Q_index).__name__}') if Q_index < 0 or Q_index >= len(self._Q): raise IndexError( - f"Q_index {Q_index} is out of bounds for Q of length {len(self._Q)}" + f'Q_index {Q_index} is out of bounds for Q of length {len(self._Q)}' ) variables = [self._energy_offsets[Q_index]] @@ -375,12 +371,10 @@ def get_energy_offset_at_Q(self, Q_index: int) -> Parameter: IndexError: If Q_index is out of bounds. """ if self._Q is None: - raise ValueError("No Q values are set in the InstrumentModel.") + raise ValueError('No Q values are set in the InstrumentModel.') if Q_index < 0 or Q_index >= len(self._Q): - raise IndexError( - f"Q_index {Q_index} is out of bounds for Q of length {len(self._Q)}" - ) + raise IndexError(f'Q_index {Q_index} is out of bounds for Q of length {len(self._Q)}') return self._energy_offsets[Q_index] @@ -427,11 +421,11 @@ def __repr__(self) -> str: """ return ( - f"{self.__class__.__name__}(" - f"unique_name={self.unique_name!r}, " - f"unit={self.unit}, " - f"Q_len={None if self._Q is None else len(self._Q)}, " - f"resolution_model={self._resolution_model!r}, " - f"background_model={self._background_model!r}" - f")" + f'{self.__class__.__name__}(' + f'unique_name={self.unique_name!r}, ' + f'unit={self.unit}, ' + f'Q_len={None if self._Q is None else len(self._Q)}, ' + f'resolution_model={self._resolution_model!r}, ' + f'background_model={self._background_model!r}' + f')' ) diff --git a/src/easydynamics/sample_model/model_base.py b/src/easydynamics/sample_model/model_base.py index 17d884df..6c1392ea 100644 --- a/src/easydynamics/sample_model/model_base.py +++ b/src/easydynamics/sample_model/model_base.py @@ -25,9 +25,9 @@ class ModelBase(EasyScienceModelBase): def __init__( self, - display_name: str = "MyModelBase", + display_name: str = 'MyModelBase', unique_name: str | None = None, - unit: str | sc.Unit | None = "meV", + unit: str | sc.Unit | None = 'meV', components: ModelComponent | ComponentCollection | None = None, Q: Q_type | None = None, ) -> None: @@ -59,8 +59,8 @@ def __init__( components, (ModelComponent, ComponentCollection) ): raise TypeError( - f"Components must be a ModelComponent, a ComponentCollection or None, " - f"got {type(components).__name__}" + f'Components must be a ModelComponent, a ComponentCollection or None, ' + f'got {type(components).__name__}' ) self._components = ComponentCollection() @@ -93,8 +93,8 @@ def evaluate( if not self._component_collections: raise ValueError( - "No components in the model to evaluate. " - "Run generate_component_collections() first" + 'No components in the model to evaluate. ' + 'Run generate_component_collections() first' ) y = [collection.evaluate(x) for collection in self._component_collections] @@ -156,8 +156,8 @@ def unit(self, unit_str: str) -> None: read-only. """ raise AttributeError( - f"Unit is read-only. Use convert_unit to change the unit between allowed types " - f"or create a new {self.__class__.__name__} with the desired unit." + f'Unit is read-only. Use convert_unit to change the unit between allowed types ' + f'or create a new {self.__class__.__name__} with the desired unit.' ) # noqa: E501 def convert_unit(self, unit: str | sc.Unit) -> None: @@ -176,9 +176,7 @@ def convert_unit(self, unit: str | sc.Unit) -> None: old_unit = self._unit if not isinstance(unit, (str, sc.Unit)): - raise TypeError( - f"Unit must be a string or sc.Unit, got {type(unit).__name__}" - ) + raise TypeError(f'Unit must be a string or sc.Unit, got {type(unit).__name__}') try: for component in self.components: component.convert_unit(unit) @@ -215,9 +213,7 @@ def components(self, value: ModelComponent | ComponentCollection | None) -> None ComponentCollection, or None. """ if not isinstance(value, (ModelComponent, ComponentCollection, type(None))): - raise TypeError( - "Components must be a ModelComponent or a ComponentCollection" - ) + raise TypeError('Components must be a ModelComponent or a ComponentCollection') self.clear_components() if value is not None: @@ -259,8 +255,8 @@ def Q(self, value: Q_type | None) -> None: if len(old_Q) != len(new_Q) or not np.allclose(old_Q, new_Q): raise ValueError( - "New Q values are not similar to the old ones. " - "To change Q values, first run clear_Q()." + 'New Q values are not similar to the old ones. ' + 'To change Q values, first run clear_Q().' ) def clear_Q(self, confirm: bool = False) -> None: @@ -275,7 +271,7 @@ def clear_Q(self, confirm: bool = False) -> None: """ if not confirm: raise ValueError( - "Clearing Q values requires confirmation. Set confirm=True to proceed." + 'Clearing Q values requires confirmation. Set confirm=True to proceed.' ) self._Q = None self._on_Q_change() @@ -322,13 +318,11 @@ def get_all_variables(self, Q_index: int | None = None) -> list[Parameter]: ] else: if not isinstance(Q_index, int): - raise TypeError( - f"Q_index must be an int or None, got {type(Q_index).__name__}" - ) + raise TypeError(f'Q_index must be an int or None, got {type(Q_index).__name__}') if Q_index < 0 or Q_index >= len(self._component_collections): raise IndexError( - f"Q_index {Q_index} is out of bounds for component collections " - f"of length {len(self._component_collections)}" + f'Q_index {Q_index} is out of bounds for component collections ' + f'of length {len(self._component_collections)}' ) all_vars = self._component_collections[Q_index].get_all_variables() return all_vars @@ -349,11 +343,11 @@ def get_component_collection(self, Q_index: int) -> ComponentCollection: ComponentCollections. """ if not isinstance(Q_index, int): - raise TypeError(f"Q_index must be an int, got {type(Q_index).__name__}") + raise TypeError(f'Q_index must be an int, got {type(Q_index).__name__}') if Q_index < 0 or Q_index >= len(self._component_collections): raise IndexError( - f"Q_index {Q_index} is out of bounds for component collections " - f"of length {len(self._component_collections)}" + f'Q_index {Q_index} is out of bounds for component collections ' + f'of length {len(self._component_collections)}' ) return self._component_collections[Q_index] @@ -396,6 +390,6 @@ def __repr__(self) -> str: str: A string representation of the ModelBase. """ return ( - f"{self.__class__.__name__}(unique_name={self.unique_name}, " - f"unit={self.unit}), Q = {self.Q}, components = {self.components}" + f'{self.__class__.__name__}(unique_name={self.unique_name}, ' + f'unit={self.unit}), Q = {self.Q}, components = {self.components}' ) diff --git a/tests/unit/easydynamics/analysis/test_analysis_base.py b/tests/unit/easydynamics/analysis/test_analysis_base.py index 035a65b2..0869ab80 100644 --- a/tests/unit/easydynamics/analysis/test_analysis_base.py +++ b/tests/unit/easydynamics/analysis/test_analysis_base.py @@ -265,6 +265,21 @@ def test_extra_parameters_setter_invalid_type(self, analysis_base, invalid_extra ): analysis_base.extra_parameters = invalid_extra_parameters + ############# + # Other methods + ############# + + def test_normalize_resolution_calls_instrument_model(self, analysis_base): + with patch.object( + analysis_base.instrument_model, 'normalize_resolution' + ) as mock_normalize_resolution: + analysis_base.normalize_resolution() + mock_normalize_resolution.assert_called_once() + + ############# + # Private methods + ############# + def test_on_experiment_changed_updates_Q(self, analysis_base): # WHEN fake_Q = [1, 2, 3] From cb3db0e06e57e0ae7c84d8bc867a8b282e9e8efc Mon Sep 17 00:00:00 2001 From: henrikjacobsenfys Date: Thu, 26 Mar 2026 04:38:45 +0100 Subject: [PATCH 3/4] add missing tests --- .../sample_model/test_instrument_model.py | 10 ++++++++++ .../unit/easydynamics/sample_model/test_model_base.py | 11 +++++++++++ 2 files changed, 21 insertions(+) diff --git a/tests/unit/easydynamics/sample_model/test_instrument_model.py b/tests/unit/easydynamics/sample_model/test_instrument_model.py index 634dea8a..cdf5b48b 100644 --- a/tests/unit/easydynamics/sample_model/test_instrument_model.py +++ b/tests/unit/easydynamics/sample_model/test_instrument_model.py @@ -351,6 +351,16 @@ def test_get_all_variables_with_nonint_Q_index_raises(self, instrument_model): ): instrument_model.get_all_variables(Q_index='invalid_index') + def test_normalize_resolution(self, instrument_model): + # WHEN + instrument_model.resolution_model.normalize_area = MagicMock() + + # THEN + instrument_model.normalize_resolution() + + # EXPECT + instrument_model.resolution_model.normalize_area.assert_called_once() + def test_generate_energy_offsets_Q_none(self, instrument_model): # WHEN instrument_model._Q = None diff --git a/tests/unit/easydynamics/sample_model/test_model_base.py b/tests/unit/easydynamics/sample_model/test_model_base.py index d1ade1e3..0d312fcd 100644 --- a/tests/unit/easydynamics/sample_model/test_model_base.py +++ b/tests/unit/easydynamics/sample_model/test_model_base.py @@ -360,6 +360,17 @@ def test_clear_Q_raises_without_confirm(self, model_base): with pytest.raises(ValueError, match='Clearing Q values requires confirmation'): model_base.clear_Q() + def test_normalize_area(self, model_base): + # WHEN + + # THEN + model_base.normalize_area() + + # EXPECT + for collection in model_base._component_collections: + total_area = sum(component.area.value for component in collection.components) + assert total_area == 1.0 + def test_repr(self, model_base): # WHEN repr_str = repr(model_base) From 08cc148e034225df39978c9558d68fb94086d975 Mon Sep 17 00:00:00 2001 From: henrikjacobsenfys Date: Thu, 26 Mar 2026 09:15:06 +0100 Subject: [PATCH 4/4] minor fix --- tests/unit/easydynamics/sample_model/test_model_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/easydynamics/sample_model/test_model_base.py b/tests/unit/easydynamics/sample_model/test_model_base.py index 0d312fcd..5bb47613 100644 --- a/tests/unit/easydynamics/sample_model/test_model_base.py +++ b/tests/unit/easydynamics/sample_model/test_model_base.py @@ -369,7 +369,7 @@ def test_normalize_area(self, model_base): # EXPECT for collection in model_base._component_collections: total_area = sum(component.area.value for component in collection.components) - assert total_area == 1.0 + assert total_area == pytest.approx(1.0) def test_repr(self, model_base): # WHEN