diff --git a/.github/workflows/code.yml b/.github/workflows/code.yml index 4ea4421a6..be29098be 100644 --- a/.github/workflows/code.yml +++ b/.github/workflows/code.yml @@ -91,8 +91,8 @@ jobs: - name: Cached Python ${{ matrix.python-version }} environment uses: mamba-org/setup-micromamba@v3 with: - cache-environment: true - cache-environment-key: env-key-${{ matrix.python-version }} + cache-environment: ${{ matrix.experimental != true }} + cache-environment-key: ${{ matrix.experimental != true && format('env-key-{0}', matrix.python-version) || '' }} condarc: | channels: - conda-forge diff --git a/.gitignore b/.gitignore index 9dc467792..9c7575d27 100644 --- a/.gitignore +++ b/.gitignore @@ -87,6 +87,7 @@ docs/source/_build/ # local developer files dev_* .dev/ +.nogit_* # RE.md saved by local bluesky session .re_md_dict.yml diff --git a/RELEASE_NOTES.rst b/RELEASE_NOTES.rst index cfe44d93a..1dfe29aea 100644 --- a/RELEASE_NOTES.rst +++ b/RELEASE_NOTES.rst @@ -30,6 +30,11 @@ describe future plans. Release expected 2026-Q4. + Fixes + ----- + + * Preserve full precision during forward-solution constraint checks. (:issue:`427`) + 1.0.0 ##### diff --git a/docs/source/guides/how_forward_solution.rst b/docs/source/guides/how_forward_solution.rst index 76cccd4a5..0b103548c 100644 --- a/docs/source/guides/how_forward_solution.rst +++ b/docs/source/guides/how_forward_solution.rst @@ -76,13 +76,13 @@ Two pickers are provided: * - Function - Behavior - * - :func:`~hklpy2.misc.pick_first_solution` *(default)* + * - :func:`~hklpy2.utils.pick_first_solution` *(default)* - Returns the first solution in the list as supplied by the solver. - * - :func:`~hklpy2.misc.pick_closest_solution` + * - :func:`~hklpy2.utils.pick_closest_solution` - Returns the solution whose real-axis values are closest (RMS distance) to the current motor positions. -The default is :func:`~hklpy2.misc.pick_first_solution`. +The default is :func:`~hklpy2.utils.pick_first_solution`. Check the current picker ------------------------- @@ -97,7 +97,7 @@ Switch to pick_closest_solution at runtime Assign directly to :attr:`~hklpy2.diffract.DiffractometerBase._forward_solution`:: - >>> from hklpy2.misc import pick_closest_solution + >>> from hklpy2.utils import pick_closest_solution >>> e4cv._forward_solution = pick_closest_solution Switch back:: @@ -114,13 +114,13 @@ Pass ``forward_solution_function`` as a dotted name string to >>> import hklpy2 >>> e4cv = hklpy2.creator( ... name="e4cv", - ... forward_solution_function="hklpy2.misc.pick_closest_solution", + ... forward_solution_function="hklpy2.utils.pick_closest_solution", ... ) Or pass a callable directly to :class:`~hklpy2.diffract.DiffractometerBase.__init__` when subclassing:: - >>> from hklpy2.misc import pick_closest_solution + >>> from hklpy2.utils import pick_closest_solution >>> e4cv = MyDiffractometerClass( ... "", ... name="e4cv", @@ -171,8 +171,8 @@ All pickers must follow this interface: .. seealso:: - :func:`~hklpy2.misc.pick_first_solution`, - :func:`~hklpy2.misc.pick_closest_solution` — built-in pickers. + :func:`~hklpy2.utils.pick_first_solution`, + :func:`~hklpy2.utils.pick_closest_solution` — built-in pickers. :attr:`~hklpy2.diffract.DiffractometerBase._forward_solution` — the attribute that holds the active picker. diff --git a/src/hklpy2/backends/hkl_soleil.py b/src/hklpy2/backends/hkl_soleil.py index b54459675..7d48b8d4f 100644 --- a/src/hklpy2/backends/hkl_soleil.py +++ b/src/hklpy2/backends/hkl_soleil.py @@ -444,7 +444,15 @@ def extras(self, values: NamedFloatDict) -> None: self.engine.parameter_set(k, p) def forward(self, pseudos: NamedFloatDict) -> list[NamedFloatDict]: - """Compute list of solutions(reals) from pseudos (hkl -> [angles]).""" + """ + Compute list of solutions(reals) from pseudos (hkl -> [angles]). + + The underlying geometry engine returns a bounded set of equivalent + solutions, ordered using the current real-axis geometry. The returned + order can therefore change when ``set_reals()`` is given a different + starting position. This method preserves that order making no + assumption about the selection of a specific solution from the set. + """ from gi.repository import GLib # W0611 logger.debug("(%r) forward(%r)", __name__, pseudos) @@ -462,10 +470,7 @@ def forward(self, pseudos: NamedFloatDict) -> list[NamedFloatDict]: for glist_item in raw: geo = glist_item.geometry_get() sol = dict( - zip( - geo.axis_names_get(), - roundoff_list(geo.axis_values_get(LIBHKL_USER_UNITS)), - ) + zip(geo.axis_names_get(), geo.axis_values_get(LIBHKL_USER_UNITS)) ) solutions.append(sol) return solutions diff --git a/src/hklpy2/backends/tests/test_hkl_soleil.py b/src/hklpy2/backends/tests/test_hkl_soleil.py index a6e16680f..8ae85d32d 100644 --- a/src/hklpy2/backends/tests/test_hkl_soleil.py +++ b/src/hklpy2/backends/tests/test_hkl_soleil.py @@ -60,6 +60,169 @@ def test_hkl_soleil(): hkl_soleil.to_hkl([1, 2]) +APS_POLAR_R110 = { + "name": "543d0a3", + "pseudos": {"h": 1.0, "k": 1.0, "l": 0.0}, + "reals": { + "tau": 0.0, + "mu": 9.89806867221, + "chi": 113.105327243248, + "phi": 0.00173136532, + "gamma": 9.42401385126, + "delta": -0.00013200263, + }, + "wavelength": 0.6199478461064903, +} +APS_POLAR_R004 = { + "name": "88e3c3d", + "pseudos": {"h": 0.0, "k": 0.0, "l": 4.0}, + "reals": { + "tau": 0.0, + "mu": 10.848157453245, + "chi": 30.583556860225, + "phi": 0.00038517158, + "gamma": 10.38248391045, + "delta": -0.00011434739, + }, + "wavelength": 0.6199478461064903, +} + + +@pytest.mark.parametrize( + "parms, context", + [ + pytest.param( + { + "reals": { + "tau": 0.0, + "mu": 9.89806867221, + "chi": 113.105327243248, + "phi": 0.00173136532, + "gamma": 9.42401385126, + "delta": 0.0, + }, + "hkl": {"h": 1.0, "k": 1.0, "l": 0.0}, + "expected": [ + { + "tau": 0.0, + "mu": 9.8670, + "chi": 113.1061, + "phi": 0.0017, + "gamma": 9.3620, + "delta": 0.0, + }, + { + "tau": 0.0, + "mu": -9.8670, + "chi": -66.8939, + "phi": 0.0017, + "gamma": -9.3620, + "delta": 0.0, + }, + ], + }, + does_not_raise(), + id="reported-110-position", + ), + pytest.param( + { + "reals": { + "tau": 0.0, + "mu": 9.89806867221, + "chi": 31.0, + "phi": 0.00173136532, + "gamma": 9.42401385126, + "delta": 0.0, + }, + "hkl": {"h": 1.0, "k": 1.0, "l": 0.0}, + "expected": [ + { + "tau": 0.0, + "mu": 179.4950, + "chi": -66.8939, + "phi": 0.0017, + "gamma": 9.3620, + "delta": 0.0, + }, + { + "tau": 0.0, + "mu": -179.4950, + "chi": 113.1061, + "phi": 0.0017, + "gamma": -9.3620, + "delta": 0.0, + }, + ], + }, + does_not_raise(), + id="reported-110-axes-with-chi-31", + ), + pytest.param( + { + "reals": { + "tau": 0.0, + "mu": 9.89806867221, + "chi": 125.0, + "phi": 0.00173136532, + "gamma": 9.42401385126, + "delta": 0.0, + }, + "hkl": {"h": 0.0, "k": 0.0, "l": 6.0}, + "expected": [ + { + "tau": 0.0, + "mu": 177.9117, + "chi": 30.5462, + "phi": 0.0017, + "gamma": -15.4822, + "delta": 0.0, + }, + { + "tau": 0.0, + "mu": -177.9117, + "chi": -149.4538, + "phi": 0.0017, + "gamma": 15.4822, + "delta": 0.0, + }, + ], + }, + does_not_raise(), + id="reported-110-axes-with-chi-125-for-006", + ), + ], +) +def test_aps_polar_forward_depends_on_start_position(parms, context): + """The backend returns two solutions with position-dependent ordering.""" + with context: + solver = hkl_soleil.HklSolver("APS POLAR", engine="hkl") + solver.mode = "4-circles constant phi horizontal" + solver.wavelength = 0.6199478461064903 + solver.sample = { + "name": "pr4310-exafs-cube1", + "lattice": { + "a": 5.38, + "b": 5.467, + "c": 14.066, + "alpha": 90.0, + "beta": 101.0, + "gamma": 90.0, + }, + "order": [], + "reflections": [], + } + solver.calculate_UB(APS_POLAR_R110, APS_POLAR_R004) + solver.set_reals(parms["reals"]) + solutions = solver.forward(parms.get("hkl", {"h": 1.0, "k": 1.0, "l": 0.0})) + + assert len(solutions) == len(parms["expected"]) + for actual, expected in zip(solutions, parms["expected"]): + assert actual.keys() == expected.keys() + np.testing.assert_allclose( + list(actual.values()), list(expected.values()), atol=0.001 + ) + + def test_HklSolver(): solver = hkl_soleil.HklSolver(geometry="E4CV", engine="hkl") assert solver.wavelength == 1.54 diff --git a/src/hklpy2/ops.py b/src/hklpy2/ops.py index f22512d40..2333252d6 100644 --- a/src/hklpy2/ops.py +++ b/src/hklpy2/ops.py @@ -40,6 +40,7 @@ from .utils import _SolverDirty from .utils import axes_to_dict from .utils import convert_units +from .utils import roundoff from .utils import unique_name __all__ = ["Core"] @@ -727,7 +728,11 @@ def forward(self, pseudos: AnyAxesType, wavelength: float | None = None) -> list reals[name] = constraint.apply_cut(reals[name]) if self.constraints.valid(**reals): - solutions.append(self.diffractometer.RealPosition(**reals)) + user_reals = { + axis: roundoff(value, self.diffractometer.digits) + for axis, value in reals.items() + } + solutions.append(self.diffractometer.RealPosition(**user_reals)) else: # Log which constraint(s) rejected this solution. for name, constraint in self.constraints.items(): diff --git a/src/hklpy2/tests/test_i427.py b/src/hklpy2/tests/test_i427.py new file mode 100644 index 000000000..2e1a1b0e1 --- /dev/null +++ b/src/hklpy2/tests/test_i427.py @@ -0,0 +1,176 @@ +# Copyright (c) 2023-2026 UChicago Argonne, LLC +# SPDX-License-Identifier: LicenseRef-UChicago-Argonne-LLC-License +"""Regression test for issue #427.""" + +import logging +import re +from contextlib import nullcontext as does_not_raise + +import pytest + +from ..diffract import creator +from ..exceptions import NoForwardSolutions + + +@pytest.fixture() +def aps_polar(): + """Create the APS POLAR configuration reported in issue #427.""" + sim = creator( + name="probe", + geometry="APS POLAR", + reals={ + "tau": None, + "mu": None, + "chi": None, + "phi": None, + "gamma": None, + "delta": None, + }, + ) + sim.add_sample("pr4310-exafs-cube1", 5.38, b=5.467, c=14.066, beta=101.0) + wavelength = 0.6199478461064903 + sim.beam.wavelength.put(wavelength) + r110 = sim.add_reflection( + (1, 1, 0), + { + "tau": 0.0, + "mu": 9.89806867221, + "chi": 113.105327243248, + "phi": 0.00173136532, + "gamma": 9.42401385126, + "delta": -0.00013200263, + }, + wavelength=wavelength, + name="543d0a3", + ) + r004 = sim.add_reflection( + (0, 0, 4), + { + "tau": 0.0, + "mu": 10.848157453245, + "chi": 30.583556860225, + "phi": 0.00038517158, + "gamma": 10.38248391045, + "delta": -0.00011434739, + }, + wavelength=wavelength, + name="88e3c3d", + ) + sim.core.calc_UB(r110, r004) + sim.core.mode = "4-circles constant phi horizontal" + sim.core.presets = {"delta": 0.0} + return sim + + +@pytest.mark.parametrize( + "parms, context", + [ + pytest.param( + {"chi": 0.0, "expected": (-66.8937, -9.3620)}, + does_not_raise(), + id="current chi zero selects negative solution", + ), + pytest.param( + {"chi": 30.5462, "expected": (113.1063, 9.3620)}, + does_not_raise(), + id="current chi near 004 selects positive solution", + ), + ], +) +def test_forward_solution_changes_with_current_chi(aps_polar, parms, context): + """The reported APS POLAR setup returns different first solutions.""" + with context: + aps_polar.chi.move(parms["chi"]) + solutions = aps_polar.core.forward({"h": 1, "k": 1, "l": 0}) + selected = aps_polar.forward({"h": 1, "k": 1, "l": 0}) + + assert len(solutions) == 2 + assert (selected.chi, selected.gamma) == pytest.approx( + parms["expected"], abs=0.001 + ) + assert selected == solutions[0] + + +@pytest.mark.parametrize( + "parms, context", + [ + pytest.param( + {"chi_limits": (112.0, 114.0), "expected": (113.1063, 9.3620)}, + does_not_raise(), + id="tight chi limits retain positive solution", + ), + pytest.param( + {"chi_limits": (-68.0, -66.0), "expected": (-66.8937, -9.3620)}, + does_not_raise(), + id="tight chi limits retain negative solution", + ), + pytest.param( + {"chi_limits": (113.0, 113.0)}, + pytest.raises(NoForwardSolutions, match=re.escape("No solutions.")), + id="value above high limit is rejected before rounding", + ), + pytest.param( + {"chi_limits": (-66.893, -66.8)}, + pytest.raises(NoForwardSolutions, match=re.escape("No solutions.")), + id="value below low limit is rejected before rounding", + ), + pytest.param( + {"digits": 2, "chi": 30.5462, "expected": (113.11, 9.36)}, + does_not_raise(), + id="accepted result uses configured digits", + ), + ], +) +def test_forward_tight_chi_limits_select_expected_solution(aps_polar, parms, context): + """Tight chi limits retain the corresponding reported solution.""" + with context: + if "digits" in parms: + aps_polar.digits = parms["digits"] + aps_polar.core.constraints["chi"].limits = parms.get("chi_limits", (-180, 180)) + if "chi" in parms: + aps_polar.chi.move(parms["chi"]) + solutions = aps_polar.core.forward({"h": 1, "k": 1, "l": 0}) + selected = aps_polar.forward({"h": 1, "k": 1, "l": 0}) + + if "expected" in parms: + assert len(solutions) == (2 if "digits" in parms else 1) + assert (selected.chi, selected.gamma) == pytest.approx( + parms["expected"], abs=0.001 + ) + assert selected == solutions[0] + + +@pytest.mark.parametrize( + "parms, context", + [ + pytest.param( + {"chi": 125.0, "gamma_limits": (-180.0, 180.0), "expected_count": 2}, + does_not_raise(), + id="all gamma solutions survive", + ), + pytest.param( + {"chi": 125.0, "gamma_limits": (0.0, 180.0), "expected_count": 1}, + does_not_raise(), + id="positive gamma solution survives", + ), + ], +) +def test_forward_006_constraint_diagnostic(aps_polar, parms, context, caplog): + """Report raw and Core-filtered solutions for the user's chi=125 case.""" + with context: + aps_polar.chi.move(parms["chi"]) + aps_polar.core.constraints["gamma"].limits = parms["gamma_limits"] + with caplog.at_level(logging.INFO): + raw = aps_polar.core.solver.forward({"h": 0.0, "k": 0.0, "l": 6.0}) + filtered = aps_polar.core.forward({"h": 0.0, "k": 0.0, "l": 6.0}) + + assert len(raw) == 2 + assert len(filtered) == parms["expected_count"] + assert any(solution["gamma"] > 0 for solution in raw) + if parms["gamma_limits"] == (0.0, 180.0): + assert all(solution.gamma >= 0 for solution in filtered) + assert ( + "Solution discarded" in caplog.text + if parms["expected_count"] == 1 + else True + ) diff --git a/src/hklpy2/tests/test_user.py b/src/hklpy2/tests/test_user.py index e062701a1..b3a139f71 100644 --- a/src/hklpy2/tests/test_user.py +++ b/src/hklpy2/tests/test_user.py @@ -121,7 +121,7 @@ def test_cahkl_table(fourc, capsys): # use the default "main" sample and UB matrix PseudoTuple = namedtuple("PseudoTuple", ["h", "k", "l"]) - rlist = [PseudoTuple(1, 0, 0), PseudoTuple(0, 1, 0)] + rlist = [PseudoTuple(1.0, 0.0, 0.0), PseudoTuple(0.0, 1.0, 0.0)] cahkl_table(*rlist, digits=1) out, err = capsys.readouterr() assert len(err) == 0