From fcb611cccc9e4a1bbcb4e17c037a761764b2aadb Mon Sep 17 00:00:00 2001 From: Piotr Rozyczko Date: Mon, 15 Jun 2026 19:13:40 +0200 Subject: [PATCH 1/2] fixed issue #367 --- .../calculators/refl1d/wrapper.py | 7 +- .../calculators/refnx/wrapper.py | 10 +- .../model/resolution_functions.py | 42 ++++- tests/calculators/refnx/test_refnx_wrapper.py | 2 +- .../test_cross_engine_resolution.py | 144 ++++++++++++++++++ tests/model/test_model.py | 13 +- tests/model/test_resolution_functions.py | 30 ++-- 7 files changed, 211 insertions(+), 37 deletions(-) create mode 100644 tests/integration/test_cross_engine_resolution.py diff --git a/src/easyreflectometry/calculators/refl1d/wrapper.py b/src/easyreflectometry/calculators/refl1d/wrapper.py index 7a07c917..6985d47c 100644 --- a/src/easyreflectometry/calculators/refl1d/wrapper.py +++ b/src/easyreflectometry/calculators/refl1d/wrapper.py @@ -8,8 +8,6 @@ from refl1d import names from refl1d.sample.layers import Repeat -from easyreflectometry.model import PercentageFwhm - from ..wrapper_base import WrapperBase RESOLUTION_PADDING = 3.5 @@ -205,12 +203,9 @@ def calculate(self, q_array: np.ndarray, model_name: str) -> np.ndarray: Reflectivity calculated at q. """ sample = _build_sample(self.storage, model_name) + # smearing() returns sigma, which is exactly what refl1d's probe.dQ expects. dq_array = self._resolution_function.smearing(q_array) - if isinstance(self._resolution_function, PercentageFwhm): - # Get percentage of Q and change from sigma to FWHM - dq_array = dq_array * q_array / 100 / (2 * np.sqrt(2 * np.log(2))) - if not self._magnetism: probe = _get_probe( q_array=q_array, diff --git a/src/easyreflectometry/calculators/refnx/wrapper.py b/src/easyreflectometry/calculators/refnx/wrapper.py index 3742d727..65dc8662 100644 --- a/src/easyreflectometry/calculators/refnx/wrapper.py +++ b/src/easyreflectometry/calculators/refnx/wrapper.py @@ -8,6 +8,7 @@ from refnx import reflect from easyreflectometry.model import PercentageFwhm +from easyreflectometry.model.resolution_functions import SIGMA_TO_FWHM from ..wrapper_base import WrapperBase @@ -191,9 +192,12 @@ def calculate(self, q_array: np.ndarray, model_name: str) -> np.ndarray: dq_vector = self._resolution_function.smearing(q_array) if isinstance(self._resolution_function, PercentageFwhm): - # FWHM Percentage resolution is constant given as - # For a constant resolution percentage refnx supports to pass a scalar value rather than a vector - dq_vector = dq_vector[0] + # refnx interprets a scalar x_err as a constant dq/q (FWHM percentage), + # so pass the percentage directly rather than a per-point vector. + dq_vector = self._resolution_function.constant + else: + # smearing() returns sigma; refnx expects the FWHM at each point. + dq_vector = dq_vector * SIGMA_TO_FWHM return model(x=q_array, x_err=dq_vector) diff --git a/src/easyreflectometry/model/resolution_functions.py b/src/easyreflectometry/model/resolution_functions.py index fe5d2254..9579ad66 100644 --- a/src/easyreflectometry/model/resolution_functions.py +++ b/src/easyreflectometry/model/resolution_functions.py @@ -6,6 +6,14 @@ Gaussian distribution with a FWHM of the percentage of the q value. To convert from a sigma value to a FWHM value we use the formula FWHM = 2.35 * sigma [2 * np.sqrt(2 * np.log(2)) * sigma]. + +The :meth:`ResolutionFunction.smearing` contract returns **sigma** +(the standard deviation of the Gaussian resolution) for every resolution +type. This matches the ``sQz`` convention used by data reduction and the +natural output of :class:`Pointwise`. Each calculation engine wrapper is +responsible for converting sigma to the width convention of its backend +(FWHM for refnx, sigma for refl1d), so that vector resolutions are +interpreted consistently across engines (see GitHub issue #367). """ from __future__ import annotations @@ -19,10 +27,15 @@ DEFAULT_RESOLUTION_FWHM_PERCENTAGE = 5.0 +# Conversion factor between sigma and FWHM for a Gaussian: FWHM = SIGMA_TO_FWHM * sigma. +SIGMA_TO_FWHM = 2 * np.sqrt(2 * np.log(2)) + class ResolutionFunction: @abstractmethod - def smearing(self, q: Union[np.array, float]) -> np.array: ... + def smearing(self, q: Union[np.array, float]) -> np.array: + """Return the resolution as sigma (standard deviation) at each ``q``.""" + ... @abstractmethod def as_dict(self, skip: Optional[List[str]] = None) -> dict: ... @@ -51,8 +64,14 @@ def __init__(self, constant: Union[None, float] = None): self.constant = constant def smearing(self, q: Union[np.array, float]) -> np.array: - """Smearing function.""" - return np.ones(np.array(q).size) * self.constant + """Return per-point sigma values from the constant FWHM percentage. + + ``constant`` is a FWHM percentage of ``q``; it is converted to an + absolute sigma so the smearing() contract is sigma for all types. + """ + q_array = np.asarray(q, dtype=float) + fwhm = (self.constant / 100.0) * q_array + return fwhm / SIGMA_TO_FWHM def as_dict( self, skip: Optional[List[str]] = None @@ -68,8 +87,13 @@ def __init__(self, q_data_points: np.array, fwhm_values: np.array): self.fwhm_values = fwhm_values def smearing(self, q: Union[np.array, float]) -> np.array: - """Smearing function.""" - return np.interp(q, self.q_data_points, self.fwhm_values) + """Return per-point sigma values from the FWHM knots. + + The stored ``fwhm_values`` are FWHM widths; they are interpolated + onto ``q`` and converted to sigma to satisfy the smearing() contract. + """ + fwhm = np.interp(np.asarray(q, dtype=float), self.q_data_points, self.fwhm_values) + return fwhm / SIGMA_TO_FWHM def as_dict( self, skip: Optional[List[str]] = None @@ -110,10 +134,12 @@ def __init__(self, q_data_points: List[np.ndarray]): self.q_data_points = q_data_points def smearing(self, q: Optional[Union[np.ndarray, float]] = None) -> np.ndarray: - """Return the resolution width interpolated onto ``q``. + """Return the resolution sigma interpolated onto ``q``. - The width at each data point is ``sqrt(sQz)``; values are linearly - interpolated onto the requested ``q``. When ``q`` is ``None`` the widths + ``sQz`` is the variance of ``Qz``, so the sigma at each data point is + ``sqrt(sQz)``; values are linearly interpolated onto the requested + ``q``. This already satisfies the sigma smearing() contract, so no + FWHM conversion is applied. When ``q`` is ``None`` the sigma values are returned at the stored data points. """ Qz = np.asarray(self.q_data_points[0], dtype=float) diff --git a/tests/calculators/refnx/test_refnx_wrapper.py b/tests/calculators/refnx/test_refnx_wrapper.py index f9d2a4cc..bb99d633 100644 --- a/tests/calculators/refnx/test_refnx_wrapper.py +++ b/tests/calculators/refnx/test_refnx_wrapper.py @@ -451,7 +451,7 @@ def test_calculate_github_test4_spline_resolution(self): p.add_item('Item3', 'MyModel') p.add_item('Item4', 'MyModel') p.update_model('MyModel', bkg=0) - sigma_to_fwhm = 2.355 + sigma_to_fwhm = 2.0 * np.sqrt(2.0 * np.log(2.0)) p.set_resolution_function(LinearSpline(test4_dat[:, 0], sigma_to_fwhm * test4_dat[:, 3])) assert_allclose(p.calculate(test4_dat[:, 0], 'MyModel'), test4_dat[:, 1], rtol=0.03) diff --git a/tests/integration/test_cross_engine_resolution.py b/tests/integration/test_cross_engine_resolution.py new file mode 100644 index 00000000..07feff80 --- /dev/null +++ b/tests/integration/test_cross_engine_resolution.py @@ -0,0 +1,144 @@ +# SPDX-FileCopyrightText: 2026 EasyScience contributors +# SPDX-License-Identifier: BSD-3-Clause + +"""Cross-engine regression tests for resolution function width conventions. + +These tests verify that the same model + resolution function produces +consistent results across the refnx and refl1d engines, confirming that the +sigma/FWHM convention conversion is applied correctly at each engine +boundary. + +.. note:: + + The engines use different internal resolution algorithms (refnx uses + pointwise convolution, refl1d uses oversampling), so results can differ + by ~10-20% even with identical width conventions. The tolerance is set + wide enough to accommodate this while still catching a 2.355x convention + error, which would produce >100% differences. + +See GitHub issue #367 for background. +""" + +import numpy as np +import pytest +from numpy.testing import assert_allclose + +from easyreflectometry.calculators.refl1d.wrapper import Refl1dWrapper +from easyreflectometry.calculators.refnx.wrapper import RefnxWrapper +from easyreflectometry.model.resolution_functions import SIGMA_TO_FWHM +from easyreflectometry.model.resolution_functions import LinearSpline +from easyreflectometry.model.resolution_functions import PercentageFwhm +from easyreflectometry.model.resolution_functions import Pointwise + + +def _build_simple_model_refnx(wrapper): + """Build a simple single-film model on a refnx wrapper.""" + wrapper.reset_storage() + wrapper.create_material('Substrate') + wrapper.update_material('Substrate', real=2.07, imag=0.0) + wrapper.create_material('Film') + wrapper.update_material('Film', real=3.45, imag=0.0) + wrapper.create_layer('SubstrateLayer') + wrapper.assign_material_to_layer('Substrate', 'SubstrateLayer') + wrapper.create_layer('FilmLayer') + wrapper.assign_material_to_layer('Film', 'FilmLayer') + wrapper.update_layer('FilmLayer', thick=100.0, rough=3.0) + wrapper.create_item('Item') + wrapper.add_layer_to_item('FilmLayer', 'Item') + wrapper.add_layer_to_item('SubstrateLayer', 'Item') + wrapper.create_model('MyModel') + wrapper.add_item('Item', 'MyModel') + wrapper.update_model('MyModel', bkg=0.0) + + +def _build_simple_model_refl1d(wrapper): + """Build the same single-film model on a refl1d wrapper.""" + wrapper.reset_storage() + wrapper.create_material('Substrate') + wrapper.update_material('Substrate', rho=2.07, irho=0.0) + wrapper.create_material('Film') + wrapper.update_material('Film', rho=3.45, irho=0.0) + wrapper.create_layer('SubstrateLayer') + wrapper.assign_material_to_layer('Substrate', 'SubstrateLayer') + wrapper.create_layer('FilmLayer') + wrapper.assign_material_to_layer('Film', 'FilmLayer') + wrapper.update_layer('FilmLayer', thickness=100.0, interface=3.0) + wrapper.create_item('Item') + wrapper.add_layer_to_item('FilmLayer', 'Item') + wrapper.add_layer_to_item('SubstrateLayer', 'Item') + wrapper.create_model('MyModel') + wrapper.add_item('Item', 'MyModel') + wrapper.update_model('MyModel', bkg=0.0) + + +@pytest.mark.parametrize('resolution_pct', [1.0, 5.0, 10.0]) +def test_percentage_fwhm_consistent_across_engines(resolution_pct): + """PercentageFwhm resolution gives consistent results across engines.""" + q = np.geomspace(0.005, 0.3, 100) + + refnx_w = RefnxWrapper() + _build_simple_model_refnx(refnx_w) + refnx_w.set_resolution_function(PercentageFwhm(resolution_pct)) + refnx_r = refnx_w.calculate(q, 'MyModel') + + refl1d_w = Refl1dWrapper() + _build_simple_model_refl1d(refl1d_w) + refl1d_w.set_resolution_function(PercentageFwhm(resolution_pct)) + refl1d_r = refl1d_w.calculate(q, 'MyModel') + + assert_allclose(refnx_r, refl1d_r, rtol=0.25) + + +def test_linear_spline_consistent_across_engines(): + """LinearSpline resolution gives consistent results across engines. + + This is the key regression test for issue #367 -- before the fix, refl1d + over-smeared by ~2.355x with LinearSpline because the FWHM->sigma + conversion was missing, while refnx treated the same vector as FWHM. + """ + q = np.geomspace(0.005, 0.3, 100) + + # A plausible per-point FWHM resolution that increases with q. + q_knots = np.linspace(0.001, 0.5, 10) + fwhm_knots = 0.02 * q_knots + 0.001 + + refnx_w = RefnxWrapper() + _build_simple_model_refnx(refnx_w) + refnx_w.set_resolution_function(LinearSpline(q_knots, fwhm_knots)) + refnx_r = refnx_w.calculate(q, 'MyModel') + + refl1d_w = Refl1dWrapper() + _build_simple_model_refl1d(refl1d_w) + refl1d_w.set_resolution_function(LinearSpline(q_knots, fwhm_knots)) + refl1d_r = refl1d_w.calculate(q, 'MyModel') + + assert_allclose(refnx_r, refl1d_r, rtol=0.25) + + +def test_pointwise_consistent_across_engines(): + """Pointwise resolution (sigma from sQz) is consistent across engines. + + Before the fix, refnx treated the sigma values as FWHM and so + under-smeared by ~2.355x relative to refl1d. + """ + q = np.geomspace(0.005, 0.3, 100) + + # sQz is the variance of Qz; sigma = sqrt(sQz) increases with q. The + # magnitude mirrors the LinearSpline test (whose FWHM knots become these + # sigma values) so both engines apply the same modest smearing. + qz = np.linspace(0.001, 0.5, 50) + r = np.ones_like(qz) # only kept for serialization round-trips + sigma = (0.02 * qz + 0.001) / SIGMA_TO_FWHM + sqz = sigma**2 + + refnx_w = RefnxWrapper() + _build_simple_model_refnx(refnx_w) + refnx_w.set_resolution_function(Pointwise([qz, r, sqz])) + refnx_r = refnx_w.calculate(q, 'MyModel') + + refl1d_w = Refl1dWrapper() + _build_simple_model_refl1d(refl1d_w) + refl1d_w.set_resolution_function(Pointwise([qz, r, sqz])) + refl1d_r = refl1d_w.calculate(q, 'MyModel') + + assert_allclose(refnx_r, refl1d_r, rtol=0.25) diff --git a/tests/model/test_model.py b/tests/model/test_model.py index b11149c1..a2dd46ea 100644 --- a/tests/model/test_model.py +++ b/tests/model/test_model.py @@ -45,8 +45,9 @@ def test_default(self): assert_equal(p.background.min, 0.0) assert_equal(p.background.max, np.inf) assert_equal(p.background.fixed, True) - assert p._resolution_function.smearing([1]) == 5.0 - assert p._resolution_function.smearing([100]) == 5.0 + sigma_to_fwhm = 2.0 * np.sqrt(2.0 * np.log(2.0)) + assert np.allclose(p._resolution_function.smearing([1]), 5.0 / 100.0 * 1.0 / sigma_to_fwhm) + assert np.allclose(p._resolution_function.smearing([100]), 5.0 / 100.0 * 100.0 / sigma_to_fwhm) def test_from_pars(self): m1 = Material(6.908, -0.278, 'Boron') @@ -81,8 +82,9 @@ def test_from_pars(self): assert_equal(mod.background.min, 0.0) assert_equal(mod.background.max, np.inf) assert_equal(mod.background.fixed, True) - assert mod._resolution_function.smearing([1]) == 2.0 - assert mod._resolution_function.smearing([100]) == 2.0 + sigma_to_fwhm = 2.0 * np.sqrt(2.0 * np.log(2.0)) + assert np.allclose(mod._resolution_function.smearing([1]), 2.0 / 100.0 * 1.0 / sigma_to_fwhm) + assert np.allclose(mod._resolution_function.smearing([100]), 2.0 / 100.0 * 100.0 / sigma_to_fwhm) def test_add_assemblies(self): m1 = Material(6.908, -0.278, 'Boron') @@ -525,7 +527,8 @@ def test_round_trip_preserves_resolution_function(self): d = model.as_dict() global_object.map._clear() restored = Model.from_dict(d) - assert restored._resolution_function.smearing(100) == 3.0 + sigma_to_fwhm = 2.0 * np.sqrt(2.0 * np.log(2.0)) + assert np.allclose(restored._resolution_function.smearing(100), 3.0 / 100.0 * 100.0 / sigma_to_fwhm) def test_round_trip_preserves_interface(self): global_object.map._clear() diff --git a/tests/model/test_resolution_functions.py b/tests/model/test_resolution_functions.py index b8a4ea18..8a1a5116 100644 --- a/tests/model/test_resolution_functions.py +++ b/tests/model/test_resolution_functions.py @@ -6,6 +6,7 @@ import numpy as np from easyreflectometry.model.resolution_functions import DEFAULT_RESOLUTION_FWHM_PERCENTAGE +from easyreflectometry.model.resolution_functions import SIGMA_TO_FWHM from easyreflectometry.model.resolution_functions import LinearSpline from easyreflectometry.model.resolution_functions import PercentageFwhm from easyreflectometry.model.resolution_functions import Pointwise @@ -17,21 +18,22 @@ def test_constructor(self): # When resolution_function = PercentageFwhm(1.0) - # Then Expect - assert np.all(resolution_function.smearing([0, 2.5]) == np.array([1.0, 1.0])) - assert resolution_function.smearing([-100]) == np.array([1.0]) - assert resolution_function.smearing([100]) == np.array([1.0]) + # Then Expect: smearing() returns sigma = (constant / 100) * q / SIGMA_TO_FWHM + expected = (1.0 / 100.0) * np.array([0.0, 2.5]) / SIGMA_TO_FWHM + assert np.allclose(resolution_function.smearing([0, 2.5]), expected) + assert np.allclose(resolution_function.smearing([-100]), (1.0 / 100.0) * (-100.0) / SIGMA_TO_FWHM) + assert np.allclose(resolution_function.smearing([100]), (1.0 / 100.0) * 100.0 / SIGMA_TO_FWHM) def test_constructor_none(self): # When resolution_function = PercentageFwhm() - # Then Expect - assert np.all( - resolution_function.smearing([0, 2.5]) == [DEFAULT_RESOLUTION_FWHM_PERCENTAGE, DEFAULT_RESOLUTION_FWHM_PERCENTAGE] - ) - assert resolution_function.smearing([-100]) == DEFAULT_RESOLUTION_FWHM_PERCENTAGE - assert resolution_function.smearing([100]) == DEFAULT_RESOLUTION_FWHM_PERCENTAGE + # Then Expect: defaults to DEFAULT_RESOLUTION_FWHM_PERCENTAGE, returned as sigma + c = DEFAULT_RESOLUTION_FWHM_PERCENTAGE + expected = (c / 100.0) * np.array([0.0, 2.5]) / SIGMA_TO_FWHM + assert np.allclose(resolution_function.smearing([0, 2.5]), expected) + assert np.allclose(resolution_function.smearing([-100]), (c / 100.0) * (-100.0) / SIGMA_TO_FWHM) + assert np.allclose(resolution_function.smearing([100]), (c / 100.0) * 100.0 / SIGMA_TO_FWHM) def test_as_dict(self): # When @@ -57,10 +59,10 @@ def test_constructor(self): # When resolution_function = LinearSpline(q_data_points=[0, 10], fwhm_values=[5, 10]) - # Then Expect - assert np.all(resolution_function.smearing([0, 2.5]) == np.array([5, 6.25])) - assert resolution_function.smearing([-100]) == np.array([5.0]) - assert resolution_function.smearing([100]) == np.array([10.0]) + # Then Expect: smearing() returns sigma (FWHM knots converted to sigma) + assert np.allclose(resolution_function.smearing([0, 2.5]), np.array([5, 6.25]) / SIGMA_TO_FWHM) + assert np.allclose(resolution_function.smearing([-100]), np.array([5.0]) / SIGMA_TO_FWHM) + assert np.allclose(resolution_function.smearing([100]), np.array([10.0]) / SIGMA_TO_FWHM) def test_as_dict(self): # When From 2ae07f528d9bc91c821b507c98f458f38e5c9d53 Mon Sep 17 00:00:00 2001 From: rozyczko Date: Tue, 16 Jun 2026 08:50:53 +0200 Subject: [PATCH 2/2] disable automatic parallelization for windows --- .github/workflows/test.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6bf99c45..acd12e38 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -293,8 +293,11 @@ jobs: cd easyreflectometry_py$py_ver echo "Running tests" - pixi run python -m pytest ../tests/integration/ --color=yes -n auto -v ${{ needs.env-prepare.outputs.pytest-marks }} - + if [[ "${{ matrix.os }}" == "windows-2022" ]]; then + pixi run python -m pytest ../tests/integration/ --color=yes -v ${{ needs.env-prepare.outputs.pytest-marks }} + else + pixi run python -m pytest ../tests/integration/ --color=yes -n auto -v ${{ needs.env-prepare.outputs.pytest-marks }} + fi echo "Exiting pixi project directory" cd .. done