From 78d6e2202972a996cf4791dedcb8d497f825a7c2 Mon Sep 17 00:00:00 2001 From: John Travers Date: Fri, 13 Nov 2026 17:33:52 +0000 Subject: [PATCH 1/2] Bound the radial expansion's polarisation-column comparison by rounding Both polarisation columns of the cumulative phase come from identical stored terms (now asserted exactly), but delta_phi contracts them in one np.tensordot, and the BLAS kernel may round each output column differently: on an AVX-512 build with OpenBLAS 7 and 19 of 1536 elements differed in the last bit. Compare the columns within 4 gamma_n(u) of sum |w| |t| per element (testing.md section 3, class R). The bound was spot-checked: a 1e-13 relative perturbation of one column fails it by 12 to 17 times. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XGENPwwkUUG9KSAwneHjgH --- tests/translate/test_radial.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/translate/test_radial.py b/tests/translate/test_radial.py index 5003687a0..169ce40ac 100644 --- a/tests/translate/test_radial.py +++ b/tests/translate/test_radial.py @@ -12,6 +12,7 @@ from numpy.testing import assert_allclose from scipy.integrate import quad_vec +from bounds import assert_within from hankel_oracles import gamma from solprop.constants import C from solprop.exceptions import TranslationError @@ -107,6 +108,10 @@ def test_varying_radial_medium_uses_cumulative_expansion(case, comoving): assert isinstance(prefactor, ParameterExpansion) assert operator.phi_expansion.parameters == ("density",) assert prefactor.parameters == ("density",) + # Structural: one material sample serves both polarisations, so the stored + # terms are the same numbers; only their contraction below may round apart. + terms = operator.phi_expansion.terms + np.testing.assert_array_equal(terms[..., 0, :], terms[..., 1, :]) for left, right in ( (0.0, grid.length), (0.21 * grid.length, 0.79 * grid.length), @@ -135,7 +140,22 @@ def test_varying_radial_medium_uses_cumulative_expansion(case, comoving): ) actual = delta_phi(operator, left, right)[0] assert_allclose(actual[:, 0], expected, rtol=0, atol=bound) - np.testing.assert_array_equal(actual[:, 0], actual[:, 1]) + # The polarisation columns hold identical terms, but the increment is one + # BLAS product (np.tensordot) whose kernel may round each output column + # differently (seen with OpenBLAS on AVX-512). Each column is a length-n + # sum of real weight times term: within gamma_n(u) sum |w| |t| of exact per + # component, so two columns differ by at most twice that per component and + # sqrt(2) times more in modulus; gamma(2 n) = 4 gamma_n(u) covers it. + expansion = operator.phi_expansion + weights = expansion.weights(right) - expansion.weights(left) + magnitude = np.tensordot(np.abs(weights), np.abs(expansion.terms), axes=1) + assert_within( + actual[:, 1], + actual[:, 0], + gamma(2 * weights.size) * magnitude[0][:, 0], + elementwise=True, + what="polarisation columns", + ) for z in (0.0, 0.37 * grid.length, grid.length): expected = model.prefactor(space.k_r**2, z) * grid.crop_scale assert_allclose( From 19c33be6661abfe55801ea5a5faef4798b7a0d6d Mon Sep 17 00:00:00 2001 From: John Travers Date: Fri, 13 Nov 2026 17:35:41 +0000 Subject: [PATCH 2/2] Make the CI device change filter safe under pipefail The device filter piped grep -vE into grep -qE. On a long change list with the native match first, grep -q exits at that match, the upstream grep dies of SIGPIPE, and under pipefail the negated pipeline sets device=false despite a native change. The second grep now reads the whole pipe and discards its output. The current step runs as bash -e, so this hardens against a shell-policy change rather than fixing a skipped lane. tests/test_ci_changes.py runs the workflow's own step under bash -e and bash -e -o pipefail: a 100 000-line list with the native match first (fails under pipefail before this change), and the existing positive and negative classifications, including an unknown base. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XGENPwwkUUG9KSAwneHjgH --- .github/workflows/ci.yml | 4 ++- tests/test_ci_changes.py | 66 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 tests/test_ci_changes.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 21ab85691..b1161dbec 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -79,8 +79,10 @@ jobs: fi echo "lanes=$lanes" | tee -a "$GITHUB_OUTPUT" # Whether a device build could have changed; without a diff to read, it could. + # The second grep reads the whole pipe: grep -q stops at its first match, the + # first grep then dies of SIGPIPE, and under pipefail that reads as no match. device=true - if [ -f changed.txt ] && ! grep -vE '\.md$' changed.txt | grep -qE '^native/|^\.github/workflows/ci\.yml$'; then + if [ -f changed.txt ] && ! grep -vE '\.md$' changed.txt | grep -E '^native/|^\.github/workflows/ci\.yml$' > /dev/null; then device=false fi echo "device=$device" | tee -a "$GITHUB_OUTPUT" diff --git a/tests/test_ci_changes.py b/tests/test_ci_changes.py new file mode 100644 index 000000000..32e20322b --- /dev/null +++ b/tests/test_ci_changes.py @@ -0,0 +1,66 @@ +# SPDX-FileCopyrightText: 2026 John C. Travers and contributors +# +# SPDX-License-Identifier: GPL-3.0-or-later + +"""The CI change filter that decides whether the device compilation lanes run.""" + +import os +import subprocess +import sys +import textwrap +from pathlib import Path + +import pytest + +WORKFLOW = Path(__file__).resolve().parents[1] / ".github/workflows/ci.yml" +LAST = ' echo "device=$device" | tee -a "$GITHUB_OUTPUT"' + +pytestmark = pytest.mark.skipif( + sys.platform == "win32", reason="the CI change filter runs in Linux Bash" +) + + +def _device(directory: Path, changed: str | None, *, pipefail: bool) -> str: + """Run the workflow's device filter on a change list; return what it writes.""" + workflow = WORKFLOW.read_text(encoding="utf-8") + start = workflow.index(" device=true\n") + end = workflow.index(LAST, start) + len(LAST) + if changed is not None: + (directory / "changed.txt").write_text(changed, encoding="utf-8") + output = directory / "github-output" + shell = ["bash", "-e", *(("-o", "pipefail") if pipefail else ())] + subprocess.run( + [*shell, "-c", textwrap.dedent(workflow[start:end])], + cwd=directory, + env={**os.environ, "GITHUB_OUTPUT": str(output)}, + check=True, + capture_output=True, + ) + return output.read_text(encoding="utf-8") + + +@pytest.mark.parametrize("pipefail", [False, True]) +def test_device_filter_selects_native_change_first_in_large_list(tmp_path, pipefail): + # The match comes first and the list is long enough that a reader stopping at it + # leaves the upstream grep writing into a closed pipe (SIGPIPE). + changed = "native/src/core.cpp\n" + "".join( + f"src/other/{index:08d}.py\n" for index in range(100_000) + ) + assert _device(tmp_path, changed, pipefail=pipefail) == "device=true\n" + + +@pytest.mark.parametrize("pipefail", [False, True]) +@pytest.mark.parametrize( + "changed,expected", + [ + ("native/src/core.cpp\n", "true"), + ("docs/index.md\n.github/workflows/ci.yml\n", "true"), + (None, "true"), + ("native/README.md\ndocs/dev/testing.md\n", "false"), + ("src/solprop/grid.py\ngui/app.py\n", "false"), + ], +) +def test_device_filter_classifies_change_lists(tmp_path, changed, expected, pipefail): + # None is a push whose base is unknown: no list to read, so a device build may + # have changed. + assert _device(tmp_path, changed, pipefail=pipefail) == f"device={expected}\n"