diff --git a/roofit/doc/developers/roofit_hs3.md b/roofit/doc/developers/roofit_hs3.md index 5f718110a40fd..b3bd5c68da938 100644 --- a/roofit/doc/developers/roofit_hs3.md +++ b/roofit/doc/developers/roofit_hs3.md @@ -196,6 +196,49 @@ For more complicated cases where members are lists of elements, the methods `is_seq()`, `set_seq()`, `is_map()`, `set_map()` and ranged-based for-loops via `children()` might come in handy. +### Conformance testing + +The HS3 conformance test suite checks that RooFit imports and evaluates a +set of frozen HS3 files like the reference implementation. It consists of +two repositories: the fixtures in +[HS3TestSuite](https://github.com/hep-statistics-serialization-standard/HS3TestSuite) +and the Python runner in +[HS3TestSuiteCode](https://github.com/hep-statistics-serialization-standard/HS3TestSuiteCode). + +The suite is enabled with the CMake option `-Dtest_roofit_hs3testsuite=ON`, +which requires `-Dtesting=ON`, `-Dpyroot=ON` and the `jsonschema` Python +package. Both repositories are fetched at configuration time, so the option +requires network access; alternatively, point +`FETCHCONTENT_SOURCE_DIR_HS3TESTSUITE` and +`FETCHCONTENT_SOURCE_DIR_HS3TESTSUITECODE` to local checkouts. The option is +enabled in the ROOT CI. Run the suite with + +``` {.sh} + ctest -R roofit-hs3-hs3-suite --output-on-failure +``` + +Each check of each fixture is reported as a separate test case by +`roofit/hs3/test/test_hs3_suite.py`, which uses +`roofit/hs3/test/hs3testsuite_roofit_backend.py` to run the checks with +RooFit. + +Both repositories are pinned to explicit commits in +`roofit/hs3/test/CMakeLists.txt`. Checks that are known to fail with the +current RooFit are listed in +`roofit/hs3/test/hs3testsuite_expected_failures.txt`, one +`::` per line, together with a one-line reason. They are +run as expected failures: if one of them starts to pass, or if an entry +matches no check, the test fails, so that the list can only shrink. When you +fix a conformance issue in RooFit, remove the corresponding entries in the +same pull request. + +To bump the pinned commits, update `hs3testsuite_tag` and +`hs3testsuitecode_tag` in +`roofit/hs3/test/CMakeLists.txt`, re-run the suite and update the list of +expected failures accordingly. Make sure the runner and the fixtures are +compatible, and adapt the RooFit backend if the backend interface of the +runner has changed. + ### Contributing If you encounter a missing importer or exporter, please consider diff --git a/roofit/hs3/test/CMakeLists.txt b/roofit/hs3/test/CMakeLists.txt index 4a7b26b32d6c1..ad974c4bdb254 100644 --- a/roofit/hs3/test/CMakeLists.txt +++ b/roofit/hs3/test/CMakeLists.txt @@ -3,18 +3,44 @@ ROOT_ADD_GTEST(testRooFitHS3 testRooFitHS3.cxx LIBRARIES RooFitCore RooFit RooFi ROOT_ADD_GTEST(testJSONInterface testJSONInterface.cxx LIBRARIES RooFitHS3 Matrix) if(test_roofit_hs3testsuite) + # The HS3 conformance test suite is split into two repositories: the frozen fixtures + # (HS3TestSuite) and the Python runner (HS3TestSuiteCode). Both are pinned to explicit + # commits. See roofit/doc/developers/roofit_hs3.md ("Conformance testing") for how to + # bump them together with hs3testsuite_expected_failures.txt. + set(hs3testsuite_url https://github.com/hep-statistics-serialization-standard/HS3TestSuite.git) + set(hs3testsuite_tag 9f799b27f63b40d1ba96ff66d14d61787892606d) + set(hs3testsuitecode_url https://github.com/hep-statistics-serialization-standard/HS3TestSuiteCode.git) + set(hs3testsuitecode_tag 36aeb1c67b14dbbd9d9e9b79a4bad561c5e6dac7) + include(FetchContent) - FetchContent_Declare( - hs3testsuite - GIT_REPOSITORY https://github.com/hep-statistics-serialization-standard/HS3TestSuite.git - GIT_TAG b2e5f19023025d03b7d2239154a02e69f43a1374 - ) # suppress git interactive prompt so fetch fails fast if the repo is gone or unreachable set(ENV{GIT_TERMINAL_PROMPT} "0") - FetchContent_MakeAvailable(hs3testsuite) - ROOT_ADD_PYUNITTEST(hs3-suite test_hs3_suite.py - PYTHON_DEPS jsonschema + foreach(repo hs3testsuite hs3testsuitecode) + string(TOUPPER ${repo} repo_upper) + # Probe the network first if the sources still have to be downloaded, so that an + # offline configuration fails with an actionable message instead of a git error + # from the FetchContent sub-build. + if(NOT FETCHCONTENT_FULLY_DISCONNECTED AND NOT FETCHCONTENT_SOURCE_DIR_${repo_upper} + AND NOT EXISTS ${FETCHCONTENT_BASE_DIR}/${repo}-src/.git) + execute_process(COMMAND ${GIT_EXECUTABLE} ls-remote --exit-code ${${repo}_url} HEAD + RESULT_VARIABLE ls_remote_result OUTPUT_QUIET ERROR_QUIET TIMEOUT 60) + if(NOT ls_remote_result EQUAL 0) + message(FATAL_ERROR "-Dtest_roofit_hs3testsuite=ON requires network access at configuration " + "time, but ${${repo}_url} could not be reached. Configure with network " + "access, provide a local checkout of commit ${${repo}_tag} with " + "-DFETCHCONTENT_SOURCE_DIR_${repo_upper}=, or disable the HS3 test " + "suite with -Dtest_roofit_hs3testsuite=OFF.") + endif() + endif() + FetchContent_Declare(${repo} GIT_REPOSITORY ${${repo}_url} GIT_TAG ${${repo}_tag}) + endforeach() + FetchContent_MakeAvailable(hs3testsuite hs3testsuitecode) + + ROOT_ADD_PYUNITTEST(hs3-suite test_hs3_suite.py + PYTHON_DEPS jsonschema ENVIRONMENT HS3TESTSUITE_ROOT=${hs3testsuite_SOURCE_DIR} + HS3TESTSUITECODE_ROOT=${hs3testsuitecode_SOURCE_DIR} + HS3TESTSUITE_EXPECTED_FAILURES=${CMAKE_CURRENT_SOURCE_DIR}/hs3testsuite_expected_failures.txt COPY_TO_BUILDDIR hs3testsuite_roofit_backend.py ) endif() diff --git a/roofit/hs3/test/hs3testsuite_expected_failures.txt b/roofit/hs3/test/hs3testsuite_expected_failures.txt new file mode 100644 index 0000000000000..806b3217fcf56 --- /dev/null +++ b/roofit/hs3/test/hs3testsuite_expected_failures.txt @@ -0,0 +1,8 @@ +# Checks of the HS3 conformance test suite that are known to fail with the +# current RooFit. See roofit/doc/developers/roofit_hs3.md, "Conformance testing". +# +# Format: one check per line, :: # +# +# These checks are run as expected failures: if one of them starts to pass, or +# if an entry matches no check, the test fails. Remove entries in the same pull +# request that fixes them. diff --git a/roofit/hs3/test/hs3testsuite_roofit_backend.py b/roofit/hs3/test/hs3testsuite_roofit_backend.py index 47fa56e77ded5..a3102ba9c7a8a 100644 --- a/roofit/hs3/test/hs3testsuite_roofit_backend.py +++ b/roofit/hs3/test/hs3testsuite_roofit_backend.py @@ -1,5 +1,6 @@ from __future__ import annotations +import json import os import sys from contextlib import contextmanager @@ -41,40 +42,86 @@ def run_structure_check(self, workspace, check: dict[str, Any]) -> None: if missing: raise AssertionError(f"missing {key}: {sorted(missing)}") - def run_twice_delta_nll_scan(self, workspace, check: dict[str, Any]) -> list[float]: - target = check["target"] - pdf = workspace.pdf(target["pdf"]) - if pdf is None: - raise AssertionError(f"PDF {target['pdf']!r} not found") - data = workspace.data(target["data"]) - if data is None: - raise AssertionError(f"data {target['data']!r} not found") + def run_twice_delta_nll_scan(self, workspace, check: dict[str, Any], hs3_path: Path) -> list[float]: + pdf_data_objs = [] + for pdf_name, data_name in self._resolve_target_pairs(check["target"], hs3_path): + pdf = workspace.pdf(pdf_name) + if not pdf: + raise AssertionError(f"PDF {pdf_name!r} not found") + data = workspace.data(data_name) + if not data: + raise AssertionError(f"data {data_name!r} not found") + pdf_data_objs.append((pdf, data)) self._apply_parameter_point(workspace, check["reference_point"]) with suppress_root_output(): - nll = pdf.createNLL( - data, - self.ROOT.RooFit.NumCPU(1), - self.ROOT.RooFit.EvalBackend("legacy"), - ) + nlls = [ + pdf.createNLL(data, self.ROOT.RooFit.NumCPU(1), self.ROOT.RooFit.EvalBackend("legacy")) + for pdf, data in pdf_data_objs + ] + if len(nlls) == 1: + nll = nlls[0] + else: + nll = self.ROOT.RooAddition("hs3suite_nll", "hs3suite_nll", self.ROOT.RooArgList(*nlls)) reference = float(nll.getVal()) + return [2.0 * (value - reference) for value in self._scan(workspace, check, lambda: float(nll.getVal()))] + + def run_pdf_scan(self, workspace, check: dict[str, Any], hs3_path: Path) -> list[float]: + target = check["target"] + pdf = workspace.pdf(target["pdf"]) + if not pdf: + raise AssertionError(f"PDF {target['pdf']!r} not found") + norm_set = self.ROOT.RooArgSet() + for name in target["observables"]: + observable = workspace.var(name) + if not observable: + raise AssertionError(f"observable {name!r} not found") + norm_set.add(observable) + return self._scan(workspace, check, lambda: float(pdf.getVal(norm_set))) + + def run_function_scan(self, workspace, check: dict[str, Any], hs3_path: Path) -> list[float]: + name = check["target"]["function"] + func = workspace.function(name) or workspace.arg(name) + if not func: + raise AssertionError(f"function {name!r} not found") + return self._scan(workspace, check, lambda: float(func.getVal())) + + def _scan(self, workspace, check: dict[str, Any], evaluate) -> list[float]: values = [] - scan_parameter = check["scan_parameter"] + scan_parameters = check["scan_parameters"] for point in check["scan_points"]: self._apply_parameter_point(workspace, check["reference_point"]) - var = workspace.var(scan_parameter) - if var is None: - raise AssertionError(f"scan parameter {scan_parameter!r} not found") - var.setVal(float(point)) + self._apply_parameter_point(workspace, dict(zip(scan_parameters, point, strict=True))) with suppress_root_output(): - values.append(2.0 * (float(nll.getVal()) - reference)) + values.append(evaluate()) return values + def _resolve_target_pairs(self, target: dict[str, Any], hs3_path: Path) -> list[tuple[str, str]]: + if "likelihood" not in target: + return [(target["pdf"], target["data"])] + with open(hs3_path, encoding="utf-8") as f: + likelihoods = json.load(f).get("likelihoods", []) + name = target["likelihood"] + entry = next((e for e in likelihoods if e.get("name") == name), None) + if entry is None: + raise AssertionError(f"likelihood {name!r} not found in {hs3_path}") + if len(entry["distributions"]) != len(entry["data"]): + raise AssertionError(f"likelihood {name!r}: distributions/data length mismatch") + return list(zip(entry["distributions"], entry["data"])) + def _apply_parameter_point(self, workspace, values: dict[str, float]) -> None: for name, value in values.items(): var = workspace.var(name) - if var is not None: + if var: var.setVal(float(value)) + continue + arg = workspace.arg(name) + if not arg: + raise AssertionError(f"parameter {name!r} not found") + # Parameters outside of all domains are imported as constants and cannot be + # changed. Skip them, like the HS3TestSuite reference backend does. + if not isinstance(arg, self.ROOT.RooConstVar): + raise AssertionError(f"parameter {name!r} is a {arg.ClassName()}, which cannot be set") @contextmanager diff --git a/roofit/hs3/test/test_hs3_suite.py b/roofit/hs3/test/test_hs3_suite.py index 5c7804afd028d..b0ecca89b164e 100644 --- a/roofit/hs3/test/test_hs3_suite.py +++ b/roofit/hs3/test/test_hs3_suite.py @@ -4,15 +4,30 @@ from pathlib import Path hs3_root_dir = os.environ.get("HS3TESTSUITE_ROOT") -if hs3_root_dir: - sys.path.insert(0, hs3_root_dir) +hs3_code_dir = os.environ.get("HS3TESTSUITECODE_ROOT") +if hs3_code_dir: + sys.path.insert(0, hs3_code_dir) -try: - from hs3suite.backends import _BACKENDS - from hs3suite.runner import run_suite - _BACKENDS["roofit"] = ("hs3testsuite_roofit_backend", "RooFitBackend") +def load_expected_failures(path): + """Read the list of expected failures. + + Every non-empty line that is not a comment has the form + ``:: # ``. + """ + expected = {} + if not path: + return expected + for line in Path(path).read_text().splitlines(): + entry, _, reason = line.partition("#") + entry = entry.strip() + if entry: + expected[entry] = reason.strip() + return expected + +try: + from hs3suite.runner import run_suite except ImportError as e: TestHS3Suite = type( "TestHS3Suite", @@ -20,9 +35,9 @@ dict(test_dependencies=lambda self, e=e: self.fail(f"Missing dependency: {e}")), ) else: - hs3_root_path = Path(hs3_root_dir) + expected_failures = load_expected_failures(os.environ.get("HS3TESTSUITE_EXPECTED_FAILURES")) try: - results = run_suite(hs3_root_path, "roofit") + results = run_suite(Path(hs3_root_dir), "hs3testsuite_roofit_backend:RooFitBackend") except Exception as e: def test(self, e=e): @@ -36,16 +51,32 @@ def test(self, e=e): def _make(result): def test(self): - if result.status == "failed": + if result.status in ("failed", "xfail"): self.fail(result.message) return + key = f"{result.test_id}::{result.check_id}" test.__name__ = f"test_{result.test_id}__{result.check_id}" - test.__doc__ = f"{result.test_id}::{result.check_id}" + test.__doc__ = key + # Known failures are listed in hs3testsuite_expected_failures.txt, or + # marked as "xfail" for RooFit in the manifest of the HS3TestSuite. An + # unexpected success fails the test run, so the list has to be shrunk as + # soon as a fixture starts to pass. + if result.status == "xfail" or key in expected_failures: + test = unittest.expectedFailure(test) return test func = _make(r) namespace[func.__name__] = func + + # Entries that don't correspond to any check (e.g. after a fixture was renamed + # upstream) would silently hide nothing, so they are reported as failures. + stale = sorted(set(expected_failures).difference(f"{r.test_id}::{r.check_id}" for r in results)) + + def test_expected_failures_list_is_current(self): + self.assertEqual(stale, [], "entries in the list of expected failures that match no check") + + namespace[test_expected_failures_list_is_current.__name__] = test_expected_failures_list_is_current TestHS3Suite = type("TestHS3Suite", (unittest.TestCase,), namespace) if __name__ == "__main__":