-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[HS3] [RF] Add HS3 conformance test suite integration #22567
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+173
−0
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
12e7039
[HS3] [RF] Adding test to pull and run external HS3TestSuite
stalbrec b02d03b
fix linting (import sorting)
stalbrec 5b2bdb5
[cmake] rename hs3testsuite option to test_roofit_hs3testsuite and do…
stalbrec f6a0e5f
[CMake] Update hs3testsuite directory path to use current binary dire…
stalbrec 0d462a0
[CMake] Use FetchContent for more robust HS3TestSuite download
stalbrec 09e7d24
adding roofit backend from hs3testsuite
stalbrec e2c4f09
update HS3TestSuite repository URL to the new source
stalbrec 5786763
making ruff happy
stalbrec b544a19
remove stale EXISTS guard and disable git prompt in HS3 testsuite fet…
stalbrec File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,19 @@ | ||
| ROOT_ADD_GTEST(testRooFitHS3 testRooFitHS3.cxx LIBRARIES RooFitCore RooFit RooFitHS3) | ||
| ROOT_ADD_GTEST(testHS3SimultaneousFit testHS3SimultaneousFit.cxx LIBRARIES RooFitCore RooFit RooFitHS3 RooStats) | ||
|
|
||
| if(test_roofit_hs3testsuite) | ||
| include(FetchContent) | ||
| FetchContent_Declare( | ||
| hs3testsuite | ||
| GIT_REPOSITORY https://github.com/hep-statistics-serialization-standard/HS3TestSuite.git | ||
| GIT_TAG 9d04e321ae6fddd283a35507f14ecf852eb7df61 | ||
| ) | ||
| # 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 | ||
| ENVIRONMENT HS3TESTSUITE_ROOT=${hs3testsuite_SOURCE_DIR} | ||
| COPY_TO_BUILDDIR hs3testsuite_roofit_backend.py | ||
| ) | ||
| endif() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import os | ||
| import sys | ||
| from contextlib import contextmanager | ||
| from pathlib import Path | ||
| from typing import Any | ||
|
|
||
|
|
||
| class RooFitBackend: | ||
| name = "roofit" | ||
|
|
||
| def __init__(self) -> None: | ||
| import ROOT # type: ignore | ||
|
|
||
| self.ROOT = ROOT | ||
| ROOT.gROOT.SetBatch(True) | ||
| ROOT.gErrorIgnoreLevel = ROOT.kFatal | ||
| ROOT.RooMsgService.instance().setGlobalKillBelow(ROOT.RooFit.FATAL) | ||
|
|
||
| def load_workspace(self, path: Path): | ||
| ws = self.ROOT.RooWorkspace("hs3suite_ws") | ||
| tool = self.ROOT.RooJSONFactoryWSTool(ws) | ||
| with suppress_root_output(): | ||
| tool.importJSON(str(path)) | ||
| return ws | ||
|
|
||
| def structure(self, workspace) -> dict[str, list[str]]: | ||
| return { | ||
| "pdfs": sorted(obj.GetName() for obj in workspace.allPdfs()), | ||
| "functions": sorted(obj.GetName() for obj in workspace.allFunctions()), | ||
| "data": sorted(obj.GetName() for obj in workspace.allData()), | ||
| } | ||
|
|
||
| def run_structure_check(self, workspace, check: dict[str, Any]) -> None: | ||
| actual = self.structure(workspace) | ||
| target = check.get("target", {}) | ||
| for key in ("pdfs", "functions", "data"): | ||
| required = set(target.get(key, [])) | ||
| missing = required.difference(actual[key]) | ||
| 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") | ||
|
|
||
| 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"), | ||
| ) | ||
| reference = float(nll.getVal()) | ||
| values = [] | ||
| scan_parameter = check["scan_parameter"] | ||
| 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)) | ||
| with suppress_root_output(): | ||
| values.append(2.0 * (float(nll.getVal()) - reference)) | ||
| return values | ||
|
|
||
| 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: | ||
| var.setVal(float(value)) | ||
|
|
||
|
|
||
| @contextmanager | ||
| def suppress_root_output(): | ||
| """Suppress noisy C++ diagnostics that bypass RooMsgService.""" | ||
|
|
||
| sys.stdout.flush() | ||
| sys.stderr.flush() | ||
| devnull_fd = os.open(os.devnull, os.O_WRONLY) | ||
| stdout_fd = os.dup(1) | ||
| stderr_fd = os.dup(2) | ||
| try: | ||
| os.dup2(devnull_fd, 1) | ||
| os.dup2(devnull_fd, 2) | ||
| yield | ||
| finally: | ||
| os.dup2(stdout_fd, 1) | ||
| os.dup2(stderr_fd, 2) | ||
| os.close(stdout_fd) | ||
| os.close(stderr_fd) | ||
| os.close(devnull_fd) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import os | ||
| import sys | ||
| import unittest | ||
| from pathlib import Path | ||
|
|
||
| hs3_root_dir = os.environ.get("HS3TESTSUITE_ROOT") | ||
| if hs3_root_dir: | ||
| sys.path.insert(0, hs3_root_dir) | ||
|
|
||
| try: | ||
| from hs3suite.backends import _BACKENDS | ||
| from hs3suite.runner import run_suite | ||
|
|
||
| _BACKENDS["roofit"] = ("hs3testsuite_roofit_backend", "RooFitBackend") | ||
|
|
||
| except ImportError as e: | ||
| TestHS3Suite = type( | ||
| "TestHS3Suite", | ||
| (unittest.TestCase,), | ||
| dict(test_dependencies=lambda self, e=e: self.fail(f"Missing dependency: {e}")), | ||
| ) | ||
| else: | ||
| hs3_root_path = Path(hs3_root_dir) | ||
| try: | ||
| results = run_suite(hs3_root_path, "roofit") | ||
| except Exception as e: | ||
|
|
||
| def test(self, e=e): | ||
| self.fail(f"HS3TestSuite failed to run: {e}") | ||
|
|
||
| test.__name__ = "test_hs3testsuite_execution" | ||
| TestHS3Suite = type("TestHS3Suite", (unittest.TestCase,), {test.__name__: test}) | ||
| else: | ||
| namespace = dict(__module__=__name__) | ||
| for r in results: | ||
|
|
||
| def _make(result): | ||
| def test(self): | ||
| if result.status == "failed": | ||
| self.fail(result.message) | ||
| return | ||
|
|
||
| test.__name__ = f"test_{result.test_id}__{result.check_id}" | ||
| test.__doc__ = f"{result.test_id}::{result.check_id}" | ||
| return test | ||
|
|
||
| func = _make(r) | ||
| namespace[func.__name__] = func | ||
| TestHS3Suite = type("TestHS3Suite", (unittest.TestCase,), namespace) | ||
|
|
||
| if __name__ == "__main__": | ||
| unittest.main() |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @stalbrec @guitargeek, these lines, in conjunction with globally switching on the hs3testsuite breaks ROOT's doxygen builds (and any other build that explicitly switches off testing).
I view
-Dtesting=Offas a global switch that disables testing, so I would not have expected this to break the ROOT build.OK for you if I rewrite the logic as:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the PR:
#22716
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @hageboeck,
sorry for breaking things.
I would be fine with this.
We could also change it such that
test_roofit_hs3testsuite=Offis implied by-Dtesting=Off.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about that, and thanks for the fix @hageboeck!