Update to handle multiDim histograms #207

aebid · 2026-01-16T13:35:05Z

Add 2D linearizing, multi-dim histograms, and fixed memory leak in histmerger

Copilot

Pull request overview

This pull request adds support for multi-dimensional (2D and 3D) histograms to the FLAF analysis framework and fixes a memory leak in the histogram merger. The changes enable linearization of 2D histograms into 1D representations with custom binning per y-bin range.

Changes:

Added C++ function rebinHistogramDict to rebin multi-dimensional histograms with custom binning per y-range
Extended Python histogram handling to support 1D, 2D, and 3D histograms throughout the analysis pipeline
Fixed memory leak in histogram merger by setting SetDirectory(0) on histograms during accumulation

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
include/HistHelper.h	Adds C++ `rebinHistogramDict` function for multi-dimensional histogram rebinning with custom y-range binning
Common/HistHelper.py	Extends `RebinHisto` and `GetModel` to support dict-based binning and multi-dimensional histograms; adds cache helper functions
Analysis/tasks.py	Adds variable flattening logic to extract individual variables from multi-dimensional variable definitions
Analysis/HistTupleProducer.py	Refactors to handle flattened variables and adds shifted tree processing for systematic uncertainties
Analysis/HistProducerFromNTuple.py	Updates bin counting logic to correctly handle 2D/3D histograms and adds dimension detection
Analysis/HistPlotter.py	Removes duplicate `RebinHisto` and `getNewBins` functions (now in HistHelper.py); adds 2D binning support
Analysis/HistMergerFromHists.py	Fixes memory leak by calling `SetDirectory(0)` when accumulating histograms and refactors file handling

Copilot · 2026-01-16T13:48:09Z

Analysis/HistTupleProducer.py

+        dfw_central.df = dfw_central.df.Define(f"{var}_bin", f"get_{var}_bin({var})")
+        dfw_central.colToSave.append(f"{var}_bin")
+
+    varToSave = Utilities.ListToVector(list(set(dfw_central.colToSave)))


The variables tmp_fileNames and snaps are used before they are initialized. They are first used on lines 168 and 169, but not initialized until lines 222-223. These variables should be initialized before their first use, likely at the beginning of the function or before line 166.

Suggested change

varToSave = Utilities.ListToVector(list(set(dfw_central.colToSave)))

varToSave = Utilities.ListToVector(list(set(dfw_central.colToSave)))

tmp_fileNames = []

snaps = []

Copilot · 2026-01-16T13:48:11Z

Analysis/HistTupleProducer.py

+        )
+        if df_central.Filter("map_placeholder > 0").Count().GetValue() <= 0:
+            raise RuntimeError("no events passed map placeolder")
+        all_shifts_to_compute.extend(unc_cfg_dict["shape"].keys())


The variable all_shifts_to_compute is used with .extend() but is never initialized. It should be initialized as an empty list before this line, similar to how norm_uncertainties and scale_uncertainties are initialized.

Copilot · 2026-01-16T13:48:12Z

Analysis/HistTupleProducer.py

+            raise RuntimeError("no events passed map placeolder")
+        all_shifts_to_compute.extend(unc_cfg_dict["shape"].keys())
+
+    for unc in ["Central"] + all_rel_uncs_to_compute:


The variable all_rel_uncs_to_compute is referenced but never defined or initialized in the visible code. This will cause a NameError at runtime.

Suggested change

for unc in ["Central"] + all_rel_uncs_to_compute:

for unc in ["Central"] + list(norm_uncertainties):

Copilot · 2026-01-16T13:48:12Z

Analysis/HistTupleProducer.py

+        df_central = createCentralQuantities(
+            df_central, col_types_central, col_names_central
+        )


The function createCentralQuantities is called but is never imported. The import statement from FLAF.Common.HistHelper import * is commented out on line 13. Either uncomment this import or add an explicit import for createCentralQuantities.

Copilot · 2026-01-16T13:48:12Z

Analysis/HistTupleProducer.py

+                f"weight_{unc}_{scale}" if unc != "Central" else "weight_Central"
+            )
+            histTupleDef.DefineWeightForHistograms(
+                dfw=dfw_central,


The variable dfw_central is used but never defined or initialized in the visible code. It should be created before being passed to DefineWeightForHistograms.

Copilot · 2026-01-16T13:48:17Z

include/HistHelper.h

+            }
+        }
+        return hist_output;
+    }


The rebinHistogramDict function lacks documentation. Add a comment block explaining the function's purpose, parameters, return value, and the algorithm used for rebinning multi-dimensional histograms into linearized 1D histograms.

Copilot · 2026-01-16T13:48:19Z

include/HistHelper.h

+        all_output_edges.erase(std::unique(all_output_edges.begin(), all_output_edges.end()), all_output_edges.end());
+
+        // Create output histogram with variable binning
+        TH1D* hist_output = new TH1D("rebinned", "rebinned", all_output_edges.size() - 1, all_output_edges.data());


The histogram name and title are hardcoded as 'rebinned'. Consider accepting these as parameters or deriving them from the input histogram to make the function more flexible and preserve histogram metadata.

Suggested change

TH1D* hist_output = new TH1D("rebinned", "rebinned", all_output_edges.size() - 1, all_output_edges.data());

TH1D* hist_output = new TH1D(hist_initial->GetName(), hist_initial->GetTitle(), all_output_edges.size() - 1, all_output_edges.data());

Copilot · 2026-01-16T13:48:21Z

Analysis/HistProducerFromNTuple.py

        for scale in scales:
            for key, filter_to_apply_base in key_filter_dict.items():
                filter_to_apply_final = filter_to_apply_base
+                print("Saving Hist for unc/scale/key:", unc, scale, key)


Debug print statement should be removed or converted to proper logging for production code.

Copilot · 2026-01-16T13:48:24Z

include/HistHelper.h

+
+    TH1D* rebinHistogramDict(TH1* hist_initial, int N_bins, 
+                                const std::vector<std::pair<float, float>>& y_bin_ranges,
+                                const std::vector<std::vector<float>>& output_bin_edges) {


The function creates a new histogram with new but there's no clear ownership or deletion mechanism documented. Consider documenting that the caller is responsible for deleting the returned histogram, or use ROOT's memory management (e.g., adding the histogram to a directory).

Suggested change

TH1D* rebinHistogramDict(TH1* hist_initial, int N_bins,

const std::vector<std::pair<float, float>>& y_bin_ranges,

const std::vector<std::vector<float>>& output_bin_edges) {

/**

* Rebin a 1D histogram using a dictionary of variable-width bin edges.

*

* The returned histogram is allocated with `new` and ownership is

* transferred to the caller. The caller is responsible for managing

* its lifetime, e.g. by:

* - explicitly deleting it with `delete`, or

* - adding it to a ROOT TDirectory/TFile so that ROOT manages

* its deletion.

*/

TH1D* rebinHistogramDict(TH1* hist_initial, int N_bins,

const std::vector<std::pair<float, float>>& y_bin_ranges,

const std::vector<std::vector<float>>& output_bin_edges) {

Copilot · 2026-01-16T13:48:24Z

Common/HistHelper.py

+    # print(col_names_cache)
+    # if "kinFit_result" in col_names_cache:
+    #    col_names_cache.remove("kinFit_result")


This comment appears to contain commented-out code.

Suggested change

# print(col_names_cache)

# if "kinFit_result" in col_names_cache:

# col_names_cache.remove("kinFit_result")

# All cache columns are forwarded to the central DataFrame as-is.

aebid · 2026-01-16T13:55:52Z

@cms-flaf-bot please test

HH_bbWW_version=PR_54

cms-flaf-bot · 2026-01-16T13:56:13Z

pipeline#13793040 started

cms-flaf-bot · 2026-01-16T13:59:08Z

pipeline#13793040 failed

aebid · 2026-01-16T14:11:40Z

@cms-flaf-bot please test

HH_bbWW_version=PR_54

cms-flaf-bot · 2026-01-16T14:11:58Z

pipeline#13793221 started

aebid · 2026-01-16T14:28:53Z

I hate merge conflicts and apparently broke everything when I was trying to fix them 😢

cms-flaf-bot · 2026-01-16T14:33:25Z

pipeline#13793221 failed

aebid · 2026-01-16T14:56:33Z

@cms-flaf-bot please test

HH_bbWW_version=PR_54

cms-flaf-bot · 2026-01-16T14:56:49Z

pipeline#13793622 started

aebid · 2026-01-16T15:02:52Z

@cms-flaf-bot please test

HH_bbWW_version=PR_54

cms-flaf-bot · 2026-01-16T15:03:16Z

pipeline#13793706 started

cms-flaf-bot · 2026-01-16T15:25:08Z

pipeline#13793706 failed

aebid · 2026-01-16T15:28:12Z

@cms-flaf-bot please test

HH_bbWW_version=PR_54

cms-flaf-bot · 2026-01-16T15:28:26Z

pipeline#13793934 started

cms-flaf-bot · 2026-01-16T15:54:19Z

pipeline#13793934 failed

cms-flaf-bot · 2026-01-16T15:57:06Z

pipeline#13793622 failed

aebid · 2026-01-16T16:07:08Z

@cms-flaf-bot please test

HH_bbWW_version=PR_54

cms-flaf-bot · 2026-01-16T16:07:25Z

pipeline#13794271 started

cms-flaf-bot · 2026-01-16T16:32:57Z

pipeline#13794271 failed

aebid · 2026-01-16T19:28:06Z

@cms-flaf-bot please test

HH_bbWW_version=PR_54

cms-flaf-bot · 2026-01-16T19:28:27Z

pipeline#13795444 started

cms-flaf-bot · 2026-01-16T19:52:54Z

pipeline#13795444 passed

aebid · 2026-01-16T22:18:01Z

@cms-flaf-bot please test

HH_bbWW_version=PR_54

cms-flaf-bot · 2026-01-16T22:18:20Z

pipeline#13796018 started

cms-flaf-bot · 2026-01-16T22:38:17Z

pipeline#13796018 failed

aebid · 2026-01-17T00:02:55Z

Testing has to wait until cms-flaf/HH_bbWW#53 is merged

aebid · 2026-01-17T17:37:08Z

@cms-flaf-bot please test

HH_bbWW_version=PR_54

cms-flaf-bot · 2026-01-17T17:37:22Z

pipeline#13799766 started

cms-flaf-bot · 2026-01-17T17:58:54Z

pipeline#13799766 failed

Update to handle multiDim histograms

3471e6c

kandrosov requested a review from Copilot January 16, 2026 13:42

Copilot started reviewing on behalf of kandrosov January 16, 2026 13:43 View session

Copilot AI reviewed Jan 16, 2026

View reviewed changes

Merge

0dddc08

aebid force-pushed the Add_MultiDim_Plots branch from 76b6877 to 0dddc08 Compare January 16, 2026 14:56

Update flaf env again after reset-merge

929d7cb

Fixed some issues with flattened vars

eaa9bbf

Formatting

711b98c

Bug fix

bda3535

Remove debug prints

ffc7872

Remove debug

129d233

Fix bug with 1D string binning

c979734

Merge with FLAF204

516c860

	for unc in ["Central"] + all_rel_uncs_to_compute:
	for unc in ["Central"] + list(norm_uncertainties):

	TH1D* hist_output = new TH1D("rebinned", "rebinned", all_output_edges.size() - 1, all_output_edges.data());
	TH1D* hist_output = new TH1D(hist_initial->GetName(), hist_initial->GetTitle(), all_output_edges.size() - 1, all_output_edges.data());

-    TH1D* rebinHistogramDict(TH1* hist_initial, int N_bins,
-                                const std::vector<std::pair<float, float>>& y_bin_ranges,
-                                const std::vector<std::vector<float>>& output_bin_edges) {
+    /**
+     * Rebin a 1D histogram using a dictionary of variable-width bin edges.
+     *
+     * The returned histogram is allocated with `new` and ownership is
+     * transferred to the caller. The caller is responsible for managing
+     * its lifetime, e.g. by:
+     *   - explicitly deleting it with `delete`, or
+     *   - adding it to a ROOT TDirectory/TFile so that ROOT manages
+     *     its deletion.
+     */
+    TH1D* rebinHistogramDict(TH1* hist_initial, int N_bins,
+                             const std::vector<std::pair<float, float>>& y_bin_ranges,
+                             const std::vector<std::vector<float>>& output_bin_edges) {

Update to handle multiDim histograms #207

Are you sure you want to change the base?

Update to handle multiDim histograms #207

Conversation

aebid commented Jan 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

aebid commented Jan 16, 2026

Uh oh!

cms-flaf-bot commented Jan 16, 2026

Uh oh!

cms-flaf-bot commented Jan 16, 2026

Uh oh!

aebid commented Jan 16, 2026

Uh oh!

cms-flaf-bot commented Jan 16, 2026

Uh oh!

aebid commented Jan 16, 2026

Uh oh!

cms-flaf-bot commented Jan 16, 2026

Uh oh!

aebid commented Jan 16, 2026

Uh oh!

cms-flaf-bot commented Jan 16, 2026

Uh oh!

aebid commented Jan 16, 2026

Uh oh!

cms-flaf-bot commented Jan 16, 2026

Uh oh!

cms-flaf-bot commented Jan 16, 2026

Uh oh!

aebid commented Jan 16, 2026

Uh oh!

cms-flaf-bot commented Jan 16, 2026

Uh oh!

cms-flaf-bot commented Jan 16, 2026

Uh oh!

cms-flaf-bot commented Jan 16, 2026

Uh oh!

aebid commented Jan 16, 2026

Uh oh!

cms-flaf-bot commented Jan 16, 2026

Uh oh!

cms-flaf-bot commented Jan 16, 2026

Uh oh!

aebid commented Jan 16, 2026