From 32acbe825ed442e5dde8410b55a80a818d4a9120 Mon Sep 17 00:00:00 2001 From: henrikjacobsenfys Date: Wed, 4 Feb 2026 20:22:28 +0100 Subject: [PATCH 01/12] jump diffusion model --- pixi.lock | 2 +- .../sample_model/diffusion_model/__init__.py | 4 +- .../brownian_translational_diffusion.py | 26 +- .../diffusion_model/diffusion_model_base.py | 34 +- .../jump_translational_diffusion.py | 339 ++++++++++++++++++ src/easydynamics/sample_model/sample_model.py | 2 +- .../test_brownian_translational_diffusion.py | 38 -- .../test_jump_translational_diffusion.py | 254 +++++++++++++ 8 files changed, 627 insertions(+), 72 deletions(-) create mode 100644 src/easydynamics/sample_model/diffusion_model/jump_translational_diffusion.py create mode 100644 tests/unit/easydynamics/sample_model/diffusion_model/test_jump_translational_diffusion.py diff --git a/pixi.lock b/pixi.lock index d9461637..da8aee45 100644 --- a/pixi.lock +++ b/pixi.lock @@ -4091,7 +4091,7 @@ packages: requires_python: '>=3.5' - pypi: ./ name: easydynamics - version: 0.1.0+devdirty7 + version: 0.1.1+devdirty2 sha256: de299c914d4a865b9e2fdefa5e3947f37b1f26f73ff9087f7918ee417f3dd288 requires_dist: - darkdetect diff --git a/src/easydynamics/sample_model/diffusion_model/__init__.py b/src/easydynamics/sample_model/diffusion_model/__init__.py index 6fd920dc..dc0a469c 100644 --- a/src/easydynamics/sample_model/diffusion_model/__init__.py +++ b/src/easydynamics/sample_model/diffusion_model/__init__.py @@ -2,9 +2,9 @@ # SPDX-License-Identifier: BSD-3-Clause from .brownian_translational_diffusion import BrownianTranslationalDiffusion -from .diffusion_model_base import DiffusionModelBase +from .jump_translational_diffusion import JumpTranslationalDiffusion __all__ = [ - 'DiffusionModelBase', 'BrownianTranslationalDiffusion', + 'JumpTranslationalDiffusion', ] diff --git a/src/easydynamics/sample_model/diffusion_model/brownian_translational_diffusion.py b/src/easydynamics/sample_model/diffusion_model/brownian_translational_diffusion.py index 749f8de4..10f43c43 100644 --- a/src/easydynamics/sample_model/diffusion_model/brownian_translational_diffusion.py +++ b/src/easydynamics/sample_model/diffusion_model/brownian_translational_diffusion.py @@ -3,24 +3,20 @@ from typing import Dict from typing import List -from typing import Union import numpy as np import scipp as sc from easyscience.variable import DescriptorNumber from easyscience.variable import Parameter -from numpy.typing import ArrayLike from scipp.constants import hbar as scipp_hbar from easydynamics.sample_model.component_collection import ComponentCollection from easydynamics.sample_model.components import Lorentzian from easydynamics.sample_model.diffusion_model.diffusion_model_base import DiffusionModelBase +from easydynamics.utils.utils import Numeric +from easydynamics.utils.utils import Q_type from easydynamics.utils.utils import _validate_and_convert_Q -Numeric = Union[float, int] - -Q_type = np.ndarray | Numeric | list | ArrayLike - class BrownianTranslationalDiffusion(DiffusionModelBase): """Model of Brownian translational diffusion, consisting of a @@ -46,7 +42,6 @@ def __init__( unit: str | sc.Unit = 'meV', scale: Numeric = 1.0, diffusion_coefficient: Numeric = 1.0, - diffusion_unit: str = 'm**2/s', ): """Initialize a new BrownianTranslationalDiffusion model. @@ -66,9 +61,6 @@ def __init__( Diffusion coefficient D. If a number is provided, it is assumed to be in the unit given by diffusion_unit. Defaults to 1.0. - diffusion_unit : str, optional - Unit for the diffusion coefficient D. Default is m**2/s. - Options are 'meV*Å**2' or 'm**2/s' """ if not isinstance(scale, (Parameter, Numeric)): raise TypeError('scale must be a number.') @@ -76,30 +68,20 @@ def __init__( if not isinstance(diffusion_coefficient, (Parameter, Numeric)): raise TypeError('diffusion_coefficient must be a number.') - if not isinstance(diffusion_unit, str): - raise TypeError("diffusion_unit must be 'meV*Å**2' or 'm**2/s'.") - - if diffusion_unit == 'meV*Å**2' or diffusion_unit == 'meV*angstrom**2': - # In this case, hbar is absorbed in the unit of D - self._hbar = DescriptorNumber('hbar', 1.0) - elif diffusion_unit == 'm**2/s' or diffusion_unit == 'm^2/s': - self._hbar = DescriptorNumber.from_scipp('hbar', scipp_hbar) - else: - raise ValueError("diffusion_unit must be 'meV*Å**2' or 'm**2/s'.") - scale = Parameter(name='scale', value=float(scale), fixed=False, min=0.0) diffusion_coefficient = Parameter( name='diffusion_coefficient', value=float(diffusion_coefficient), fixed=False, - unit=diffusion_unit, + unit='m**2/s', ) super().__init__( display_name=display_name, unique_name=unique_name, unit=unit, ) + self._hbar = DescriptorNumber.from_scipp('hbar', scipp_hbar) self._angstrom = DescriptorNumber('angstrom', 1e-10, unit='m') self._scale = scale self._diffusion_coefficient = diffusion_coefficient diff --git a/src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py b/src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py index 18b5bce8..3694f7af 100644 --- a/src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py +++ b/src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py @@ -1,17 +1,11 @@ -# SPDX-FileCopyrightText: 2025-2026 EasyDynamics contributors -# SPDX-License-Identifier: BSD-3-Clause - -import numpy as np import scipp as sc from easyscience.base_classes.model_base import ModelBase from easyscience.variable import DescriptorNumber -from numpy.typing import ArrayLike +from easyscience.variable import Parameter from scipp import UnitError from easydynamics.utils.utils import Numeric -Q_type = np.ndarray | Numeric | list | ArrayLike - class DiffusionModelBase(ModelBase): """Base class for constructing diffusion models.""" @@ -20,6 +14,7 @@ def __init__( self, display_name='MyDiffusionModel', unique_name: str | None = None, + scale: Numeric = 1.0, unit: str | sc.Unit = 'meV', ): """Initialize a new DiffusionModel. @@ -31,17 +26,22 @@ def __init__( unit : str or sc.Unit, optional Unit of the diffusion model. Defaults to "meV". """ + if not isinstance(scale, Numeric): + raise TypeError('scale must be a number.') + + scale = Parameter(name='scale', value=float(scale), fixed=False, min=0.0) try: test = DescriptorNumber(name='test', value=1, unit=unit) 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 Unitand convertible to meV.' ) from e super().__init__(display_name=display_name, unique_name=unique_name) self._unit = unit + self._scale = scale @property def unit(self) -> str: @@ -62,6 +62,24 @@ def unit(self, unit_str: str) -> None: ) ) # noqa: E501 + @property + def scale(self) -> Parameter: + """Get the scale parameter of the diffusion model. + + Returns + ------- + Parameter + Scale parameter. + """ + return self._scale + + @scale.setter + def scale(self, scale: Numeric) -> None: + """Set the scale parameter of the diffusion model.""" + if not isinstance(scale, Numeric): + raise TypeError('scale must be a number.') + self._scale.value = scale + def __repr__(self): """String representation of the Diffusion model.""" return f'{self.__class__.__name__}(display_name={self.display_name}, unit={self.unit})' diff --git a/src/easydynamics/sample_model/diffusion_model/jump_translational_diffusion.py b/src/easydynamics/sample_model/diffusion_model/jump_translational_diffusion.py new file mode 100644 index 00000000..233a7784 --- /dev/null +++ b/src/easydynamics/sample_model/diffusion_model/jump_translational_diffusion.py @@ -0,0 +1,339 @@ +from typing import Dict +from typing import List + +import numpy as np +import scipp as sc +from easyscience.variable import DescriptorNumber +from easyscience.variable import Parameter +from scipp.constants import hbar as scipp_hbar + +from easydynamics.sample_model.component_collection import ComponentCollection +from easydynamics.sample_model.components import Lorentzian +from easydynamics.sample_model.diffusion_model.diffusion_model_base import DiffusionModelBase +from easydynamics.utils.utils import Numeric +from easydynamics.utils.utils import Q_type +from easydynamics.utils.utils import _validate_and_convert_Q + + +class JumpTranslationalDiffusion(DiffusionModelBase): + """Model of Jump translational diffusion, consisting of a Lorentzian + function for each Q-value, where the width is given by :math:`D + Q^2/(1+D t Q^2)`. Q is assumed to have units of 1/angstrom. Creates + ComponentCollections with Lorentzian components for given Q-values. + + Example usage: Q=np.linspace(0.5,2,7) energy=np.linspace(-2, 2, 501) + scale=1.0 diffusion_coefficient = 2.4e-9 # m^2/s + diffusion_model=JumpTranslationalDiffusion(display_name="DiffusionModel", + scale=scale, diffusion_coefficient= diffusion_coefficient) + component_collections=diffusion_model.create_component_collections(Q) + See also the examples. + """ + + def __init__( + self, + display_name: str | None = 'JumpTranslationalDiffusion', + unique_name: str | None = None, + unit: str | sc.Unit = 'meV', + scale: Numeric = 1.0, + diffusion_coefficient: Numeric = 1.0, + relaxation_time: Numeric = 1.0, + ): + """Initialize a new JumpTranslationalDiffusion model. + + Parameters + ---------- + display_name : str + Display name of the diffusion model. + unique_name : str or None + Unique name of the diffusion model. If None, a unique name + is automatically generated. + unit : str or sc.Unit, optional + Energy unit for the underlying Lorentzian components. + Defaults to "meV". + scale : float , optional + Scale factor for the diffusion model. + diffusion_coefficient : float , optional + Diffusion coefficient D in m^2/s. Defaults to 1.0. + relaxation_time : float , optional + Relaxation time t in ps. Defaults to 1.0. + """ + super().__init__( + display_name=display_name, + unique_name=unique_name, + unit=unit, + scale=scale, + ) + + if not isinstance(diffusion_coefficient, Numeric): + raise TypeError('diffusion_coefficient must be a number.') + + if not isinstance(relaxation_time, Numeric): + raise TypeError('relaxation_time must be a number.') + + diffusion_coefficient = Parameter( + name='diffusion_coefficient', + value=float(diffusion_coefficient), + fixed=False, + unit='m**2/s', + ) + + relaxation_time = Parameter( + name='relaxation_time', + value=float(relaxation_time), + fixed=False, + unit='ps', + ) + + self._hbar = DescriptorNumber.from_scipp('hbar', scipp_hbar) + self._angstrom = DescriptorNumber('angstrom', 1e-10, unit='m') + self._diffusion_coefficient = diffusion_coefficient + self._relaxation_time = relaxation_time + + ################################ + # Properties + ################################ + + @property + def diffusion_coefficient(self) -> Parameter: + """Get the diffusion coefficient parameter D. + + Returns + ------- + Parameter + Diffusion coefficient D. + """ + return self._diffusion_coefficient + + @diffusion_coefficient.setter + def diffusion_coefficient(self, diffusion_coefficient: Numeric) -> None: + """Set the diffusion coefficient parameter D.""" + if not isinstance(diffusion_coefficient, Numeric): + raise TypeError('diffusion_coefficient must be a number.') + self._diffusion_coefficient.value = diffusion_coefficient + + @property + def relaxation_time(self) -> Parameter: + """Get the relaxation time parameter t. + + Returns + ------- + Parameter + Relaxation time t. + """ + return self._relaxation_time + + @relaxation_time.setter + def relaxation_time(self, relaxation_time: Numeric) -> None: + """Set the relaxation time parameter t.""" + if not isinstance(relaxation_time, Numeric): + raise TypeError('relaxation_time must be a number.') + self._relaxation_time.value = relaxation_time + + ################################ + # Other methods + ################################ + + def calculate_width(self, Q: Q_type) -> np.ndarray: + """Calculate the half-width at half-maximum (HWHM) for the + diffusion model. Equation: :math:`\\Gamma(Q) = \\hbar D Q^2/(1+D + t Q^2)` + + Parameters + ---------- + Q : np.ndarray | Numeric | list | ArrayLike + Scattering vector in 1/angstrom + + Returns + ------- + np.ndarray + HWHM values in the unit of the model (e.g., meV). + """ + + Q = _validate_and_convert_Q(Q) + + unit_conversion_factor_nominator = ( + self._hbar * self.diffusion_coefficient / (self._angstrom**2) + ) + unit_conversion_factor_nominator.convert_unit(self.unit) + + nominator = unit_conversion_factor_nominator.value * Q**2 + + unit_conversion_factor_denominator = ( + self.diffusion_coefficient / self._angstrom**2 * self.relaxation_time + ) + unit_conversion_factor_denominator.convert_unit('dimensionless') + + denominator = 1 + unit_conversion_factor_denominator.value * Q**2 + + width = nominator / denominator + + return width + + def calculate_EISF(self, Q: Q_type) -> np.ndarray: + """Calculate the Elastic Incoherent Structure Factor (EISF). + + Parameters + ---------- + Q : np.ndarray | Numeric | list | ArrayLike + Scattering vector in 1/angstrom + + Returns + ------- + np.ndarray + EISF values (dimensionless). + """ + Q = _validate_and_convert_Q(Q) + EISF = np.zeros_like(Q) + return EISF + + def calculate_QISF(self, Q: Q_type) -> np.ndarray: + """Calculate the Quasi-Elastic Incoherent Structure Factor + (QISF). + + Parameters + ---------- + Q : np.ndarray | Numeric | list | ArrayLike + Scattering vector in 1/angstrom + + Returns + ------- + np.ndarray + QISF values (dimensionless). + """ + + Q = _validate_and_convert_Q(Q) + QISF = np.ones_like(Q) + return QISF + + def create_component_collections( + self, + Q: Q_type, + component_display_name: str = 'Brownian translational diffusion', + ) -> List[ComponentCollection]: + """Create ComponentCollection components for the Brownian. + + translational diffusion model at given Q values. Args: + ---------- Q : Number, list, or np.ndarray + Scattering vector values. + component_display_name : str + Name of the Brownian Diffusion Lorentzian component. + Returns + ------- + List[ComponentCollection] + List of ComponentCollections with Brownian Diffusion + Lorentzian components. + """ + Q = _validate_and_convert_Q(Q) + + if not isinstance(component_display_name, str): + raise TypeError('component_name must be a string.') + + component_collection_list = [None] * len(Q) + # In more complex models, this is used to scale the area of the + # Lorentzians and the delta function. + QISF = self.calculate_QISF(Q) + + # Create a Lorentzian component for each Q-value, with width + # D*Q^2 and area equal to scale. No delta function, as the EISF + # is 0. + for i, Q_value in enumerate(Q): + component_collection_list[i] = ComponentCollection( + display_name=f'{self.display_name}_Q{Q_value:.2f}', unit=self.unit + ) + + lorentzian_component = Lorentzian( + display_name=component_display_name, + unit=self.unit, + ) + + # Make the width dependent on Q + dependency_expression = self._write_width_dependency_expression(Q[i]) + dependency_map = self._write_width_dependency_map_expression() + + lorentzian_component.width.make_dependent_on( + dependency_expression=dependency_expression, + dependency_map=dependency_map, + ) + + # Make the area dependent on Q + area_dependency_map = self._write_area_dependency_map_expression() + lorentzian_component.area.make_dependent_on( + dependency_expression=self._write_area_dependency_expression(QISF[i]), + dependency_map=area_dependency_map, + ) + + # Resolving the dependency can do weird things to the units, + # so we make sure it's correct. + lorentzian_component.width.convert_unit(self.unit) + component_collection_list[i].append_component(lorentzian_component) + + return component_collection_list + + ################################ + # Private methods + ################################ + + def _write_width_dependency_expression(self, Q: float) -> str: + """Write the dependency expression for the width as a function + of Q to make dependent Parameters. + + Parameters + ---------- + Q : float + Scattering vector in 1/angstrom + Returns + ------- + str + Dependency expression for the width. + """ + if not isinstance(Q, (float)): + raise TypeError('Q must be a float.') + + # Q is given as a float, so we need to add the units + return f'hbar * D* {Q} **2*1/(angstrom**2)/(1 + (D * t* {Q} **2/(angstrom**2)))' + + def _write_width_dependency_map_expression(self) -> Dict[str, DescriptorNumber]: + """Write the dependency map expression to make dependent + Parameters. + """ + return { + 'D': self._diffusion_coefficient, + 't': self._relaxation_time, + 'hbar': self._hbar, + 'angstrom': self._angstrom, + } + + def _write_area_dependency_expression(self, QISF: float) -> str: + """Write the dependency expression for the area to make + dependent Parameters. + + Returns + ------- + str + Dependency expression for the area. + """ + if not isinstance(QISF, (float)): + raise TypeError('QISF must be a float.') + + return f'{QISF} * scale' + + def _write_area_dependency_map_expression(self) -> Dict[str, DescriptorNumber]: + """Write the dependency map expression to make dependent + Parameters. + """ + return { + 'scale': self._scale, + } + + ################################ + # dunder methods + ################################ + + def __repr__(self): + """String representation of the JumpTranslationalDiffusion + model. + """ + return ( + f'JumpTranslationalDiffusion(display_name={self.display_name}, ' + f'diffusion_coefficient={self._diffusion_coefficient}, scale={self._scale})' + ) diff --git a/src/easydynamics/sample_model/sample_model.py b/src/easydynamics/sample_model/sample_model.py index af5550a9..d48412f3 100644 --- a/src/easydynamics/sample_model/sample_model.py +++ b/src/easydynamics/sample_model/sample_model.py @@ -5,7 +5,7 @@ import scipp as sc from easyscience.variable import Parameter -from easydynamics.sample_model.diffusion_model import DiffusionModelBase +from easydynamics.sample_model.diffusion_model.diffusion_model_base import DiffusionModelBase from easydynamics.sample_model.model_base import ModelBase from easydynamics.utils import _detailed_balance_factor from easydynamics.utils.utils import Numeric diff --git a/tests/unit/easydynamics/sample_model/diffusion_model/test_brownian_translational_diffusion.py b/tests/unit/easydynamics/sample_model/diffusion_model/test_brownian_translational_diffusion.py index 7476755b..2d28726c 100644 --- a/tests/unit/easydynamics/sample_model/diffusion_model/test_brownian_translational_diffusion.py +++ b/tests/unit/easydynamics/sample_model/diffusion_model/test_brownian_translational_diffusion.py @@ -37,7 +37,6 @@ def test_init_default(self, brownian_diffusion_model): 'unit': 123, 'scale': 1.0, 'diffusion_coefficient': 1.0, - 'diffusion_unit': 'm**2/s', }, UnitError, 'Invalid unit', @@ -47,7 +46,6 @@ def test_init_default(self, brownian_diffusion_model): 'unit': 123, 'scale': 'invalid', 'diffusion_coefficient': 1.0, - 'diffusion_unit': 'm**2/s', }, TypeError, 'scale must be a number', @@ -57,38 +55,16 @@ def test_init_default(self, brownian_diffusion_model): 'unit': 123, 'scale': 1.0, 'diffusion_coefficient': 'invalid', - 'diffusion_unit': 'm**2/s', }, TypeError, 'diffusion_coefficient must be a number', ), - ( - { - 'unit': 123, - 'scale': 1.0, - 'diffusion_coefficient': 1.0, - 'diffusion_unit': 123, - }, - TypeError, - 'diffusion_unit must be ', - ), ], ) def test_input_type_validation_raises(self, kwargs, expected_exception, expected_message): with pytest.raises(expected_exception, match=expected_message): BrownianTranslationalDiffusion(display_name='BrownianTranslationalDiffusion', **kwargs) - def test_diffusion_unit_value_error(self): - # WHEN THEN EXPECT - with pytest.raises(ValueError, match='diffusion_unit must be .'): - BrownianTranslationalDiffusion( - display_name='BrownianTranslationalDiffusion', - unit='meV', - scale=1.0, - diffusion_coefficient=1.0, - diffusion_unit='invalid_unit', - ) - def test_scale_setter(self, brownian_diffusion_model): # WHEN brownian_diffusion_model.scale = 2.0 @@ -136,20 +112,6 @@ def test_calculate_width(self, brownian_diffusion_model): expected_widths = 1.0 * unit_conversion_factor.value * (Q_values**2) np.testing.assert_allclose(widths, expected_widths, rtol=1e-5) - def test_calculate_width_diffusion_unit_mev_angstrom2(self): - # WHEN - diffusion_model = BrownianTranslationalDiffusion( - diffusion_coefficient=2.0, diffusion_unit='meV*Å**2' - ) - Q_values = np.array([0.1, 0.2, 0.3]) # Example Q values in Å^-1 - - # WHEN - widths = diffusion_model.calculate_width(Q_values) - - # THEN EXPECT - expected_widths = 2.0 * (Q_values**2) - np.testing.assert_allclose(widths, expected_widths, rtol=1e-5) - def test_calculate_EISF(self, brownian_diffusion_model): # WHEN Q_values = np.array([0.1, 0.2, 0.3]) # Example Q values in Å^-1 diff --git a/tests/unit/easydynamics/sample_model/diffusion_model/test_jump_translational_diffusion.py b/tests/unit/easydynamics/sample_model/diffusion_model/test_jump_translational_diffusion.py new file mode 100644 index 00000000..9c54918c --- /dev/null +++ b/tests/unit/easydynamics/sample_model/diffusion_model/test_jump_translational_diffusion.py @@ -0,0 +1,254 @@ +import numpy as np +import pytest +import scipp as sc +from easyscience.variable import DescriptorNumber +from scipp import UnitError +from scipp.constants import hbar as scipp_hbar + +from easydynamics.sample_model.diffusion_model.jump_translational_diffusion import ( + JumpTranslationalDiffusion, +) + +hbar_1 = DescriptorNumber('hbar', 1.0) +hbar = DescriptorNumber.from_scipp('hbar', scipp_hbar) +angstrom = DescriptorNumber('angstrom', 1e-10, unit='m') + + +class TestJumpTranslationalDiffusion: + @pytest.fixture + def jump_diffusion_model(self): + return JumpTranslationalDiffusion() + + def test_init_default(self, jump_diffusion_model): + # WHEN THEN EXPECT + assert jump_diffusion_model.display_name == 'JumpTranslationalDiffusion' + assert jump_diffusion_model.unit == 'meV' + assert jump_diffusion_model.scale.value == 1.0 + assert jump_diffusion_model.diffusion_coefficient.value == 1.0 + assert jump_diffusion_model.relaxation_time.value == 1.0 + + @pytest.mark.parametrize( + 'kwargs,expected_exception, expected_message', + [ + ( + { + 'unit': 123, + 'scale': 1.0, + 'diffusion_coefficient': 1.0, + 'relaxation_time': 1.0, + }, + UnitError, + 'Invalid unit', + ), + ( + { + 'unit': 'meV', + 'scale': 'invalid', + 'diffusion_coefficient': 1.0, + 'relaxation_time': 1.0, + }, + TypeError, + 'scale must be a number', + ), + ( + { + 'unit': 'meV', + 'scale': 1.0, + 'diffusion_coefficient': 'invalid', + 'relaxation_time': 1.0, + }, + TypeError, + 'diffusion_coefficient must be a number', + ), + ( + { + 'unit': 'meV', + 'scale': 1.0, + 'diffusion_coefficient': 1.0, + 'relaxation_time': 'invalid', + }, + TypeError, + 'relaxation_time must be a number', + ), + ], + ) + def test_input_type_validation_raises(self, kwargs, expected_exception, expected_message): + with pytest.raises(expected_exception, match=expected_message): + JumpTranslationalDiffusion(display_name='JumpTranslationalDiffusion', **kwargs) + + def test_diffusion_coefficient_setter(self, jump_diffusion_model): + # WHEN + jump_diffusion_model.diffusion_coefficient = 3.0 + + # THEN EXPECT + assert jump_diffusion_model.diffusion_coefficient.value == 3.0 + + def test_diffusion_coefficient_setter_raises(self, jump_diffusion_model): + # WHEN THEN EXPECT + with pytest.raises(TypeError, match='diffusion_coefficient must be a number.'): + jump_diffusion_model.diffusion_coefficient = 'invalid' # Invalid type + + def test_relaxation_time_setter(self, jump_diffusion_model): + # WHEN + jump_diffusion_model.relaxation_time = 2.5 + + # THEN EXPECT + assert jump_diffusion_model.relaxation_time.value == 2.5 + + def test_relaxation_time_setter_raises(self, jump_diffusion_model): + # WHEN THEN EXPECT + with pytest.raises(TypeError, match='relaxation_time must be a number.'): + jump_diffusion_model.relaxation_time = 'invalid' # Invalid type + + def test_calculate_width_type_error(self, jump_diffusion_model): + # WHEN THEN EXPECT + with pytest.raises(TypeError, match='Q must be '): + jump_diffusion_model.calculate_width(Q='invalid') # Invalid type + + def test_calculate_width(self, jump_diffusion_model): + "Test the calculation relying solely on a scipp implementation" + 'instead of our Parameters' + # WHEN + Q_values = sc.linspace('Q', 0.5, 1.5, num=6, unit='1/angstrom') + relaxation_time_sc = jump_diffusion_model.relaxation_time.value * sc.Unit( + jump_diffusion_model.relaxation_time.unit + ) + diffusion_coefficient_sc = jump_diffusion_model.diffusion_coefficient.value * sc.Unit( + jump_diffusion_model.diffusion_coefficient.unit + ) + + # THEN + widths = jump_diffusion_model.calculate_width(Q_values) + + denominator = diffusion_coefficient_sc * relaxation_time_sc * Q_values**2 + denominator = denominator.to(unit='1') + + # EXPECT + expected_widths = scipp_hbar * diffusion_coefficient_sc * (Q_values**2) / (1 + denominator) + + expected_widths = expected_widths.to(unit=jump_diffusion_model.unit) + + np.testing.assert_allclose(widths, expected_widths.values, rtol=1e-5) + + def test_calculate_EISF(self, jump_diffusion_model): + # WHEN + Q_values = np.array([0.1, 0.2, 0.3]) # Example Q values in Å^-1 + + # THEN + EISF = jump_diffusion_model.calculate_EISF(Q_values) + + # EXPECT + expected_EISHF = np.zeros_like(Q_values) + np.testing.assert_array_equal(EISF, expected_EISHF) + + def test_calculate_EISF_type_error(self, jump_diffusion_model): + # WHEN THEN EXPECT + with pytest.raises(TypeError, match='Q must be '): + jump_diffusion_model.calculate_EISF(Q='invalid') # Invalid type + + def test_calculate_QISF(self, jump_diffusion_model): + # WHEN + Q_values = np.array([0.1, 0.2, 0.3]) # Example Q values in Å^-1 + + # THEN + QISF = jump_diffusion_model.calculate_QISF(Q_values) + + # EXPECT + expected_QISF = np.ones_like(Q_values) + np.testing.assert_array_equal(QISF, expected_QISF) + + def test_calculate_QISF_type_error(self, jump_diffusion_model): + # WHEN THEN EXPECT + with pytest.raises(TypeError, match='Q must be '): + jump_diffusion_model.calculate_QISF(Q='invalid') # Invalid type + + @pytest.mark.parametrize( + 'Q', + [ + (0.5), + ([1.0, 2.0, 3.0]), + (np.array([1.0, 2.0, 3.0])), + ], + ids=[ + 'python_scalar', + 'python_list', + 'numpy_array', + ], + ) + def test_create_component_collections(self, jump_diffusion_model, Q): + # WHEN + + # THEN + component_collections = jump_diffusion_model.create_component_collections(Q=Q) + + # EXPECT + expected_widths = jump_diffusion_model.calculate_width(Q) + for model_index in range(len(component_collections)): + model = component_collections[model_index] + assert len(model.components) == 1 + component = model.components[0] + assert component.width.unit == jump_diffusion_model.unit + assert np.isclose(component.width.value, expected_widths[model_index]) + assert component.width.independent is False + + def test_create_component_collections_component_name_must_be_string( + self, jump_diffusion_model + ): + # WHEN THEN EXPECT + with pytest.raises(TypeError, match='component_name must be a string.'): + jump_diffusion_model.create_component_collections( + Q=np.array([0.1, 0.2, 0.3]), component_display_name=123 + ) + + def test_create_component_collections_Q_type_error(self, jump_diffusion_model): + # WHEN THEN EXPECT + with pytest.raises(TypeError, match='Q must be a '): + jump_diffusion_model.create_component_collections(Q='invalid') # Invalid type + + def test_create_component_collections_Q_1dimensional_error(self, jump_diffusion_model): + # WHEN THEN EXPECT + with pytest.raises(ValueError, match='Q must be a 1-dimensional array.'): + jump_diffusion_model.create_component_collections( + Q=np.array([[0.1, 0.2], [0.3, 0.4]]) + ) # Invalid shape + + def test_write_width_dependency_expression(self, jump_diffusion_model): + # WHEN THEN + expression = jump_diffusion_model._write_width_dependency_expression(0.5) + + # EXPECT + expected_expression = ( + 'hbar * D* 0.5 **2*1/(angstrom**2)/(1 + (D * t* 0.5 **2/(angstrom**2)))' + ) + assert expression == expected_expression + + def test_write_width_dependency_map_expression(self, jump_diffusion_model): + # WHEN THEN + expression_map = jump_diffusion_model._write_width_dependency_map_expression() + + # EXPECT + expected_map = { + 'D': jump_diffusion_model.diffusion_coefficient, + 't': jump_diffusion_model.relaxation_time, + 'hbar': jump_diffusion_model._hbar, + 'angstrom': jump_diffusion_model._angstrom, + } + + assert expression_map == expected_map + + def test_write_width_dependency_expression_raises(self, jump_diffusion_model): + with pytest.raises(TypeError, match='Q must be a float'): + jump_diffusion_model._write_width_dependency_expression('invalid') + + def test_write_area_dependency_expression_raises(self, jump_diffusion_model): + with pytest.raises(TypeError, match='QISF must be a float'): + jump_diffusion_model._write_area_dependency_expression('invalid') + + def test_repr(self, jump_diffusion_model): + # WHEN THEN + repr_str = repr(jump_diffusion_model) + + # EXPECT + assert 'JumpTranslationalDiffusion' in repr_str + assert 'diffusion_coefficient' in repr_str + assert 'scale=' in repr_str From d2c80c0df7cd52ed79c0b6376d18b18648bd32f2 Mon Sep 17 00:00:00 2001 From: henrikjacobsenfys Date: Wed, 4 Feb 2026 20:35:02 +0100 Subject: [PATCH 02/12] fix notebook --- docs/docs/tutorials/diffusion_model.ipynb | 2 -- docs/docs/tutorials/sample_model.ipynb | 2 -- .../diffusion_model/brownian_translational_diffusion.py | 9 ++++----- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/docs/docs/tutorials/diffusion_model.ipynb b/docs/docs/tutorials/diffusion_model.ipynb index 9277486e..be9ec8e1 100644 --- a/docs/docs/tutorials/diffusion_model.ipynb +++ b/docs/docs/tutorials/diffusion_model.ipynb @@ -39,13 +39,11 @@ "energy = np.linspace(-2, 2, 501)\n", "scale = 1.0\n", "diffusion_coefficient = 2.4e-9 # m^2/s\n", - "diffusion_unit = 'm**2/s'\n", "\n", "diffusion_model = BrownianTranslationalDiffusion(\n", " display_name='DiffusionModel',\n", " scale=scale,\n", " diffusion_coefficient=diffusion_coefficient,\n", - " diffusion_unit=diffusion_unit,\n", ")\n", "\n", "component_collections = diffusion_model.create_component_collections(Q)\n", diff --git a/docs/docs/tutorials/sample_model.ipynb b/docs/docs/tutorials/sample_model.ipynb index 802aff0b..5371f7df 100644 --- a/docs/docs/tutorials/sample_model.ipynb +++ b/docs/docs/tutorials/sample_model.ipynb @@ -48,12 +48,10 @@ "\n", "scale = 1.0\n", "diffusion_coefficient = 2.4e-9 # m^2/s\n", - "diffusion_unit = 'm**2/s'\n", "diffusion_model = BrownianTranslationalDiffusion(\n", " display_name='DiffusionModel',\n", " scale=scale,\n", " diffusion_coefficient=diffusion_coefficient,\n", - " diffusion_unit=diffusion_unit,\n", ")\n", "\n", "\n", diff --git a/src/easydynamics/sample_model/diffusion_model/brownian_translational_diffusion.py b/src/easydynamics/sample_model/diffusion_model/brownian_translational_diffusion.py index 10f43c43..04adf4d1 100644 --- a/src/easydynamics/sample_model/diffusion_model/brownian_translational_diffusion.py +++ b/src/easydynamics/sample_model/diffusion_model/brownian_translational_diffusion.py @@ -57,15 +57,14 @@ def __init__( Defaults to "meV". scale : float or Parameter, optional Scale factor for the diffusion model. - diffusion_coefficient : float or Parameter, optional - Diffusion coefficient D. If a number is provided, - it is assumed to be in the unit given by diffusion_unit. + diffusion_coefficient : Number, optional + Diffusion coefficient D in m^2/s. Defaults to 1.0. """ - if not isinstance(scale, (Parameter, Numeric)): + if not isinstance(scale, Numeric): raise TypeError('scale must be a number.') - if not isinstance(diffusion_coefficient, (Parameter, Numeric)): + if not isinstance(diffusion_coefficient, Numeric): raise TypeError('diffusion_coefficient must be a number.') scale = Parameter(name='scale', value=float(scale), fixed=False, min=0.0) From 6749a8b1d0135bb44d0a81386f85525ef1f1a59b Mon Sep 17 00:00:00 2001 From: henrikjacobsenfys Date: Wed, 4 Feb 2026 20:44:41 +0100 Subject: [PATCH 03/12] fix weakref? --- tests/unit/easydynamics/sample_model/test_sample_model.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/easydynamics/sample_model/test_sample_model.py b/tests/unit/easydynamics/sample_model/test_sample_model.py index 8a383ee3..58735adf 100644 --- a/tests/unit/easydynamics/sample_model/test_sample_model.py +++ b/tests/unit/easydynamics/sample_model/test_sample_model.py @@ -51,7 +51,7 @@ def sample_model(self): return sample_model - def test_init(self, sample_model): + def test_init(self, sample_model, reset_global_object): # WHEN THEN model = sample_model @@ -142,7 +142,7 @@ def test_remove_diffusion_model_raises_with_invalid_name(self, sample_model): ): sample_model.remove_diffusion_model('non_existent_model') - def test_diffusion_model_setter(self, sample_model): + def test_diffusion_model_setter(self, sample_model, reset_global_object): # WHEN model = sample_model new_diffusion_model1 = BrownianTranslationalDiffusion() From 8d6906df33a3cfb5bb43c372e92babe9144d3f4d Mon Sep 17 00:00:00 2001 From: henrikjacobsenfys Date: Wed, 4 Feb 2026 20:56:23 +0100 Subject: [PATCH 04/12] fix weakref? --- tests/unit/easydynamics/sample_model/test_sample_model.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/unit/easydynamics/sample_model/test_sample_model.py b/tests/unit/easydynamics/sample_model/test_sample_model.py index 58735adf..8810572e 100644 --- a/tests/unit/easydynamics/sample_model/test_sample_model.py +++ b/tests/unit/easydynamics/sample_model/test_sample_model.py @@ -4,6 +4,7 @@ from unittest.mock import Mock from unittest.mock import patch +import easyscience.global_object import numpy as np import pytest from scipp import UnitError @@ -51,7 +52,9 @@ def sample_model(self): return sample_model - def test_init(self, sample_model, reset_global_object): + def test_init(self, sample_model): + easyscience.global_object.map._clear() + # WHEN THEN model = sample_model @@ -142,7 +145,8 @@ def test_remove_diffusion_model_raises_with_invalid_name(self, sample_model): ): sample_model.remove_diffusion_model('non_existent_model') - def test_diffusion_model_setter(self, sample_model, reset_global_object): + def test_diffusion_model_setter(self, sample_model): + easyscience.global_object.map._clear() # WHEN model = sample_model new_diffusion_model1 = BrownianTranslationalDiffusion() From 7211f3873b14725b2683a88bada178a0c5fc8bae Mon Sep 17 00:00:00 2001 From: henrikjacobsenfys Date: Wed, 4 Feb 2026 21:02:20 +0100 Subject: [PATCH 05/12] fix weakref??? --- tests/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index aefc6c0b..45cef61c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -20,6 +20,6 @@ # monkeypatch.setattr(easyscience.global_object, 'map', Map()) -@pytest.fixture(autouse=False) +@pytest.fixture(autouse=True) def reset_global_object(): easyscience.global_object.map._clear() From 2c32af98fb822db4c36282b996a8e5355b1069ee Mon Sep 17 00:00:00 2001 From: henrikjacobsenfys Date: Wed, 4 Feb 2026 21:16:01 +0100 Subject: [PATCH 06/12] ... --- tests/unit/easydynamics/sample_model/test_sample_model.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/unit/easydynamics/sample_model/test_sample_model.py b/tests/unit/easydynamics/sample_model/test_sample_model.py index 8810572e..311ef1ca 100644 --- a/tests/unit/easydynamics/sample_model/test_sample_model.py +++ b/tests/unit/easydynamics/sample_model/test_sample_model.py @@ -4,7 +4,6 @@ from unittest.mock import Mock from unittest.mock import patch -import easyscience.global_object import numpy as np import pytest from scipp import UnitError @@ -53,7 +52,6 @@ def sample_model(self): return sample_model def test_init(self, sample_model): - easyscience.global_object.map._clear() # WHEN THEN model = sample_model @@ -146,7 +144,6 @@ def test_remove_diffusion_model_raises_with_invalid_name(self, sample_model): sample_model.remove_diffusion_model('non_existent_model') def test_diffusion_model_setter(self, sample_model): - easyscience.global_object.map._clear() # WHEN model = sample_model new_diffusion_model1 = BrownianTranslationalDiffusion() From 01a5d6b6dc68109b223bdc56caac247914b94642 Mon Sep 17 00:00:00 2001 From: henrikjacobsenfys Date: Wed, 4 Feb 2026 22:28:07 +0100 Subject: [PATCH 07/12] weakref... --- .../brownian_translational_diffusion.py | 22 +------------------ tests/conftest.py | 11 ---------- .../sample_model/test_model_base.py | 2 +- .../sample_model/test_resolution_model.py | 4 +--- .../sample_model/test_sample_model.py | 6 ++++- 5 files changed, 8 insertions(+), 37 deletions(-) diff --git a/src/easydynamics/sample_model/diffusion_model/brownian_translational_diffusion.py b/src/easydynamics/sample_model/diffusion_model/brownian_translational_diffusion.py index 04adf4d1..05878cee 100644 --- a/src/easydynamics/sample_model/diffusion_model/brownian_translational_diffusion.py +++ b/src/easydynamics/sample_model/diffusion_model/brownian_translational_diffusion.py @@ -67,8 +67,6 @@ def __init__( if not isinstance(diffusion_coefficient, Numeric): raise TypeError('diffusion_coefficient must be a number.') - scale = Parameter(name='scale', value=float(scale), fixed=False, min=0.0) - diffusion_coefficient = Parameter( name='diffusion_coefficient', value=float(diffusion_coefficient), @@ -79,30 +77,12 @@ def __init__( display_name=display_name, unique_name=unique_name, unit=unit, + scale=scale, ) self._hbar = DescriptorNumber.from_scipp('hbar', scipp_hbar) self._angstrom = DescriptorNumber('angstrom', 1e-10, unit='m') - self._scale = scale self._diffusion_coefficient = diffusion_coefficient - @property - def scale(self) -> Parameter: - """Get the scale parameter of the diffusion model. - - Returns - ------- - Parameter - Scale parameter. - """ - return self._scale - - @scale.setter - def scale(self, scale: Numeric) -> None: - """Set the scale parameter of the diffusion model.""" - if not isinstance(scale, (Numeric)): - raise TypeError('scale must be a number.') - self._scale.value = scale - @property def diffusion_coefficient(self) -> Parameter: """Get the diffusion coefficient parameter D. diff --git a/tests/conftest.py b/tests/conftest.py index 45cef61c..f3086384 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,17 +8,6 @@ import easyscience.global_object import pytest -# from easyscience.global_object.map import Map - - -# @pytest.fixture(autouse=True) -# def reset_global_object(monkeypatch): -# # Before each test -# monkeypatch.setattr(easyscience.global_object, 'map', Map()) -# yield -# # After each test (cleanup) -# monkeypatch.setattr(easyscience.global_object, 'map', Map()) - @pytest.fixture(autouse=True) def reset_global_object(): diff --git a/tests/unit/easydynamics/sample_model/test_model_base.py b/tests/unit/easydynamics/sample_model/test_model_base.py index 31feb66a..44541e12 100644 --- a/tests/unit/easydynamics/sample_model/test_model_base.py +++ b/tests/unit/easydynamics/sample_model/test_model_base.py @@ -14,7 +14,7 @@ class TestModelBase: @pytest.fixture - def model_base(self, reset_global_object): + def model_base(self): component1 = Gaussian( display_name='TestGaussian1', area=1.0, diff --git a/tests/unit/easydynamics/sample_model/test_resolution_model.py b/tests/unit/easydynamics/sample_model/test_resolution_model.py index 120cbf9b..7743056b 100644 --- a/tests/unit/easydynamics/sample_model/test_resolution_model.py +++ b/tests/unit/easydynamics/sample_model/test_resolution_model.py @@ -136,9 +136,7 @@ def test_append_component_collection(self, resolution_model): ], ids=['DeltaFunction', 'Polynomial'], ) - def test_append_invalid_component_type_raises( - self, resolution_model, invalid_component, reset_global_object - ): + def test_append_invalid_component_type_raises(self, resolution_model, invalid_component): # WHEN / THEN / EXPECT # appending a single component with pytest.raises( diff --git a/tests/unit/easydynamics/sample_model/test_sample_model.py b/tests/unit/easydynamics/sample_model/test_sample_model.py index 311ef1ca..e5f7a9a7 100644 --- a/tests/unit/easydynamics/sample_model/test_sample_model.py +++ b/tests/unit/easydynamics/sample_model/test_sample_model.py @@ -22,6 +22,7 @@ class TestSampleModel: def sample_model(self): component1 = Gaussian( display_name='TestGaussian1', + unique_name='TestGaussian1', area=1.0, center=0.0, width=1.0, @@ -29,6 +30,7 @@ def sample_model(self): ) component2 = Lorentzian( display_name='TestLorentzian1', + unique_name='TestLorentzian1', area=2.0, center=1.0, width=0.5, @@ -38,7 +40,9 @@ def sample_model(self): component_collection.append_component(component1) component_collection.append_component(component2) - diffusion_model = BrownianTranslationalDiffusion(display_name='DiffusionModel') + diffusion_model = BrownianTranslationalDiffusion( + display_name='DiffusionModel', unique_name='DiffusionModel' + ) sample_model = SampleModel( display_name='InitModel', From 90120561223b42152a98ffccb3c8c989b3799f24 Mon Sep 17 00:00:00 2001 From: henrikjacobsenfys Date: Wed, 4 Feb 2026 22:44:10 +0100 Subject: [PATCH 08/12] claude fixed it? --- tests/conftest.py | 69 ++++++++++++++++++- .../sample_model/test_resolution_model.py | 2 +- 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index f3086384..d11735d3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,10 +5,73 @@ # TODO: remove once weakref bug is fixed -import easyscience.global_object +# import easyscience.global_object +# import pytest + + +# @pytest.fixture(autouse=True) +# def reset_global_object(): +# easyscience.global_object.map._clear() + +from unittest.mock import patch + import pytest @pytest.fixture(autouse=True) -def reset_global_object(): - easyscience.global_object.map._clear() +def patch_easyscience_map(): + """Patch the problematic Map methods.""" + from easyscience.global_object.map import Map + + # Store the original methods + original_add_vertex = Map.add_vertex + # original_vertices = Map.vertices + + def safe_add_vertex(self, obj: object, obj_type: str = None): + try: + return original_add_vertex(self, obj, obj_type) + except KeyError: + # Object was garbage collected during setup + name = obj.unique_name + # Clean up any partial state + if hasattr(self, '_Map__type_dict') and name in self._Map__type_dict: + del self._Map__type_dict[name] + if name in self._store: + del self._store[name] + + def safe_vertices(self): + """Safe version of vertices() that handles dictionary changes + during iteration.""" + max_retries = 3 + for attempt in range(max_retries): + try: + return list(self._store.keys()) + except RuntimeError as e: + if 'dictionary changed size during iteration' in str(e): + if attempt < max_retries - 1: + # Force cleanup and try again + import gc + + gc.collect() + continue + else: + # Last attempt - return what we can get + try: + # Try to get keys in a different way + keys = [] + for k in list(self._store.data.keys()): + if k in self._store: + keys.append(k) + return keys + except: # noqa: E722 + return [] + else: + raise + return [] + + # Apply the patches + with ( + patch.object(Map, 'add_vertex', safe_add_vertex), + patch.object(Map, 'vertices', safe_vertices), + ): + yield diff --git a/tests/unit/easydynamics/sample_model/test_resolution_model.py b/tests/unit/easydynamics/sample_model/test_resolution_model.py index 7743056b..d45eee19 100644 --- a/tests/unit/easydynamics/sample_model/test_resolution_model.py +++ b/tests/unit/easydynamics/sample_model/test_resolution_model.py @@ -89,7 +89,7 @@ def test_init_raises_with_invalid_components(self, invalid_component, expected_e collection.append_component(invalid_component) ResolutionModel(components=collection) - def test_append_and_remove_and_clear_component(self, resolution_model, reset_global_object): + def test_append_and_remove_and_clear_component(self, resolution_model): # WHEN new_component = Gaussian(unique_name='NewGaussian') From 636736654cadcdd74f92091fa0f0c00c62da5e24 Mon Sep 17 00:00:00 2001 From: henrikjacobsenfys Date: Thu, 5 Feb 2026 10:28:34 +0100 Subject: [PATCH 09/12] respond to PR comments --- .../diffusion_model/diffusion_model_base.py | 3 +++ .../jump_translational_diffusion.py | 23 ++++++++++--------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py b/src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py index 3694f7af..fe5fb55d 100644 --- a/src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py +++ b/src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py @@ -1,3 +1,6 @@ +# SPDX-FileCopyrightText: 2025-2026 EasyDynamics contributors +# SPDX-License-Identifier: BSD-3-Clause + import scipp as sc from easyscience.base_classes.model_base import ModelBase from easyscience.variable import DescriptorNumber diff --git a/src/easydynamics/sample_model/diffusion_model/jump_translational_diffusion.py b/src/easydynamics/sample_model/diffusion_model/jump_translational_diffusion.py index 233a7784..efc97d1a 100644 --- a/src/easydynamics/sample_model/diffusion_model/jump_translational_diffusion.py +++ b/src/easydynamics/sample_model/diffusion_model/jump_translational_diffusion.py @@ -151,12 +151,12 @@ def calculate_width(self, Q: Q_type) -> np.ndarray: Q = _validate_and_convert_Q(Q) - unit_conversion_factor_nominator = ( + unit_conversion_factor_numerator = ( self._hbar * self.diffusion_coefficient / (self._angstrom**2) ) - unit_conversion_factor_nominator.convert_unit(self.unit) + unit_conversion_factor_numerator.convert_unit(self.unit) - nominator = unit_conversion_factor_nominator.value * Q**2 + numerator = unit_conversion_factor_numerator.value * Q**2 unit_conversion_factor_denominator = ( self.diffusion_coefficient / self._angstrom**2 * self.relaxation_time @@ -165,8 +165,7 @@ def calculate_width(self, Q: Q_type) -> np.ndarray: denominator = 1 + unit_conversion_factor_denominator.value * Q**2 - width = nominator / denominator - + width = numerator / denominator return width def calculate_EISF(self, Q: Q_type) -> np.ndarray: @@ -208,19 +207,21 @@ def calculate_QISF(self, Q: Q_type) -> np.ndarray: def create_component_collections( self, Q: Q_type, - component_display_name: str = 'Brownian translational diffusion', + component_display_name: str = 'Jump translational diffusion', ) -> List[ComponentCollection]: - """Create ComponentCollection components for the Brownian. + """Create ComponentCollection components for the diffusion model + at given Q values. - translational diffusion model at given Q values. Args: - ---------- Q : Number, list, or np.ndarray + Args: + ---------- + Q : Number, list, or np.ndarray Scattering vector values. component_display_name : str - Name of the Brownian Diffusion Lorentzian component. + Name of the Jump Diffusion Lorentzian component. Returns ------- List[ComponentCollection] - List of ComponentCollections with Brownian Diffusion + List of ComponentCollections with Jump Diffusion Lorentzian components. """ Q = _validate_and_convert_Q(Q) From e0711642d990fe878f47f9eaab53616cd73fadc6 Mon Sep 17 00:00:00 2001 From: henrikjacobsenfys Date: Thu, 5 Feb 2026 10:35:45 +0100 Subject: [PATCH 10/12] Move scale test to base model and add some formatting --- .../brownian_translational_diffusion.py | 18 +++++++++++++++++- .../diffusion_model/diffusion_model_base.py | 8 ++++++++ .../test_brownian_translational_diffusion.py | 12 ------------ .../diffusion_model/test_diffusion_model.py | 12 ++++++++++++ 4 files changed, 37 insertions(+), 13 deletions(-) diff --git a/src/easydynamics/sample_model/diffusion_model/brownian_translational_diffusion.py b/src/easydynamics/sample_model/diffusion_model/brownian_translational_diffusion.py index 05878cee..d277d227 100644 --- a/src/easydynamics/sample_model/diffusion_model/brownian_translational_diffusion.py +++ b/src/easydynamics/sample_model/diffusion_model/brownian_translational_diffusion.py @@ -83,6 +83,10 @@ def __init__( self._angstrom = DescriptorNumber('angstrom', 1e-10, unit='m') self._diffusion_coefficient = diffusion_coefficient + # ------------------------------------------------------------------ + # Properties + # ------------------------------------------------------------------ + @property def diffusion_coefficient(self) -> Parameter: """Get the diffusion coefficient parameter D. @@ -97,10 +101,14 @@ def diffusion_coefficient(self) -> Parameter: @diffusion_coefficient.setter def diffusion_coefficient(self, diffusion_coefficient: Numeric) -> None: """Set the diffusion coefficient parameter D.""" - if not isinstance(diffusion_coefficient, (Numeric)): + if not isinstance(diffusion_coefficient, Numeric): raise TypeError('diffusion_coefficient must be a number.') self._diffusion_coefficient.value = diffusion_coefficient + # ------------------------------------------------------------------ + # Other methods + # ------------------------------------------------------------------ + def calculate_width(self, Q: Q_type) -> np.ndarray: """Calculate the half-width at half-maximum (HWHM) for the diffusion model. @@ -226,6 +234,10 @@ def create_component_collections( return component_collection_list + # ------------------------------------------------------------------ + # Private methods + # ------------------------------------------------------------------ + def _write_width_dependency_expression(self, Q: float) -> str: """Write the dependency expression for the width as a function of Q to make dependent Parameters. @@ -277,6 +289,10 @@ def _write_area_dependency_map_expression(self) -> Dict[str, DescriptorNumber]: 'scale': self.scale, } + # ------------------------------------------------------------------ + # dunder methods + # ------------------------------------------------------------------ + def __repr__(self): """String representation of the BrownianTranslationalDiffusion model. diff --git a/src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py b/src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py index fe5fb55d..6326abe1 100644 --- a/src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py +++ b/src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py @@ -46,6 +46,10 @@ def __init__( self._unit = unit self._scale = scale + # ------------------------------------------------------------------ + # Properties + # ------------------------------------------------------------------ + @property def unit(self) -> str: """Get the unit of the DiffusionModel. @@ -83,6 +87,10 @@ def scale(self, scale: Numeric) -> None: raise TypeError('scale must be a number.') self._scale.value = scale + # ------------------------------------------------------------------ + # dunder methods + # ------------------------------------------------------------------ + def __repr__(self): """String representation of the Diffusion model.""" return f'{self.__class__.__name__}(display_name={self.display_name}, unit={self.unit})' diff --git a/tests/unit/easydynamics/sample_model/diffusion_model/test_brownian_translational_diffusion.py b/tests/unit/easydynamics/sample_model/diffusion_model/test_brownian_translational_diffusion.py index 2d28726c..0d0963c0 100644 --- a/tests/unit/easydynamics/sample_model/diffusion_model/test_brownian_translational_diffusion.py +++ b/tests/unit/easydynamics/sample_model/diffusion_model/test_brownian_translational_diffusion.py @@ -65,18 +65,6 @@ def test_input_type_validation_raises(self, kwargs, expected_exception, expected with pytest.raises(expected_exception, match=expected_message): BrownianTranslationalDiffusion(display_name='BrownianTranslationalDiffusion', **kwargs) - def test_scale_setter(self, brownian_diffusion_model): - # WHEN - brownian_diffusion_model.scale = 2.0 - - # THEN EXPECT - assert brownian_diffusion_model.scale.value == 2.0 - - def test_scale_setter_raises(self, brownian_diffusion_model): - # WHEN THEN EXPECT - with pytest.raises(TypeError, match='scale must be a number.'): - brownian_diffusion_model.scale = 'invalid' # Invalid type - def test_diffusion_coefficient_setter(self, brownian_diffusion_model): # WHEN brownian_diffusion_model.diffusion_coefficient = 3.0 diff --git a/tests/unit/easydynamics/sample_model/diffusion_model/test_diffusion_model.py b/tests/unit/easydynamics/sample_model/diffusion_model/test_diffusion_model.py index e7bca65a..b8eb0956 100644 --- a/tests/unit/easydynamics/sample_model/diffusion_model/test_diffusion_model.py +++ b/tests/unit/easydynamics/sample_model/diffusion_model/test_diffusion_model.py @@ -23,3 +23,15 @@ def test_unit_setter_raises(self, diffusion_model): match='Unit is read-only. Use convert_unit to change the unit between allowed types', ): diffusion_model.unit = 'eV' + + def test_scale_setter(self, diffusion_model): + # WHEN + diffusion_model.scale = 2.0 + + # THEN EXPECT + assert diffusion_model.scale.value == 2.0 + + def test_scale_setter_raises(self, diffusion_model): + # WHEN THEN EXPECT + with pytest.raises(TypeError, match='scale must be a number.'): + diffusion_model.scale = 'invalid' # Invalid type From a87c053af53d66c54aabca1f6f1844d8f3e1b0e5 Mon Sep 17 00:00:00 2001 From: henrikjacobsenfys Date: Thu, 5 Feb 2026 10:38:48 +0100 Subject: [PATCH 11/12] fix typo --- .../sample_model/diffusion_model/diffusion_model_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py b/src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py index 6326abe1..a6711334 100644 --- a/src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py +++ b/src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py @@ -39,7 +39,7 @@ def __init__( test.convert_unit('meV') except Exception as e: raise UnitError( - f'Invalid unit: {unit}. Unit must be a string or scipp Unitand convertible to meV.' + f'Invalid unit: {unit}. Unit must be a string or scipp Unit and convertible to meV.' # noqa: E501 ) from e super().__init__(display_name=display_name, unique_name=unique_name) From a1901ffbc38ff5f8926f7f41070629a4968395d6 Mon Sep 17 00:00:00 2001 From: henrikjacobsenfys Date: Thu, 5 Feb 2026 10:45:51 +0100 Subject: [PATCH 12/12] small fix --- .../diffusion_model/jump_translational_diffusion.py | 2 +- .../diffusion_model/test_jump_translational_diffusion.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/easydynamics/sample_model/diffusion_model/jump_translational_diffusion.py b/src/easydynamics/sample_model/diffusion_model/jump_translational_diffusion.py index efc97d1a..8bb65480 100644 --- a/src/easydynamics/sample_model/diffusion_model/jump_translational_diffusion.py +++ b/src/easydynamics/sample_model/diffusion_model/jump_translational_diffusion.py @@ -291,7 +291,7 @@ def _write_width_dependency_expression(self, Q: float) -> str: raise TypeError('Q must be a float.') # Q is given as a float, so we need to add the units - return f'hbar * D* {Q} **2*1/(angstrom**2)/(1 + (D * t* {Q} **2/(angstrom**2)))' + return f'hbar * D* {Q} **2/(angstrom**2)/(1 + (D * t* {Q} **2/(angstrom**2)))' def _write_width_dependency_map_expression(self) -> Dict[str, DescriptorNumber]: """Write the dependency map expression to make dependent diff --git a/tests/unit/easydynamics/sample_model/diffusion_model/test_jump_translational_diffusion.py b/tests/unit/easydynamics/sample_model/diffusion_model/test_jump_translational_diffusion.py index 9c54918c..90a842d6 100644 --- a/tests/unit/easydynamics/sample_model/diffusion_model/test_jump_translational_diffusion.py +++ b/tests/unit/easydynamics/sample_model/diffusion_model/test_jump_translational_diffusion.py @@ -138,8 +138,8 @@ def test_calculate_EISF(self, jump_diffusion_model): EISF = jump_diffusion_model.calculate_EISF(Q_values) # EXPECT - expected_EISHF = np.zeros_like(Q_values) - np.testing.assert_array_equal(EISF, expected_EISHF) + expected_EISF = np.zeros_like(Q_values) + np.testing.assert_array_equal(EISF, expected_EISF) def test_calculate_EISF_type_error(self, jump_diffusion_model): # WHEN THEN EXPECT @@ -218,7 +218,7 @@ def test_write_width_dependency_expression(self, jump_diffusion_model): # EXPECT expected_expression = ( - 'hbar * D* 0.5 **2*1/(angstrom**2)/(1 + (D * t* 0.5 **2/(angstrom**2)))' + 'hbar * D* 0.5 **2/(angstrom**2)/(1 + (D * t* 0.5 **2/(angstrom**2)))' ) assert expression == expected_expression