General Repository Improvements by dprim7 · Pull Request #356 · LPC-HH/HH4b

dprim7 · 2026-07-16T20:01:33Z

This PR addresses several software design issues that hurt reproducibitly and readability for group members working with this repository.

Changes

Remove hardcoded personal paths

--data-dir / --dir-name CLI options in PostProcess.py, PostProcessTT.py, jmsr_templates.py, eventlist.py, and overlap/eventlist_config.py no longer carry a default pointing at a personal ceph directory. They are now required=True (or None with a comment), so anyone running a script gets an explicit error rather than silently reading someone else's data.
jmsr/jmsr_templates.py: same treatment for the --dir-name click option.
overlap/eventlist_config.py, Scaling_Toys.py: replaced hardcoded data_dir strings in Namespace literals with None and an inline comment; callers must set this before use.
Scaling_Toys.py: HH4B_DIR now resolves from file instead of an absolute path, consistent with how PostProcess.py already derives the repo root.

Remove stale versioned-copy scripts

Deleted data/make_filelists_v15.py, src/HH4b/boosted/TestToysAll.py, and src/HH4b/boosted/TestToys_v2.py. These were renamed/versioned copies of active scripts and had not been updated alongside them, creating ambiguity about which file is canonical for a new contributor. Git history preserves the content if it is ever needed.

Notebook hygiene

Added nbstripout to the pre-commit hook so Jupyter output cells are stripped before commits, keeping diffs readable.
Lowered the check-added-large-files limit from 10 MB to 1 MB.
Added /*.pid to .gitignore.

Decompose postprocess_run3

The main entry point was a ~900-line monolith. Eight named helper functions have been extracted (_get_mass_windows, _setup_shape_var, _load_samples, _combine_years, _build_combined_cutflow, _run_fom_scans, _save_cutflows, _make_templates, _save_event_lists), each with a one-line docstring. The orchestrator now reads as ~55 lines of numbered steps.

One latent bug was fixed during this refactor: bg_keys / processes were not filtered to actually-loaded samples before being passed to abcd and the template builders, causing a KeyError on partial datasets (e.g. when running on a subset of samples).

Test suite

tests/test_postprocess_helpers.py — 19 unit tests for the extracted helpers using only synthetic DataFrames and tmp_path.
tests/test_postprocess_integration.py — 5 integration tests that exercise the full _load_samples → _combine_years → postprocess_run3 pipeline against a small real-data fixture. Tests are guarded with @requires_fixture (pytest.mark.skipif) and skip cleanly when the fixture is absent, so CI is never blocked on data availability.
scripts/make_test_data.py — one-shot script to slice ~300 rows from a real skimmer output into tests/fixtures/skimmer/ for use by the integration tests. Uses reset_index(drop=True).to_parquet() (not index=False) to preserve the parquet MultiIndex column schema.

Usage:

to create the fixture for new ntuples, use

PYTHONPATH=src python3 src/HH4b/postprocessing/PostProcess.py \
  --templates-tag test_fixture \
  --data-dir tests/fixtures/skimmer \
  --tag 25May9_v12v2_private_signal \
  --years 2022 \
  --txbb glopart-v2 \
  --mass H2PNetMass \
  --bdt-model 25Feb5_v13_glopartv2_rawmass \
  --bdt-config v13_glopartv2 \
  --no-vbf \
  --no-fom-scan --no-fom-scan-vbf --no-fom-scan-bin1 --no-fom-scan-bin2 \
  --dummy-txbb-sfs \
  --templates

to run a script (for instance PostProcess.py) on the fixture, use

 PYTHONPATH=src python src/HH4b/postprocessing/PostProcess.py \
  --templates-tag test_fixture \
  --data-dir tests/fixtures/skimmer \
  --tag 25May9_v12v2_private_signal \
  --years 2022 \
  --txbb glopart-v2 \
  --mass H2PNetMass \
  --bdt-model 25Feb5_v13_glopartv2_rawmass \
  --bdt-config v13_glopartv2 \
  --no-vbf \
  --no-fom-scan --no-fom-scan-vbf --no-fom-scan-bin1 --no-fom-scan-bin2 \
  --dummy-txbb-sfs \
  --templates

pyproject.toml: suppress pandas.errors.PerformanceWarning in the test environment — this warning fires when signal samples with many JEC shifts accumulate >100 DataFrame blocks; with filterwarnings = error it raised a spurious KeyError that does not occur in normal runs.

Several scripts had personal EOS/ceph paths baked in as default values for --data-dir (PostProcess.py, PostProcessTT.py, PostProcessTT.py, overlap/eventlist.py) and as a module-level constant (Scaling_Toys.py). Anyone other than the original author would silently get a non-existent path and no useful error message. Changes: - PostProcess.py, PostProcessTT.py, overlap/eventlist.py: make --data-dir required=True with a descriptive help string showing the expected path shape - jmsr/jmsr_templates.py: same treatment for the click --dir-name option - overlap/eventlist_config.py, Scaling_Toys.py: replace hardcoded data_dir string in Namespace literals with None and an inline comment; callers must set this before use - Scaling_Toys.py: replace hardcoded HH4B_DIR = "/home/users/woodson/HH4b/" with Path(__file__).resolve().parents[3], consistent with how PostProcess.py already derives the repo root

data/make_filelists_v15.py, src/HH4b/boosted/TestToys_v2.py, and src/HH4b/boosted/TestToysAll.py were parallel copies of existing files with version suffixes appended to the filename. None are imported or referenced anywhere in the codebase. Keeping them creates ambiguity about which file is canonical for a new contributor. - make_filelists_v15.py: superseded by the current data/make_filelists.py - TestToys_v2.py, TestToysAll.py: later iterations of TestToys.py left on disk; git history preserves the content if it is ever needed again

Notebook outputs were being committed to git, making diffs unreadable and bloating the repository (the largest notebook was ~8.4 MB). nbstripout automatically strips cell outputs and metadata before each commit, so the stored notebooks contain only source cells. Also lower the check-added-large-files threshold from 10 MB to 1 MB. The old limit was permissive enough to let most notebooks with outputs through. 1 MB is generous for source files while still catching accidentally staged data files or plot outputs. To install hooks locally after pulling this change: pre-commit install

run.pid is written to the repo root when a background job is running (e.g. a condor submission script). The other runtime artifacts that accumulate in the root (*.root, *.parquet, *.out, *.sh, *.log) were already covered by existing gitignore patterns, but *.pid was missing.

Eight helper functions extracted from the monolithic postprocess_run3: _get_mass_windows, _setup_shape_var, _load_samples, _combine_years, _build_combined_cutflow, _run_fom_scans, _save_cutflows, _save_event_lists, _make_templates. The orchestrator now reads as ~60 lines of numbered steps. Also filters bg_keys / processes to samples that were actually loaded before combining years, preventing KeyError in abcd / template builders when the data directory contains only a subset of the expected samples.

Unit tests (tests/test_postprocess_helpers.py, 19 tests) cover each helper extracted in the previous commit: _get_mass_windows, _setup_shape_var, _combine_years, _build_combined_cutflow, _save_cutflows, _save_event_lists. All helpers are exercised with synthetic DataFrames—no parquet files, no network, no GPU. Integration tests (tests/test_postprocess_integration.py, 5 tests) run the full _load_samples → _combine_years → _build_combined_cutflow pipeline against a small real fixture created by scripts/make_test_data.py. Tests are automatically skipped when the fixture directory is absent, so CI passes even without it. Two fixes accompany the tests: 1. pyproject.toml – suppress pandas.errors.PerformanceWarning in pytest and extend per-file ruff ignores to cover test/script patterns (PLC0415 for lazy in-function imports, ARG001/ARG005 for unused args in mock stubs). The PerformanceWarning is triggered when >100 columns are added one-by-one to bdt_events for signal samples with many JEC shifts; with filterwarnings=error it becomes an exception mid-insert, leaving the DataFrame in an inconsistent state. The production code path is unaffected. 2. scripts/make_test_data.py – use reset_index(drop=True).to_parquet() instead of to_parquet(index=False). The index=False argument also flattens MultiIndex column metadata to strings like "('col', '0')", destroying the tuple structure that load_run3_samples relies on. reset_index(drop=True) removes the row index without touching column metadata.

cmantill · 2026-07-21T21:33:15Z

Why are you getting rid of this script if we still use it?

Fixed, the file is restored

cmantill · 2026-07-21T21:34:22Z

+
+The resulting directory has the same structure as a real skimmer output:
+
+    <out-dir>/<tag>/<year>/<sample>/parquet/out_0.parquet


where would this fixture be used? can you post an example in the PR description?

Added the usage example in the PR description

cmantill

Left two small comments. How and when are the tests in tests meant to be run?

dprim7 · 2026-08-08T00:07:33Z

Left two small comments. How and when are the tests in tests meant to be run?

The test suite can be run with pytest but is also useful as a coding agent guardrail (gives the agent context on intended functionality).

Example for running the full suite:

 PYTHONPATH=src python -m pytest -q

or on a single script:

PYTHONPATH=src python -m pytest tests/test_postprocess_integration.py -q

This file was removed in 0a3d580 ("Remove stale versioned-copy scripts") on the assumption it was superseded by data/make_filelists.py. It is not — the v15 filelist generator is still needed for NanoAOD-v15 sample bookkeeping. Restored verbatim from main; the other two scripts removed in that commit (TestToys_v2.py, TestToysAll.py) are left deleted.

cmantill · 2026-08-18T20:02:08Z

@dprim7 can you resolve conflicts?

dprim7 added 6 commits July 16, 2026 11:50

dprim7 requested review from cmantill, jmduarte and zichunhao July 16, 2026 20:01

pre-commit-ci Bot and others added 4 commits July 16, 2026 20:03

style: pre-commit fixes

faaffa1

trigger CI

2df0806

style: pre-commit fixes

bebca66

Merge branch 'main' into reproducibility-improvements-v2

93b68e1

cmantill reviewed Jul 21, 2026

View reviewed changes

cmantill reviewed Aug 4, 2026

View reviewed changes

Merge branch 'main' into reproducibility-improvements-v2

c7aedfb

Navigation Menu

Uh oh!

General Repository Improvements - #356

General Repository Improvements#356
dprim7 wants to merge 12 commits into
LPC-HH:mainfrom
dprim7:reproducibility-improvements-v2

dprim7 commented Jul 16, 2026 •

edited

Loading

Uh oh!

cmantill Jul 21, 2026

Uh oh!

dprim7 Aug 18, 2026

Uh oh!

cmantill Jul 21, 2026

Uh oh!

dprim7 Aug 8, 2026

Uh oh!

cmantill left a comment

Uh oh!

dprim7 commented Aug 8, 2026 •

edited

Loading

Uh oh!

cmantill commented Aug 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		The resulting directory has the same structure as a real skimmer output:

		<out-dir>/<tag>/<year>/<sample>/parquet/out_0.parquet

Uh oh!

Conversation

dprim7 commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Remove hardcoded personal paths

Remove stale versioned-copy scripts

Notebook hygiene

Decompose postprocess_run3

Test suite

Usage:

Uh oh!

cmantill Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

dprim7 Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

cmantill Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

dprim7 Aug 8, 2026

Choose a reason for hiding this comment

Uh oh!

cmantill left a comment

Choose a reason for hiding this comment

Uh oh!

dprim7 commented Aug 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmantill commented Aug 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dprim7 commented Jul 16, 2026 •

edited

Loading

dprim7 commented Aug 8, 2026 •

edited

Loading