Vendor Eigen instead of letting lwtnn configure its own

.github/workflows/wheels.yml

@@ -59,6 +59,17 @@ jobs:
         env:
           CIBW_ARCHS: ${{ matrix.arch }}
           MACOSX_DEPLOYMENT_TARGET: 11.0
+          CORRECTIONLIB_ULP_REPORT: ${{ github.workspace }}/ulp-report.md
+
+      # cibuildwheel overwrites GITHUB_STEP_SUMMARY with its own build table once it
+      # finishes, so the tests collect this separately and it gets appended after
+      - name: Report float assertion ULPs
+        if: always()
+        shell: bash
+        run: |
+          if [ -f ulp-report.md ]; then
+            cat ulp-report.md >> "$GITHUB_STEP_SUMMARY"
+          fi
 
       - name: Upload wheels
         uses: actions/upload-artifact@v7

.gitmodules

@@ -16,3 +16,7 @@
 [submodule "lwtnn"]
 	path = lwtnn
 	url = https://github.com/lwtnn/lwtnn.git
+[submodule "eigen"]
+	path = eigen
+	url = https://gitlab.com/libeigen/eigen.git
+	shallow = true

CMakeLists.txt

@@ -40,14 +40,32 @@ set(THREADS_PREFER_PTHREAD_FLAG ON)
 find_package(Threads)
 find_package(ZLIB)
 
-# Configure lwtnn to build static library only and download dependencies
+# Configure lwtnn to build static library only and download dependencies.
+# Eigen comes from the vendored copy rather than from lwtnn's BUILTIN_EIGEN, which
+# configures Eigen's own CMake project and so fails on systems that have MPFR but no
+# GMP development headers. Eigen is header only, so pointing lwtnn's FindEigen3 at
+# the headers is all it takes; override to build against another copy, e.g.
+# -DEIGEN3_INCLUDE_DIR=/usr/include/eigen3
+set(EIGEN3_INCLUDE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/eigen"
+  CACHE PATH "Directory holding the Eigen headers to build the bundled lwtnn against")
 set(BUILD_STATIC_LIBRARY ON)
 set(BUILD_TESTING OFF)
 set(BUILTIN_BOOST ON)
-set(BUILTIN_EIGEN ON)
+set(BUILTIN_EIGEN OFF)
 set(CMAKE_POSITION_INDEPENDENT_CODE ON)
 add_subdirectory(lwtnn EXCLUDE_FROM_ALL)
 
+# lwtnn drops EIGEN3_INCLUDE_DIR verbatim into its targets' usage requirements, and
+# its install(EXPORT) rules reject an absolute in-tree path there. lwtnn does its own
+# BUILD_INTERFACE wrapping in the BUILTIN_EIGEN branch; do the same for our copy.
+foreach(lwtnn_target IN ITEMS lwtnn lwtnn-stat)
+  get_target_property(lwtnn_include_dirs ${lwtnn_target} INTERFACE_INCLUDE_DIRECTORIES)
+  string(REPLACE "${EIGEN3_INCLUDE_DIR}" "$<BUILD_INTERFACE:${EIGEN3_INCLUDE_DIR}>"
+    lwtnn_include_dirs "${lwtnn_include_dirs}")
+  set_target_properties(${lwtnn_target} PROPERTIES
+    INTERFACE_INCLUDE_DIRECTORIES "${lwtnn_include_dirs}")
+endforeach()
+
 
 configure_file(include/version.h.in ${CMAKE_CURRENT_BINARY_DIR}/include/correctionlib_version.h)
 install(FILES ${CMAKE_CURRENT_BINARY_DIR}/include/correctionlib_version.h DESTINATION ${PKG_INSTALL}/include)

pyproject.toml

@@ -147,7 +147,10 @@ Homepage = "https://github.com/cms-nanoAOD/correctionlib"
 [tool.scikit-build]
 metadata.version.provider = "scikit_build_core.metadata.setuptools_scm"
 cmake.version = ">=3.11.0"
-sdist.include = ["src/correctionlib/version.py"]
+# only Eigen/ is needed to build lwtnn; the rest of the Eigen checkout (tests,
+# docs, demos, unsupported/) would roughly triple the sdist for nothing
+sdist.exclude = ["eigen/**"]
+sdist.include = ["src/correctionlib/version.py", "eigen/Eigen/**"]
 
 [tool.scikit-build.cmake.define]
 BUILD_DEMO = "OFF"
@@ -159,11 +162,16 @@ write_to = "src/correctionlib/version.py"
 skip = ["cp3{9,10}*-win_arm64"]
 test-groups = ["test"]
 test-command = "python -m pytest {package}/tests"
-test-skip = ["*-musllinux_*", "cp3{10,11,12}-win32", "cp*-win_arm64", "cp*-manylinux_aarch64"]
+test-skip = ["*-musllinux_*", "cp3{10,11,12}-win32", "cp*-win_arm64"]
 
 [tool.cibuildwheel.environment]
 # this makes sure that we build only on platforms that have a corresponding numpy wheel
 PIP_ONLY_BINARY = ":all:"
 
+[tool.cibuildwheel.linux]
+# the linux tests run in a container, so the ULP report path has to be handed in
+# explicitly; the other platforms see the runner environment already
+environment-pass = ["CORRECTIONLIB_ULP_REPORT"]
+
 [tool.codespell]
 ignore-regex = "[A-Za-z0-9+/]{100,}"  # ignore long base64 strings

tests/conftest.py

@@ -0,0 +1,138 @@
+"""Reports how far our approximate float assertions actually land from their reference.
+
+The wheel builds run this suite on every architecture we ship, so the summary printed
+at the end of a run is a per-architecture record of the floating point spread, readable
+straight out of the Actions log. See #348, where an aarch64 build differed from the
+reference by a single ULP and the whole suite got skipped on that platform in response.
+
+Set CORRECTIONLIB_ULP_REPORT to also collect the report as markdown, which is how
+wheels.yml lifts it into the GitHub Actions job summary. It cannot be written to
+GITHUB_STEP_SUMMARY directly from here: cibuildwheel overwrites that file once its
+own build table is ready, which is after every test run has finished.
+"""
+
+import os
+import pathlib
+import platform
+import struct
+import sys
+import sysconfig
+from collections import defaultdict
+
+import pytest
+
+
+def _ordinal(value: float) -> int:
+    """Map a double onto an integer such that adjacent doubles are adjacent integers."""
+    (bits,) = struct.unpack("<q", struct.pack("<d", value))
+    # negative doubles run backwards when read as signed integers; reflect them so
+    # that the ordering is monotonic across zero
+    return bits if bits >= 0 else -0x8000000000000000 - bits
+
+
+def ulp_distance(actual: float, expected: float) -> float:
+    """Count the representable doubles between two floats."""
+    if actual != actual or expected != expected:  # NaN
+        return float("inf")
+    if actual in (float("inf"), float("-inf")) or expected in (
+        float("inf"),
+        float("-inf"),
+    ):
+        return float("inf")
+    if actual == expected:
+        return 0
+    try:
+        return abs(_ordinal(actual) - _ordinal(expected))
+    except (OverflowError, struct.error):
+        return float("inf")
+
+
+_measurements: "defaultdict[str, list[tuple[float, float, float]]]" = defaultdict(list)
+
+
+@pytest.fixture
+def ulp_report(request):
+    """Assert approximate equality, recording the ULP distance for the run summary.
+
+    Takes the same keyword arguments as pytest.approx; pass ``label`` to name the
+    measurement when a test makes more than one.
+    """
+
+    def check(actual, expected, label=None, **approx_kwargs):
+        name = request.node.name
+        if label is not None:
+            name = f"{name}[{label}]"
+        _measurements[name].append((actual, expected, ulp_distance(actual, expected)))
+        assert actual == pytest.approx(expected, **approx_kwargs)
+
+    return check
+
+
+def _build_tag() -> str:
+    """Name this interpreter and platform the way the wheel it came from is named."""
+    return f"{sys.implementation.cache_tag}-{sysconfig.get_platform()}"
+
+
+def _markdown_report_path(host_mount="/host"):
+    """Where to collect the markdown report, if anywhere.
+
+    cibuildwheel runs the Linux tests inside a container, where the runner's
+    filesystem is mounted at /host, so a path handed to us by the workflow needs
+    translating before we can write to it.
+    """
+    target = os.environ.get("CORRECTIONLIB_ULP_REPORT")
+    if not target and not os.environ.get("CIBUILDWHEEL"):
+        # outside cibuildwheel nothing overwrites the job summary, so use it directly
+        target = os.environ.get("GITHUB_STEP_SUMMARY")
+    if not target:
+        return None
+    path = pathlib.Path(target)
+    if path.parent.is_dir():
+        return path
+    in_container = pathlib.Path(host_mount) / path.relative_to(path.anchor)
+    return in_container if in_container.parent.is_dir() else None
+
+
+def _write_markdown_report(path, rows):
+    header = "### Float assertion ULP report"
+    existing = path.read_text(encoding="utf-8") if path.exists() else ""
+    lines = []
+    if header not in existing:
+        lines += [
+            header,
+            "",
+            "| build | assertion | ulp | actual | expected |",
+            "| --- | --- | ---: | --- | --- |",
+        ]
+    build = _build_tag()
+    for name, ulps, actual, expected in rows:
+        lines.append(f"| `{build}` | {name} | {ulps} | `{actual!r}` | `{expected!r}` |")
+    with path.open("a", encoding="utf-8") as fp:
+        fp.write("\n".join(lines) + "\n")
+
+
+def pytest_terminal_summary(terminalreporter):
+    if not _measurements:
+        return
+    terminalreporter.write_sep("=", "float assertion ULP report")
+    terminalreporter.write_line(
+        f"{platform.machine()} {sys.platform} "
+        f"python {'.'.join(str(v) for v in sys.version_info[:3])}"
+        f"{'t' if sysconfig.get_config_var('Py_GIL_DISABLED') else ''}"
+    )
+    width = max(len(name) for name in _measurements)
+    rows = []
+    for name, measurements in sorted(_measurements.items()):
+        actual, expected, ulps = max(measurements, key=lambda m: m[2])
+        detail = "exact" if ulps == 0 else f"{actual!r} vs {expected!r}"
+        count = f" (worst of {len(measurements)})" if len(measurements) > 1 else ""
+        terminalreporter.write_line(f"{name:<{width}}  {ulps:>4} ulp  {detail}{count}")
+        rows.append((name, ulps, actual, expected))
+
+    path = _markdown_report_path()
+    if path is None:
+        return
+    try:
+        _write_markdown_report(path, rows)
+    except OSError as exc:  # never fail a test run over the report
+        terminalreporter.write_line(f"could not write ULP report to {path}: {exc}")

tests/test_core.py

@@ -141,7 +141,7 @@ def test_evaluator():
     platform.architecture() in {("32bit", "ELF"), ("32bit", "")},
     reason="cibuildwheel tests fail while building i686 wheels due to floating point rounding differences of order 1e-16",
 )
-def test_tformula():
+def test_tformula(ulp_report):
     def evaluate(expr, variables, parameters):
         cset = {
             "schema_version": 2,
@@ -431,11 +431,12 @@ def evaluate(expr, variables, parameters):
             * (math.log(x) / math.log(10) - v[5])
         )
         # the following shows a small numerical error: 1.2512381067949132 - 1.251238106794914 == -8e-16
-        assert evaluate(
-            "max(0.0001,[0]+[1]/(pow(log10(x),2)+[2])+[3]*exp(-1*([4]*((log10(x)-[5])*(log10(x)-[5])))))",
-            [x],
-            v,
-        ) == pytest.approx(
+        ulp_report(
+            evaluate(
+                "max(0.0001,[0]+[1]/(pow(log10(x),2)+[2])+[3]*exp(-1*([4]*((log10(x)-[5])*(log10(x)-[5])))))",
+                [x],
+                v,
+            ),
             max(
                 0.0001,
                 v[0]

tests/test_lwtnn.py

@@ -24,7 +24,7 @@ def test_lwtnn_bad_opaque():
         CorrectionSet.from_string(json.dumps(data))
 
 
-def test_lwtnn_example():
+def test_lwtnn_example(ulp_report):
     cset = CorrectionSet.from_file(str(LWTNN_TEST_FIXTURE))
     corr = cset["electron_fastsim_sf"]
 
@@ -38,4 +38,7 @@ def test_lwtnn_example():
         gen_phi,
         gen_iso,
     )
-    assert sf == 0.95186825355646787
+    # lwtnn's Eigen kernels sum in an order that depends on the vector ISA and the
+    # Eigen version, so the last couple of ULPs are not reproducible across
+    # platforms -- an aarch64 wheel build differed by one ULP here, see #348.
+    ulp_report(sf, 0.95186825355646787, rel=1e-12)

tests/test_ndpolyfit.py

@@ -1,10 +1,9 @@
 import numpy as np
-import pytest
 
 from correctionlib import convert
 
 
-def test_ndpoly():
+def test_ndpoly(ulp_report):
     corr, _ = convert.ndpolyfit(
         points=[np.array([0.0, 1.0, 0.0, 1.0]), np.array([10.0, 20.0, 10.0, 20.0])],
         values=np.array([0.9, 0.95, 0.94, 0.98]),
@@ -13,4 +12,4 @@ def test_ndpoly():
         degree=(1, 1),
     )
     ceval = corr.to_evaluator()
-    assert ceval.evaluate(0.2, 13.0) == pytest.approx(1.0801881480751705)
+    ulp_report(ceval.evaluate(0.2, 13.0), 1.0801881480751705)