diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 0703194..d567848 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -102,6 +102,6 @@ Verified 2026-08-27; re-check before relying on any of it. | Layout | `dc_make/` (datacard construction: `maker.py`, `create_datacards.py`, `process.py`, `model.py`, `uncertainty.py`, `binner.py`), `bin_opt/` (binning optimisation and limits), `common/` (`tools.py`, `param_parse.py`), `law/tasks.py`, `fit_val/`, `config/`, `legacy/`, `test/` (unit tests and their scratch analysis) | | Consumers | HH_bbtautau and HH_bbWW use this as a submodule; H_mumu does not | | Cards | analysis-side, e.g. `HH_bbWW/config/Datacards/CI_card.yaml` | -| Tests | `test/test_law_chain.py` (the law tasks on the real FLAF and law classes over a scratch analysis, `test/statinference_fixture.py`: per card the outputs, inputs and payload commands; no ROOT, Combine or dhi), run by `unit-tests` against a pinned FLAF commit | +| Tests | `test/test_law_chain.py` (the law tasks on the real FLAF and law classes over a scratch analysis, `test/statinference_fixture.py`: per card the outputs, inputs and payload commands) and `test/test_shape_plots.py` (which histograms a card's shape plots show, through `input_categories`); no ROOT, Combine or dhi; run by `unit-tests` against a pinned FLAF commit | | Workflows | `formatting-check`, `unit-tests`, `repo-sanity-checks` (FLAF's sanity checks and FLAF's `run_tools/check_era_rename.py` for leftovers of the `period` → `era` rename), `trigger-flaf-integration`. Formatting is checked automatically — do not comment on it. A change to an assertion of `test/test_law_chain.py` (not to its helpers) changes what the chain is guaranteed to read, write or run — comment on it | | Integration test | Triggered by `@cms-flaf-bot please test`; the pipeline configuration lives in `cms-flaf/FLAF_ci` | diff --git a/.github/workflows/unit-tests.yaml b/.github/workflows/unit-tests.yaml index 31f06a6..d9ca069 100644 --- a/.github/workflows/unit-tests.yaml +++ b/.github/workflows/unit-tests.yaml @@ -46,3 +46,4 @@ jobs: run: >- python -m pytest -q StatInference/test/test_law_chain.py + StatInference/test/test_shape_plots.py diff --git a/README.md b/README.md index c4fd2cc..ff42a13 100644 --- a/README.md +++ b/README.md @@ -101,12 +101,13 @@ rm -r output/binning_v2/tauTau_2018/workers `test/test_law_chain.py` checks the law tasks on the real FLAF and law classes over a scratch analysis (`test/statinference_fixture.py`): for each card, what it writes, which merged -histograms it reads and the commands it runs. It needs no ROOT, Combine or dhi. The -`unit-tests` workflow runs it against a pinned FLAF commit; to run it locally, put a FLAF -checkout at that commit (`ref:` in `.github/workflows/unit-tests.yaml`) next to this one and -run from the directory holding both: +histograms it reads and the commands it runs. `test/test_shape_plots.py` checks which +histograms the shape plots of a card show, for categories read through `input_categories`. +They need no ROOT, Combine or dhi. The `unit-tests` workflow runs them against a pinned FLAF +commit; to run them locally, put a FLAF checkout at that commit (`ref:` in +`.github/workflows/unit-tests.yaml`) next to this one and run from the directory holding both: ```bash pip install pyyaml numpy luigi==3.8.1 law==0.1.21 pytest -PYTHONPATH=$PWD python -m pytest -q StatInference/test/test_law_chain.py +PYTHONPATH=$PWD python -m pytest -q StatInference/test/test_law_chain.py StatInference/test/test_shape_plots.py ``` diff --git a/law/CreateDatacardsTask.py b/law/CreateDatacardsTask.py index 56408ab..231c1d2 100644 --- a/law/CreateDatacardsTask.py +++ b/law/CreateDatacardsTask.py @@ -8,7 +8,7 @@ from FLAF.RunKit.run_tools import ps_call from FLAF.run_tools.law_customizations import HTCondorWorkflow, copy_param -from StatInference.common.tools import CategoryNaming, importROOT +from StatInference.common.tools import CategoryNaming, InputCategories, importROOT from .PreprocessShapesTask import PreprocessShapesTask from .StatInferenceTask import StatInferenceTask @@ -223,23 +223,39 @@ def _stitch_grid(panels, out_path): return True @staticmethod - def _present_keys(shape_file, cfg): - """The (channel, region, category) triples the summed shape file actually holds.""" + def _present_keys(shape_file, cfg, points): + """({(channel, region, category): input directory} for the datacard bins the summed + shape file holds, [every path looked up], or None when the file cannot be read). + + Each category is looked up where the maker reads it for these model points: its + `input_categories` directory, else its own name. A category that the points read + from different directories is not plotted, since one plot cannot show them. + """ ROOT = importROOT() - present = set() + input_categories = InputCategories.fromConfig(cfg) + present, looked_up = {}, [] f = ROOT.TFile.Open(shape_file) if not f or f.IsZombie(): - return present + return present, None try: for category in cfg["categories"]: region, _, cat = category.rpartition("/") + directories = {input_categories.path(category, p) for p in points} + if len(directories) != 1: + print( + f"WARNING: {category} is read from {', '.join(sorted(directories))} " + "for the model points of one variable; it is not plotted" + ) + continue + (directory,) = directories for channel in cfg["channels"]: - path = "/".join(x for x in (channel, region, cat) if x) + path = "/".join(x for x in (channel, directory) if x) + looked_up.append(path) if f.Get(path): - present.add((channel, region, cat)) + present[(channel, region, cat)] = directory finally: f.Close() - return present + return present, looked_up def _stitch_grids(self, cfg, plots_dir, variable, panel_of_key): """One grid per base category: its slices across, the channels down.""" @@ -289,6 +305,7 @@ def plot_rebinned_shapes(self, base_dir, config, output_dir): # The datacard bin is the sum over the sub-eras (getCombinedShape does the same at # card-build time), so the sub-era files are hadd'ed into the one file the plotter # reads. Plotting them separately would show something the fit never sees. + variable_points = self.get_variable_points() for variable in self.get_required_variables(): inputs = [ p @@ -315,23 +332,37 @@ def plot_rebinned_shapes(self, base_dir, config, output_dir): # from the shapes. HistPlotter exits non-zero on the first key it cannot find, # so passing the full list costs every panel and grid that would have come # after the gap, not just the missing one. - present = self._present_keys(summed, cfg) - keys, outputs = [], [] + present, looked_up = self._present_keys( + summed, cfg, variable_points[variable] + ) + keys, outputs, panel_of_key = [], [], {} for category in cfg["categories"]: # "SR/res2b_dnn0" -> region "SR", category "res2b_dnn0": HistPlotter # navigates channel -> region -> category. region, _, cat = category.rpartition("/") for channel in cfg["channels"]: - if (channel, region, cat) not in present: + directory = present.get((channel, region, cat)) + if directory is None: continue - keys.append(f"{channel}:{cat}:{region}") - outputs.append( - os.path.join( - plots_dir, f"{variable}_{channel}_{region}_{cat}.pdf" - ) + # The key names the directory the shapes are read from; the file and + # the grid keep the datacard bin's name. + in_region, _, in_cat = directory.rpartition("/") + keys.append(f"{channel}:{in_cat}:{in_region}") + output = os.path.join( + plots_dir, f"{variable}_{channel}_{region}_{cat}.pdf" ) + outputs.append(output) + panel_of_key[f"{channel}:{cat}:{region}"] = output if not keys: - print(f"WARNING: no shapes to plot for {variable}") + if looked_up is None: + reason = f"{summed} cannot be read" + elif looked_up: + reason = f"none of {', '.join(looked_up)} is in the summed shapes" + else: + reason = "no category is read from one directory" + print(f"WARNING: no shapes to plot for {variable}: {reason}") + if os.path.exists(summed): + os.remove(summed) continue cmd = [ "python3", @@ -372,9 +403,11 @@ def plot_rebinned_shapes(self, base_dir, config, output_dir): "--wantLogScale", "y", ] + if self.user_custom: + cmd += ["--user-custom", self.user_custom] try: ps_call(cmd, verbose=1) - self._stitch_grids(cfg, plots_dir, variable, dict(zip(keys, outputs))) + self._stitch_grids(cfg, plots_dir, variable, panel_of_key) except Exception as e: # The datacards are the product that matters; a plotting failure should be # loud but must not leave the task looking broken with valid cards on disk. diff --git a/law/StatInferenceTask.py b/law/StatInferenceTask.py index e2c7319..7d0ead7 100644 --- a/law/StatInferenceTask.py +++ b/law/StatInferenceTask.py @@ -159,27 +159,34 @@ def get_masses(self): ) return sorted(masses) - def get_required_variables(self): - """The Hists_merged variables this chain reads: one per mass point (the 2D - shape the datacards are built from), derived from the datacard config's - input_file_pattern. - - This is the *complete* input list -- there is no intersection with global.yaml's - histTuple_flavor variable list anywhere downstream, so a mass point present here - is read whether or not the active flavour happens to mention it. - """ + def get_variable_points(self): + """{variable: [model point, ...]}: the mass points whose input is that + Hists_merged variable, each as {parameter: value}, from the datacard config's + input_file_pattern.""" data = self.get_config_data() model = data["model"] pattern = model["input_file_pattern"] param_name = model["parameters"][0] - variables = set() + points = {} for mass in self.get_masses(): rel = Template(pattern).safe_substitute({"ERA": "ERA", param_name: mass}) # "//.root": the variable is the directory holding # the file, not the file itself. - variables.add(os.path.basename(os.path.dirname(rel))) - return sorted(variables) + variable = os.path.basename(os.path.dirname(rel)) + points.setdefault(variable, []).append({param_name: mass}) + return points + + def get_required_variables(self): + """The Hists_merged variables this chain reads: one per mass point (the 2D + shape the datacards are built from), derived from the datacard config's + input_file_pattern. + + This is the *complete* input list -- there is no intersection with global.yaml's + histTuple_flavor variable list anywhere downstream, so a mass point present here + is read whether or not the active flavour happens to mention it. + """ + return sorted(self.get_variable_points()) def merged_hist_reqs(self, eras): """{(era, variable): MergedHists} -- every merged input file for those eras. diff --git a/test/test_shape_plots.py b/test/test_shape_plots.py new file mode 100644 index 0000000..925b015 --- /dev/null +++ b/test/test_shape_plots.py @@ -0,0 +1,330 @@ +#!/usr/bin/env python3 +"""The shape plots of CreateDatacardsTask for categories read through `input_categories`. + +The maker reads a mapped datacard category from its input directory, with the model point +substituted (`SR/res2b` from `SR_SL_M${MX}/res2b`); the plots must show the same histograms. +On the scratch analysis of statinference_fixture with a single-lepton-like card (two mass +points, two mapped categories and one read under its own name), HistPlotter is asked for the +input directory of each mapped category and point, also when a directory under the configured +name exists too; the plot files and the grids keep the datacard bin's name; with nothing to +plot the paths looked up, an unreadable summed file or the lack of a category read from one +directory are named and the summed shapes removed; the plotter gets the task's --user-custom; a +category the points of one variable read from different directories is not plotted. +""" + +import importlib +import os +import re +import shutil +import sys +import tempfile +import unittest +from unittest import mock + +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) + +import statinference_fixture as fx # noqa: E402 + +VERSION = "v_plots" +HISTS_VERSION = "v_hists" +MASSES = [300, 500] +CHANNELS = ["e", "mu"] + +CARD = """ +analysis: test +type: resonant +model: + parameters: [ MX ] + param_dependent_bkg: true + input_file_pattern: ${ERA}/TwoStageDNN_M${MX}_binary/TwoStageDNN_M${MX}_binary.root +eras: [ Run3_2022 ] +channels: [ e, mu ] +categories: [ SR/res2b, SR/recovery, OS_Iso/inclusive ] +input_categories: + SR/res2b: SR_SL_M${MX}/res2b + SR/recovery: SR_SL_M${MX}/recovery +processes: + - process: data_obs + is_data: true + - process: Signal_${MX} + param_values: [ 300, 500 ] + is_signal: true + - TT +""" + +create_datacards_module = importlib.import_module( + "StatInference.law.CreateDatacardsTask" +) + + +def variable(mass): + return f"TwoStageDNN_M{mass}_binary" + + +class _ShapeFile: + def __init__(self, directories): + self.directories = directories + + def IsZombie(self): + return False + + def Get(self, path): + return path in self.directories + + def Close(self): + pass + + +def fake_root(directories_of): + """A ROOT whose summed shape file of a variable holds `directories_of[variable]`.""" + + class TFile: + @staticmethod + def Open(path): + name = os.path.basename(path)[len("_summed_") : -len(".root")] + return _ShapeFile(directories_of.get(name, set())) + + class ROOT: + pass + + ROOT.TFile = TFile + return ROOT + + +def sl_directories(mass, with_configured_name=False): + """What a single-lepton summed shape file of `mass` holds: every mass's signal region, + the OS_Iso region and, optionally, an `SR` region under the configured name.""" + regions = [f"SR_SL_M{m}" for m in MASSES] + (["SR"] if with_configured_name else []) + directories = set() + for channel in CHANNELS: + directories.add(channel) + for region in regions: + directories.add(f"{channel}/{region}") + for cat in ("res2b", "recovery"): + directories.add(f"{channel}/{region}/{cat}") + directories.add(f"{channel}/OS_Iso") + directories.add(f"{channel}/OS_Iso/inclusive") + return directories + + +class ShapePlotsTest(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.tmp = tempfile.mkdtemp(prefix="statinference_plots_") + cls.ana = fx.make_analysis(cls.tmp, "config/Datacards/card_sl.yaml") + fx.write(os.path.join(cls.ana, "config", "Datacards", "card_sl.yaml"), CARD) + fx.write( + os.path.join(cls.ana, "config", "user_custom_plots.yaml"), "# no override\n" + ) + cls.env = mock.patch.dict( + os.environ, + { + "ANALYSIS_PATH": cls.ana, + "ANALYSIS_DATA_PATH": os.path.join(cls.tmp, "data"), + }, + ) + cls.env.start() + + @classmethod + def tearDownClass(cls): + cls.env.stop() + fx.clear_process_caches() + shutil.rmtree(cls.tmp, ignore_errors=True) + + def setUp(self): + fx.clear_process_caches() + for directory in ("store", "data"): + shutil.rmtree(os.path.join(self.tmp, directory), ignore_errors=True) + for mass in MASSES: + var = variable(mass) + fx.write( + os.path.join( + self.tmp, + fx.hist_store(fx.ERA_A), + HISTS_VERSION, + "Hists_merged", + fx.ERA_A, + var, + f"{var}.root", + ), + "", + ) + + def card(self, **params): + return fx.si.CreateDatacardsTask( + version=VERSION, + era=fx.ERA_A, + hists_version=HISTS_VERSION, + branch=0, + **params, + ) + + def plot(self, card, directories_of, root=None): + """Run `card` with its commands recorded: (HistPlotter commands, the grids' panel + maps, the summed files left in plots/, printed lines).""" + calls, grids, printed = [], [], [] + + def ps_call(cmd, env=None, **kwargs): + if cmd[0] == "hadd": + fx.write(cmd[2], "") + calls.append(list(cmd)) + + def stitch_grids(task, cfg, plots_dir, var, panel_of_key): + grids.append((var, dict(panel_of_key))) + + with mock.patch.object( + create_datacards_module, "ps_call", ps_call + ), mock.patch.object( + type(card), "cmssw_env", new_callable=mock.PropertyMock + ), mock.patch.object( + create_datacards_module, + "importROOT", + lambda: root if root is not None else fake_root(directories_of), + ), mock.patch.object( + fx.si.CreateDatacardsTask, "_stitch_grids", stitch_grids + ), mock.patch( + "builtins.print", lambda *a, **k: printed.append(" ".join(map(str, a))) + ): + card.run() + plots_dir = os.path.join(card.output().abspath, "plots") + left = sorted(n for n in os.listdir(plots_dir) if n.startswith("_summed_")) + plotter = [c for c in calls if "--all_keys" in c] + return plotter, grids, left, printed + + @staticmethod + def arg(cmd, flag): + return cmd[cmd.index(flag) + 1] + + @staticmethod + def expected(mass): + """(HistPlotter keys, output file names, grid panels) of `mass`'s variable.""" + keys, outputs, panels = [], [], {} + for region, cat, in_region in ( + ("SR", "res2b", f"SR_SL_M{mass}"), + ("SR", "recovery", f"SR_SL_M{mass}"), + ("OS_Iso", "inclusive", "OS_Iso"), + ): + for channel in CHANNELS: + keys.append(f"{channel}:{cat}:{in_region}") + out = f"{variable(mass)}_{channel}_{region}_{cat}.pdf" + outputs.append(out) + panels[f"{channel}:{cat}:{region}"] = out + return keys, outputs, panels + + def check_plots(self, card, directories_of): + plotter, grids, left, _ = self.plot(card, directories_of) + self.assertEqual(len(plotter), len(MASSES)) + for cmd, (grid_var, panels) in zip(plotter, grids): + var = os.path.basename(self.arg(cmd, "--inFile"))[len("_summed_") : -5] + (mass,) = [m for m in MASSES if variable(m) == var] + keys, outputs, expected_panels = self.expected(mass) + self.assertEqual(self.arg(cmd, "--all_keys").split(","), keys) + # in the plots/ directory of the card's output, while the task writes it + files = self.arg(cmd, "--all_outFiles").split(",") + self.assertEqual([os.path.basename(f) for f in files], outputs) + self.assertEqual( + {os.path.basename(os.path.dirname(f)) for f in files}, {"plots"} + ) + self.assertEqual(grid_var, var) + self.assertEqual( + {k: os.path.basename(v) for k, v in panels.items()}, expected_panels + ) + self.assertEqual(set(panels.values()), set(files)) + self.assertEqual(left, []) + return plotter + + def test_mapped_categories_are_plotted_from_their_input_directory(self): + self.check_plots(self.card(), {variable(m): sl_directories(m) for m in MASSES}) + + def test_the_input_directory_wins_over_one_of_the_configured_name(self): + self.check_plots( + self.card(), + {variable(m): sl_directories(m, with_configured_name=True) for m in MASSES}, + ) + + def test_nothing_to_plot_names_the_paths_looked_up(self): + plotter, _, left, printed = self.plot(self.card(), {}) + self.assertEqual(plotter, []) + self.assertEqual(left, []) + for mass in MASSES: + (warning,) = [ + line + for line in printed + if line.startswith(f"WARNING: no shapes to plot for {variable(mass)}") + ] + for channel in CHANNELS: + self.assertIn(f"{channel}/SR_SL_M{mass}/res2b", warning) + self.assertIn(f"{channel}/OS_Iso/inclusive", warning) + + def test_an_unreadable_summed_file_is_named(self): + class Unreadable: + class TFile: + @staticmethod + def Open(path): + return None + + plotter, _, left, printed = self.plot(self.card(), {}, root=Unreadable) + self.assertEqual(plotter, []) + self.assertEqual(left, []) + for mass in MASSES: + var = variable(mass) + pattern = ( + f"WARNING: no shapes to plot for {var}: " + f".*/plots/_summed_{var}\\.root cannot be read" + ) + self.assertTrue( + any(re.fullmatch(pattern, line) for line in printed), printed + ) + + def test_no_category_from_one_directory_is_named(self): + with mock.patch.object( + fx.si.CreateDatacardsTask, + "_present_keys", + staticmethod(lambda shape_file, cfg, points: ({}, [])), + ): + plotter, _, left, printed = self.plot(self.card(), {}) + self.assertEqual(plotter, []) + self.assertEqual(left, []) + for mass in MASSES: + self.assertIn( + f"WARNING: no shapes to plot for {variable(mass)}: " + "no category is read from one directory", + printed, + ) + + def test_the_plotter_gets_the_user_custom(self): + directories = {variable(m): sl_directories(m) for m in MASSES} + for cmd in self.check_plots(self.card(), directories): + self.assertNotIn("--user-custom", cmd) + fx.clear_process_caches() + user_custom = os.path.join(self.ana, "config", "user_custom_plots.yaml") + for cmd in self.check_plots(self.card(user_custom=user_custom), directories): + self.assertEqual(self.arg(cmd, "--user-custom"), user_custom) + + def test_a_category_read_from_several_directories_is_not_plotted(self): + card = self.card() + cfg = card.get_config_data() + printed = [] + with mock.patch.object( + create_datacards_module, + "importROOT", + lambda: fake_root({"shared": sl_directories(300)}), + ), mock.patch( + "builtins.print", lambda *a, **k: printed.append(" ".join(map(str, a))) + ): + present, looked_up = card._present_keys( + "_summed_shared.root", cfg, [{"MX": 300}, {"MX": 500}] + ) + self.assertEqual( + present, + { + (channel, "OS_Iso", "inclusive"): "OS_Iso/inclusive" + for channel in CHANNELS + }, + ) + self.assertEqual(looked_up, [f"{c}/OS_Iso/inclusive" for c in CHANNELS]) + self.assertEqual(len([line for line in printed if "not plotted" in line]), 2) + + +if __name__ == "__main__": + unittest.main()