From eb8632c8199b67d2d27922e8b3a2abbfbc30174d Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Fri, 5 Sep 2025 16:45:27 +0200 Subject: [PATCH 01/39] [NFC] Fix white space. --- core/base/src/TROOT.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/base/src/TROOT.cxx b/core/base/src/TROOT.cxx index 9c0fe997a4757..137000cd371a7 100644 --- a/core/base/src/TROOT.cxx +++ b/core/base/src/TROOT.cxx @@ -2828,7 +2828,7 @@ void TROOT::SetBatch(Bool_t batch) /// interactive mode. /// - "server:port": turns the web display into server mode with specified port. Web widgets will not be displayed, /// only text message with window URL will be printed on standard output -/// +/// /// \note See more details related to webdisplay on RWebWindowsManager::ShowWindow void TROOT::SetWebDisplay(const char *webdisplay) From 709d6daafe497f23bcdbc470f03e0b03a08b2df1 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 9 Sep 2025 10:08:44 +0200 Subject: [PATCH 02/39] [NFC] Remove stray whitespaces. --- tutorials/math/exampleFunction.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tutorials/math/exampleFunction.py b/tutorials/math/exampleFunction.py index 16056fb53da04..9fc8bb2b665df 100644 --- a/tutorials/math/exampleFunction.py +++ b/tutorials/math/exampleFunction.py @@ -2,7 +2,7 @@ ## \ingroup tutorial_math ## \notebook ## Example of using Python functions as inputs to numerical algorithms -## using the ROOT Functor class. +## using the ROOT Functor class. ## ## \macro_image ## \macro_output @@ -39,7 +39,7 @@ def f(x): expValue = 6 if (not ROOT.TMath.AreEqualRel(value, expValue, 1.E-15)) : print("Error computing integral - computed value - different than expected, diff = ", value - expValue) - + # example multi-dim function print("\n\nUse Functor for wrapping a multi-dimensional function, the Rosenbrock Function r(x,y) and find its minimum") @@ -57,7 +57,7 @@ def RosenbrockFunction(xx): ### minimize multi-dim function using fitter class fitter = ROOT.Fit.Fitter() -#use a numpy array to pass initial parameter array +#use a numpy array to pass initial parameter array initialParams = np.array([0.,0.], dtype='d') fitter.FitFCN(func2D, initialParams) fitter.Result().Print(ROOT.std.cout) @@ -91,12 +91,12 @@ def g(x): return 2 * x def RosenbrockDerivatives(xx, icoord): x = xx[0] y = xx[1] - #derivative w.r.t x + #derivative w.r.t x if (icoord == 0) : return 2*(200*x*x*x-200*x*y+x-1) - else : + else : return 200 * (y - x * x) - + gradFunc2d = ROOT.Math.GradFunctor(RosenbrockFunction, RosenbrockDerivatives, 2) fitter = ROOT.Fit.Fitter() From a01aeee109952b0ed4579ed12f73069109681391 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Mon, 3 Nov 2025 10:56:28 +0100 Subject: [PATCH 03/39] [RF] Disable codegen backends in RooFit tests when clad=off. Although fallbacks to the cpu backend have been implemented, trying to test RooFit without clad leads to a gtest failure, since the test suite will be instantiated three times with the same backend. --- roofit/roofitcore/test/gtest_wrapper.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/roofit/roofitcore/test/gtest_wrapper.h b/roofit/roofitcore/test/gtest_wrapper.h index de31e562ad9f5..a09bd3300abfe 100644 --- a/roofit/roofitcore/test/gtest_wrapper.h +++ b/roofit/roofitcore/test/gtest_wrapper.h @@ -32,6 +32,10 @@ #define ROOFIT_EVAL_BACKENDS ROOFIT_EVAL_BACKEND_LEGACY ROOFIT_EVAL_BACKEND_CUDA RooFit::EvalBackend::Cpu() +#ifdef ROOFIT_CLAD #define ROOFIT_EVAL_BACKENDS_WITH_CODEGEN ROOFIT_EVAL_BACKENDS, ROOFIT_EVAL_BACKEND_CODEGEN RooFit::EvalBackend::CodegenNoGrad() +#else +#define ROOFIT_EVAL_BACKENDS_WITH_CODEGEN ROOFIT_EVAL_BACKENDS +#endif #endif // RooFit_gtest_wrapper_h From 974638fb94021621aaca3f139f585a84af1f5124 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Thu, 20 Mar 2025 15:39:14 +0100 Subject: [PATCH 04/39] [Core] Initialise gEnv pointer. --- core/base/src/TEnv.cxx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/base/src/TEnv.cxx b/core/base/src/TEnv.cxx index b61ab566a0549..c4779f948a674 100644 --- a/core/base/src/TEnv.cxx +++ b/core/base/src/TEnv.cxx @@ -79,9 +79,7 @@ into the local file by default. #include "THashList.h" #include "TError.h" - -TEnv *gEnv; // main environment created in TROOT - +TEnv *gEnv = nullptr; // main environment created in TROOT static struct BoolNameTable_t { const char *fName; From ac051c2696f845295b968b99acffc23d007bec5e Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Thu, 4 Sep 2025 11:34:25 +0200 Subject: [PATCH 05/39] [hist] Make TEfficiency aware of TH1::AddDirectoryStatus. This makes TEfficiency consistent with other histogram-like classes, in the sense that it only adds itself to a directory if the corresponding flags are on. --- hist/hist/src/TEfficiency.cxx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hist/hist/src/TEfficiency.cxx b/hist/hist/src/TEfficiency.cxx index 20e0d1c23143c..5e3e01570631f 100644 --- a/hist/hist/src/TEfficiency.cxx +++ b/hist/hist/src/TEfficiency.cxx @@ -1503,7 +1503,8 @@ Double_t TEfficiency::BetaMode(Double_t a,Double_t b) /// Notes: /// - calls: SetName(name), SetTitle(title) /// - set the statistic option to the default (kFCP) -/// - appends this object to the current directory SetDirectory(gDirectory) +/// - appends this object to the current directory SetDirectory(gDirectory) if +/// TH1::AddDirectoryStatus() is active. void TEfficiency::Build(const char* name,const char* title) { @@ -1511,7 +1512,7 @@ void TEfficiency::Build(const char* name,const char* title) SetTitle(title); SetStatisticOption(kDefStatOpt); - SetDirectory(gDirectory); + if (TH1::AddDirectoryStatus()) SetDirectory(gDirectory); SetBit(kPosteriorMode,false); SetBit(kShortestInterval,false); From 1eadddf2fe68de87691a8b8a452b562a1ad67992 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Mon, 8 Sep 2025 15:50:31 +0200 Subject: [PATCH 06/39] [roottest] Move hist ownership test from diffs to programmatic test. Diffs of the outputs are fragile, so the ownership of histograms is now tested programmatically. --- roottest/root/hist/misc/CMakeLists.txt | 4 +--- roottest/root/hist/misc/ownership.ref | 5 ----- roottest/root/hist/misc/runownership.C | 10 +++++++--- 3 files changed, 8 insertions(+), 11 deletions(-) delete mode 100644 roottest/root/hist/misc/ownership.ref diff --git a/roottest/root/hist/misc/CMakeLists.txt b/roottest/root/hist/misc/CMakeLists.txt index 747a3d0a118ab..e0af230801c4b 100644 --- a/roottest/root/hist/misc/CMakeLists.txt +++ b/roottest/root/hist/misc/CMakeLists.txt @@ -1,6 +1,4 @@ -ROOTTEST_ADD_TEST(runownership - MACRO runownership.C+ - OUTREF ownership.ref) +ROOTTEST_ADD_TEST(runownership MACRO runownership.C+) ROOTTEST_ADD_TEST(testSparse MACRO testSparse.cxx diff --git a/roottest/root/hist/misc/ownership.ref b/roottest/root/hist/misc/ownership.ref deleted file mode 100644 index 65dec199ebc1c..0000000000000 --- a/roottest/root/hist/misc/ownership.ref +++ /dev/null @@ -1,5 +0,0 @@ - -Processing runownership.C+... -So far: 0 -So far: 0 -(int)0 diff --git a/roottest/root/hist/misc/runownership.C b/roottest/root/hist/misc/runownership.C index efa51a2bdf49e..c5261e742d1ce 100644 --- a/roottest/root/hist/misc/runownership.C +++ b/roottest/root/hist/misc/runownership.C @@ -54,9 +54,13 @@ bool read(const char *filename = "histo.root") int runownership(const char *filename = "histo.root") { + bool failure = false; write(filename); - cout << "So far: " << TH1F_inst::fgCount << '\n'; + failure |= TH1F_inst::fgCount; + if (failure) std::cerr << "After write, instance count was " << TH1F_inst::fgCount << "\n"; read(filename); - cout << "So far: " << TH1F_inst::fgCount << '\n'; - return 0; + failure |= TH1F_inst::fgCount; + if (failure) std::cerr << "After read, instance count was " << TH1F_inst::fgCount << "\n"; + + return failure; } From 2a174e806e465afdba6f115bacdc09081ab53e79 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 9 Sep 2025 10:16:52 +0200 Subject: [PATCH 07/39] [hist] Mask SetDirectory(...) lines from SaveAs test. --- hist/hist/test/test_TH1_SaveAs.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/hist/hist/test/test_TH1_SaveAs.cxx b/hist/hist/test/test_TH1_SaveAs.cxx index 16fe750dcada3..d53b59c4db7d9 100644 --- a/hist/hist/test/test_TH1_SaveAs.cxx +++ b/hist/hist/test/test_TH1_SaveAs.cxx @@ -160,6 +160,7 @@ struct TestSaveAs { if (line != "{" && line != "}" && (line.rfind("//", 0) == 0 || line.length() < 6)) { continue; } + if (line.find("SetDirectory") != std::string::npos) continue; idx++; if (idx > NC) { infile.close(); From 29bc512f2d9ca09d868df0274f41d6c3c84c9baf Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 9 Sep 2025 10:28:26 +0200 Subject: [PATCH 08/39] [hist] Make the MapCppName test independent of gDirectory. --- hist/hist/test/test_MapCppName.cxx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hist/hist/test/test_MapCppName.cxx b/hist/hist/test/test_MapCppName.cxx index d9b78f8cb6158..6f18da7d40c37 100644 --- a/hist/hist/test/test_MapCppName.cxx +++ b/hist/hist/test/test_MapCppName.cxx @@ -32,6 +32,10 @@ TEST(TH1, MapCppNameTest) auto h3 = new TH2Poly(n3.Data(), n3.Data(), 10 , 0, 1, 10, 0, 1); h3->AddBin(0., 0., 1., 1.); auto h4 = new TProfile(n4.Data(), n4.Data(), 10, 0, 1); + h1->SetDirectory(nullptr); + h2->SetDirectory(nullptr); + h3->SetDirectory(nullptr); + h4->SetDirectory(nullptr); g1->Draw(); h1->Draw("same"); @@ -47,7 +51,7 @@ TEST(TH1, MapCppNameTest) if (!gSystem->GetPathInfo(CFile.Data(), fs)) FileSize = (Int_t)fs.fSize; - EXPECT_NEAR(FileSize, 5867, 200); + EXPECT_NEAR(FileSize, 6240, 200); gSystem->Unlink(CFile.Data()); } From 84751f99abdfa21f7fe30877fee3f022b9052317 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 9 Sep 2025 10:38:06 +0200 Subject: [PATCH 09/39] [hf] Explicitly write histograms in makeExample.C tutorial. If implicit object ownership gets in TDirectories gets switched off, the example histograms need to be added to directories or written explicitly. --- tutorials/roofit/histfactory/makeExample.C | 27 ++++++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/tutorials/roofit/histfactory/makeExample.C b/tutorials/roofit/histfactory/makeExample.C index 5210ca6d98c3f..b183ea864b155 100644 --- a/tutorials/roofit/histfactory/makeExample.C +++ b/tutorials/roofit/histfactory/makeExample.C @@ -5,15 +5,18 @@ void makeDataDriven() TFile* file = new TFile("dataDriven.root", "RECREATE"); TH1F* FlatHist = new TH1F("FlatHist","FlatHist", 2,0,2); + FlatHist->SetDirectory(file); FlatHist->SetBinContent( 1, 1.0 ); FlatHist->SetBinContent( 2, 1.0 ); TH1F* Signal = new TH1F("Signal", "Signal", 2,0,2); + Signal->SetDirectory(file); Signal->SetBinContent(1, 10); Signal->SetBinContent(2, 80); // MC Background TH1F* Background1 = new TH1F("Background1", "Background1", 2,0,2); + Background1->SetDirectory(file); Background1->SetBinContent(1, 20); Background1->SetBinContent(2, 20); @@ -21,16 +24,19 @@ void makeDataDriven() // that represents Background2 // Assume the extrapolation factor is 1.0 TH1F* ControlRegion = new TH1F("ControlRegion", "ControlRegion", 2,0,2); + ControlRegion->SetDirectory(file); ControlRegion->SetBinContent(1, 75); ControlRegion->SetBinContent(2, 35); TH1F* StatUncert = new TH1F("StatUncert", "StatUncert", 2,0,2); + StatUncert->SetDirectory(file); StatUncert->SetBinContent(1, .15); // 15% uncertainty StatUncert->SetBinContent(2, .15); // 15% uncertainty TH1F* data = new TH1F("data", "data", 2,0,2); + data->SetDirectory(file); data->SetBinContent(1, 90); data->SetBinContent(2, 110); @@ -49,6 +55,7 @@ void makeShapeSys2DDataset() TFile* file = new TFile("ShapeSys2D.root", "RECREATE"); TH2F* signal = new TH2F("signal", "signal", 2,0,2, 2,0,2); + signal->SetDirectory(file); signal->SetBinContent(1, 1, 10); signal->SetBinContent(2, 1, 10); signal->SetBinContent(1, 2, 20); @@ -56,6 +63,7 @@ void makeShapeSys2DDataset() // Background 1 TH2F* background1 = new TH2F("background1", "background1", 2,0,2, 2,0,2); + background1->SetDirectory(file); background1->SetBinContent(1, 1, 100); background1->SetBinContent(2, 1, 100); background1->SetBinContent(1, 2, 10); @@ -63,6 +71,7 @@ void makeShapeSys2DDataset() // Background 1 Error TH2F* bkg1ShapeError = new TH2F("bkg1ShapeError", "bkg1ShapeError", 2,0,2, 2,0,2); + bkg1ShapeError->SetDirectory(file); bkg1ShapeError->SetBinContent(1, 1, .10); // 10% bkg1ShapeError->SetBinContent(2, 1, .15); // 15% bkg1ShapeError->SetBinContent(1, 2, .10); // 10% @@ -71,6 +80,7 @@ void makeShapeSys2DDataset() // Background 2 TH2F* background2 = new TH2F("background2", "background2", 2,0,2, 2,0,2); + background2->SetDirectory(file); background2->SetBinContent(1, 1, 10); background2->SetBinContent(2, 1, 10); background2->SetBinContent(1, 2, 100); @@ -78,6 +88,7 @@ void makeShapeSys2DDataset() // Background 2 Error TH2F* bkg2ShapeError = new TH2F("bkg2ShapeError", "bkg2ShapeError", 2,0,2, 2,0,2); + bkg2ShapeError->SetDirectory(file); bkg2ShapeError->SetBinContent(1, 1, .05); // 5% bkg2ShapeError->SetBinContent(2, 1, .20); // 20% bkg2ShapeError->SetBinContent(1, 2, .05); // 5% @@ -85,6 +96,7 @@ void makeShapeSys2DDataset() TH2F* data = new TH2F("data", "data", 2,0,2, 2,0,2); + data->SetDirectory(file); data->SetBinContent(1, 1, 122); data->SetBinContent(2, 1, 122); data->SetBinContent(1, 2, 132); @@ -132,7 +144,10 @@ void makeShapeSysDataset() data->SetBinContent(1, 122); data->SetBinContent(2, 112); - file->Write(); + for (auto histo : {signal, background1, bkg1ShapeError, background2, bkg2ShapeError, data}) { + file->WriteTObject(histo); + } + file->Close(); @@ -171,11 +186,11 @@ void makeStatErrorDataSet() data->SetBinContent(1, 122); data->SetBinContent(2, 112); + for (auto histo : {FlatHist, signal, background1, bkg1StatUncert, background2, data}) { + file->WriteTObject(histo); + } - file->Write(); file->Close(); - - } @@ -202,8 +217,10 @@ void makeSimpleExample(){ statUncert->SetBinContent(1, .05); // 5% uncertainty statUncert->SetBinContent(2, .05); // 5% uncertainty + for (auto histo : {data, signal, background1, background2, statUncert}) { + example->WriteTObject(histo); + } - example->Write(); example->Close(); ////////////////////// From ceb5d7b2de845e6bd2f524084ea27ab2215b806f Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 9 Sep 2025 10:52:41 +0200 Subject: [PATCH 10/39] [PyROOT] Make TFile context manager test/tutorial ready for ROOT 7. - Explain that implicit assignment to directories is likely off in R7. - Make the test insensitive to any defaults by always adding the histogram to the file. --- .../pythonizations/test/tfile_context_manager.py | 1 + tutorials/io/tfile_context_manager.py | 11 ++++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/bindings/pyroot/pythonizations/test/tfile_context_manager.py b/bindings/pyroot/pythonizations/test/tfile_context_manager.py index 7ab1f1e1b17e8..257ab262a5c6c 100644 --- a/bindings/pyroot/pythonizations/test/tfile_context_manager.py +++ b/bindings/pyroot/pythonizations/test/tfile_context_manager.py @@ -64,6 +64,7 @@ def test_filewrite(self): histoname = "myhisto_3" with TFile(filename, "recreate") as outfile: hout = ROOT.TH1F(histoname, histoname, self.NBINS, self.XMIN, self.XMAX) + hout.SetDirectory(outfile) outfile.Write() self.check_file_data(outfile, filename, histoname) diff --git a/tutorials/io/tfile_context_manager.py b/tutorials/io/tfile_context_manager.py index b2b95fe625355..b235402054527 100644 --- a/tutorials/io/tfile_context_manager.py +++ b/tutorials/io/tfile_context_manager.py @@ -14,8 +14,11 @@ import ROOT from ROOT import TFile, gROOT -# By default, objects of some ROOT types such as `TH1` and its derived types +# By default, in ROOT 6, objects of some ROOT types such as `TH1` and its derived types # are automatically attached to a ROOT.TDirectory when they are created. +# This behaviour can be changed with ROOT.[Experimental.]EnableImplicitObjectOwnership() +# and the corresponding "DisableEmplicitObjectOwnership". In ROOT 7, the default +# will be *not* to implicitly assign histograms to directories. # Specifically, at any given point of a ROOT application, the ROOT.gDirectory # object tells which is the current directory where objects will be attached to. # The next line will print 'PyROOT' as the name of the current directory. @@ -25,7 +28,7 @@ # We can check to which directory a newly created histogram is attached. histo_1 = ROOT.TH1F("histo_1", "histo_1", 10, 0, 10) -print("Histogram '{}' is attached to: '{}'.\n".format(histo_1.GetName(), histo_1.GetDirectory().GetName())) +print("Histogram '{}' is attached to: '{}'.\n".format(histo_1.GetName(), "Nothing" if not histo_1.GetDirectory() else histo_1.GetDirectory().GetName())) # For quick saving and forgetting of objects into ROOT files, it is possible to # open a TFile as a Python context manager. In the context, objects can be @@ -37,7 +40,9 @@ histo_2 = ROOT.TH1F("histo_2", "histo_2", 10, 0, 10) # Inside the context, the current directory is the open file print("Current directory: '{}'.\n".format(ROOT.gDirectory.GetName())) - # And the created histogram is automatically attached to the file + # In ROOT 6, the created histogram is automatically attached to the file + # In ROOT 7, this can be done explicitly: + histo_2.SetDirectory(f) print("Histogram '{}' is attached to: '{}'.\n".format(histo_2.GetName(), histo_2.GetDirectory().GetName())) # Before exiting the context, objects can be written to the file f.WriteObject(histo_2, "my_histogram") From a2f151556d0a87e24774f2e4d633e8e905462384 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 9 Sep 2025 11:02:34 +0200 Subject: [PATCH 11/39] [PyROOT] Prepare for implicit object ownership = off. Slightly adapt ownership test to be independent of whether ROOT's implicit ownership settings are on or off. --- bindings/pyroot/pythonizations/test/memory.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/bindings/pyroot/pythonizations/test/memory.py b/bindings/pyroot/pythonizations/test/memory.py index 71c43302718d8..e68b5a59bd02d 100644 --- a/bindings/pyroot/pythonizations/test/memory.py +++ b/bindings/pyroot/pythonizations/test/memory.py @@ -127,11 +127,14 @@ def _check_object_setdirectory(self, klass, classname, args): "_check_object_setdirectory_in_memory_file_begin", "recreate") x = klass(*args) - # TEfficiency does not automatically register with the directory - if not classname == "TEfficiency": - self.assertIs(x.GetDirectory(), f1) - x.SetDirectory(ROOT.nullptr) + # Actively register the object in case implicit ownership is off: + if not x.GetDirectory(): + x.SetDirectory(f1) + + self.assertIs(x.GetDirectory(), f1) + x.SetDirectory(ROOT.nullptr) self.assertFalse(x.GetDirectory()) + # Make sure that at this point the ownership of the object is with Python ROOT.SetOwnership(x, True) From 9bf6e9792955334d0d005d68374328eccd92ad49 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 9 Sep 2025 11:24:45 +0200 Subject: [PATCH 12/39] [tutorials] Make hsimple ready for implicit object ownership=off. --- tutorials/hsimple.C | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tutorials/hsimple.C b/tutorials/hsimple.C index 52fdd50c8e3ef..db2efc5597d2c 100644 --- a/tutorials/hsimple.C +++ b/tutorials/hsimple.C @@ -60,11 +60,14 @@ TFile *hsimple(Int_t getFile=0) hfile = (TFile*)gROOT->FindObject(filename); if (hfile) hfile->Close(); hfile = new TFile(filename,"RECREATE","Demo ROOT file with histograms"); - // Create some histograms, a profile histogram and an ntuple + // Create some histograms, a profile histogram and an ntuple, add them to hfile TH1F *hpx = new TH1F("hpx","This is the px distribution",100,-4,4); + hpx->SetDirectory(hfile); hpx->SetFillColor(48); TH2F *hpxpy = new TH2F("hpxpy","py vs px",40,-4,4,40,-4,4); + hpxpy->SetDirectory(hfile); TProfile *hprof = new TProfile("hprof","Profile of pz versus px",100,-4,4,0,20); + hprof->SetDirectory(hfile); TNtuple *ntuple = new TNtuple("ntuple","Demo ntuple","px:py:pz:random:i"); gBenchmark->Start("hsimple"); From 00c518e9b1bcb61bc61be3e420389e1678bc5def Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 9 Sep 2025 11:28:30 +0200 Subject: [PATCH 13/39] [io] Prepare TFileMergerTest for implicit ownership = off. --- io/io/test/TFileMergerTests.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/io/io/test/TFileMergerTests.cxx b/io/io/test/TFileMergerTests.cxx index 3d790f80f13a2..86ee41bade459 100644 --- a/io/io/test/TFileMergerTests.cxx +++ b/io/io/test/TFileMergerTests.cxx @@ -91,6 +91,7 @@ TEST(TFileMerger, MergeSingleOnlyListed) auto hist2 = new TH1F("hist2", "hist2", 1 , 0 , 2); auto hist3 = new TH1F("hist3", "hist3", 1 , 0 , 2); auto hist4 = new TH1F("hist4", "hist4", 1 , 0 , 2); + for (auto hist : {hist1, hist2, hist3, hist4}) hist->SetDirectory(&a); hist1->Fill(1); hist2->Fill(1); hist2->Fill(2); hist3->Fill(1); hist3->Fill(1); hist3->Fill(1); From b75375b089a87419cf038e81d887e0b036114501 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 9 Sep 2025 11:32:00 +0200 Subject: [PATCH 14/39] [roottest] Prepare execFileMerger for implicit ownership = off. --- roottest/root/io/filemerger/execFileMerger.C | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/roottest/root/io/filemerger/execFileMerger.C b/roottest/root/io/filemerger/execFileMerger.C index 70eaed8a4c55f..9e44e1bbe1d5d 100644 --- a/roottest/root/io/filemerger/execFileMerger.C +++ b/roottest/root/io/filemerger/execFileMerger.C @@ -6,13 +6,14 @@ #include "THStack.h" #include "TTree.h" -void createInputs(int n = 2) +void createInputs(int n = 2) { for(UInt_t i = 0; i < (UInt_t)n; ++i ) { TFile *file = TFile::Open(TString::Format("input%d.root",i),"RECREATE"); TH1F * h = new TH1F("h1","",10,0,100); + h->SetDirectory(file); h->Fill(10.5); h->Fill(20.5); - + Int_t nbins[5]; Double_t xmin[5]; Double_t xmax[5]; From c4d7e3272478033be7c6b8e8a7b28b1c7a2b7623 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 9 Sep 2025 11:32:33 +0200 Subject: [PATCH 15/39] [NFC] Fix stray white spaces. --- roottest/root/io/filemerger/execFileMerger.C | 22 ++++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/roottest/root/io/filemerger/execFileMerger.C b/roottest/root/io/filemerger/execFileMerger.C index 9e44e1bbe1d5d..072c0b1d1a23c 100644 --- a/roottest/root/io/filemerger/execFileMerger.C +++ b/roottest/root/io/filemerger/execFileMerger.C @@ -24,7 +24,7 @@ void createInputs(int n = 2) Double_t coord[5] = {0.5, 1.5, 2.5, 3.5, 4.5}; sparse->Fill(coord); sparse->Write(); - + THStack *stack = new THStack("stack",""); h = new TH1F("hs_1","",10,0,100); h->Fill(10.5); h->Fill(20.5); @@ -41,9 +41,9 @@ void createInputs(int n = 2) gr->SetPoint(0,1,1); gr->SetPoint(1,2,2); gr->SetPoint(2,3,3); - + gr->Write(); - + TTree *tree = new TTree("tree","simplistic tree"); Int_t data = 0; tree->Branch("data",&data); @@ -51,7 +51,7 @@ void createInputs(int n = 2) data = l; tree->Fill(); } - + file->Write(); delete file; } @@ -84,7 +84,7 @@ bool check(int n = 2) { Error("execFileMerger","h1 not added properly"); result = false; } - + THnSparseF *sparse; file->GetObject("sparse",sparse); if (!sparse) { Error("execFileMerger","sparse is missing\n"); @@ -104,7 +104,7 @@ bool check(int n = 2) { result = false; } } - + THStack *stack; file->GetObject("stack",stack); if (!stack) { Error("execFileMerger","stack is missing\n"); @@ -136,18 +136,18 @@ bool check(int n = 2) { } if (gr->GetN() != ( n * 3)) { Error("execFileMerger","exgraph not added properly n=%d rather than %d",gr->GetN(),n*3); - result = false; + result = false; } else { for(Int_t k = 0; k < gr->GetN(); ++k) { double x,y; gr->GetPoint(k,x,y); if ( x != ( (k%3)+1 ) || y != ( (k%3)+1 ) ) { Error("execFileMerger","exgraph not added properly"); - result = false; + result = false; } } } - + TTree *tree; file->GetObject("tree",tree); if (!tree) { Error("execFileMerger","tree is missing\n"); @@ -155,14 +155,14 @@ bool check(int n = 2) { } if (tree->GetEntries() != n*2) { Error("execFileMerger","tree does not have the expected number of entries: %lld rather than %d",tree->GetEntries(),n*2); - result = false; + result = false; } else { if ( tree->GetEntries("data==1") != n ) { Error("execFileMerger","tree does not have the expected data. We got %lld entries with 'data==1' rather than %d",tree->GetEntries("data==1"),n); tree->Scan(); result = false; } - } + } return result; } From 088b2798619debadf8bef3621defd78265d1f1c5 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 9 Sep 2025 13:34:21 +0200 Subject: [PATCH 16/39] [hist] Ensure that items in THStack have the cleanup bit set. When histograms get added to a THStack, they are likely in multiple TLists. Therefore, the kMustCleanup bit must be set. In the past, the bit usually got set automatically when histograms added themselves to directories, but when this is disabled, the cleanup bit remained unset. --- hist/hist/src/THStack.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/hist/hist/src/THStack.cxx b/hist/hist/src/THStack.cxx index f56e7eeaeb155..5dfbe51e4a8fe 100644 --- a/hist/hist/src/THStack.cxx +++ b/hist/hist/src/THStack.cxx @@ -370,6 +370,7 @@ void THStack::Add(TH1 *h1, Option_t *option) } if (!fHists) fHists = new TList(); fHists->Add(h1,option); + h1->SetBit(kMustCleanup); // The histogram is likely in multiple lists now Modified(); //invalidate stack } From e09adf74f079163dedbc424cd532e7ccd2152c07 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 9 Sep 2025 13:42:41 +0200 Subject: [PATCH 17/39] [hf] Prepare HistFactory for ROOT without implicit ownership. --- roofit/histfactory/test/testHistFactoryPlotting.cxx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/roofit/histfactory/test/testHistFactoryPlotting.cxx b/roofit/histfactory/test/testHistFactoryPlotting.cxx index 0f6b91ac50d2e..d610ba1490127 100644 --- a/roofit/histfactory/test/testHistFactoryPlotting.cxx +++ b/roofit/histfactory/test/testHistFactoryPlotting.cxx @@ -58,7 +58,10 @@ void createToyHistos1D() histoData->Fill(x_sig); } - histoFile->Write(); + for (auto histo : {histoBG, histoSig, histoData}) { + histoFile->WriteTObject(histo); + delete histo; + } } /// Test that plotting HistFactory components works correctly. Covers the From 3189fd2d18c6c5c5c65fd8e826818b56b190b4b7 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 9 Sep 2025 13:49:15 +0200 Subject: [PATCH 18/39] [RF] Prepare RooFit for ROOT without implicit ownership. Also add an assertion to stop the test when executed in the wrong directory. Previously, the test would crash. --- roofit/roofitcore/test/testRooDataSet.cxx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/roofit/roofitcore/test/testRooDataSet.cxx b/roofit/roofitcore/test/testRooDataSet.cxx index 6d1b245ab3617..604006102eaba 100644 --- a/roofit/roofitcore/test/testRooDataSet.cxx +++ b/roofit/roofitcore/test/testRooDataSet.cxx @@ -170,6 +170,7 @@ TEST(RooDataSet, ReducingData) for (int i = 0; i < 3; ++i) { // Check with root: TH1F test_hist(("h" + std::to_string(i)).c_str(), "histo", 10, massmin, massmax); + gDirectory->Append(&test_hist, true); // TTree::Draw needs to find the histogram chi2cutval += 0.5; std::stringstream cutString; @@ -439,6 +440,7 @@ TEST(RooDataSet, SplitDataSetWithWeightErrors) TEST(RooDataSet, ReadDataSetWithErrors626) { std::unique_ptr file{TFile::Open("dataSet_with_errors_6_26_10.root", "READ")}; + ASSERT_NE(file, nullptr); auto data = file->Get("data"); From 917f546af9db1011918ebb88d33a6960f836a562 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 9 Sep 2025 15:51:24 +0200 Subject: [PATCH 19/39] [PyROOT] Remove a test that worked just by chance. The test broke when implicit object ownership was switched off in ROOT. However, this test can also break in other circumstances, because del histogram generally does not destroy the C++ object immediately. It used to work because only a single reference to the object was alive (which changed when trying to prepare the test for implicit object ownership off), but one cannot base a test on undocumented details of a Python implementation. For this reason, it does not make sense to test if gDirectory (C++) contains an entry for the object that was just "del"-ed. --- roottest/python/memory/PyROOT_memorytests.py | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/roottest/python/memory/PyROOT_memorytests.py b/roottest/python/memory/PyROOT_memorytests.py index d1c7cbd6c0ed8..d5ce70dcdc0ab 100644 --- a/roottest/python/memory/PyROOT_memorytests.py +++ b/roottest/python/memory/PyROOT_memorytests.py @@ -60,21 +60,6 @@ def test1ObjectCreationDestruction( self ): del b, c self.assertEqual( MemTester.counter, 0 ) - def test2ObjectDestructionCallback( self ): - """Test ROOT notification on object destruction""" - - # create ROOT traced object - a = TH1F( 'memtest_th1f', 'title', 100, -1., 1. ) - - # locate it - self.assertTrue( a is gROOT.FindObject( 'memtest_th1f' ) ) - - # destroy it - del a - - # should no longer be accessible - self.assertTrue( not gROOT.FindObject( 'memtest_th1f' ) ) - def test3ObjectCallHeuristics( self ): """Test memory mgmt heuristics for object calls""" @@ -139,7 +124,7 @@ def test4DestructionOfDerivedClass( self ): """Derived classes should call base dtor automatically""" MemTester = ROOT.MemTester - + class D1( MemTester ): def __init__( self ): MemTester.__init__( self ) From b1ea3e2c74f98148470d027f66718a0f473054ef Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 9 Sep 2025 16:55:41 +0200 Subject: [PATCH 20/39] [PyROOT] Prepare tests for ROOT with implicit ownership = off. --- bindings/pyroot/pythonizations/test/tdirectory_attrsyntax.py | 2 ++ .../pyroot/pythonizations/test/tdirectoryfile_attrsyntax_get.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/bindings/pyroot/pythonizations/test/tdirectory_attrsyntax.py b/bindings/pyroot/pythonizations/test/tdirectory_attrsyntax.py index 814cc87628d0c..7687d9e9c373b 100644 --- a/bindings/pyroot/pythonizations/test/tdirectory_attrsyntax.py +++ b/bindings/pyroot/pythonizations/test/tdirectory_attrsyntax.py @@ -25,11 +25,13 @@ def setUpClass(cls): dir1.cd() h1 = ROOT.TH1F("h1", "h1", cls.nbins, cls.xmin, cls.xmax) ROOT.SetOwnership(h1, False) + h1.SetDirectory(dir1) dir2 = dir1.mkdir("dir2") dir2.cd() h2 = ROOT.TH1F("h2", "h2", cls.nbins, cls.xmin, cls.xmax) ROOT.SetOwnership(h2, False) + h2.SetDirectory(dir2) def checkHisto(self, h): xaxis = h.GetXaxis() diff --git a/bindings/pyroot/pythonizations/test/tdirectoryfile_attrsyntax_get.py b/bindings/pyroot/pythonizations/test/tdirectoryfile_attrsyntax_get.py index a4d288f4337b0..e4ba206ba2d50 100644 --- a/bindings/pyroot/pythonizations/test/tdirectoryfile_attrsyntax_get.py +++ b/bindings/pyroot/pythonizations/test/tdirectoryfile_attrsyntax_get.py @@ -25,11 +25,13 @@ def setUpClass(cls): dir1.cd() h1 = ROOT.TH1F("h1", "h1", cls.nbins, cls.xmin, cls.xmax) ROOT.SetOwnership(h1, False) + h1.SetDirectory(dir1) dir2 = dir1.mkdir("dir2") dir2.cd() h2 = ROOT.TH1F("h2", "h2", cls.nbins, cls.xmin, cls.xmax) ROOT.SetOwnership(h2, False) + h2.SetDirectory(dir2) def checkHisto(self, h): xaxis = h.GetXaxis() From b88100c5dfeee394ed855052d2647f6fbdc27959 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 9 Sep 2025 16:57:17 +0200 Subject: [PATCH 21/39] [hist] Actively add histograms to ROOT directory in FitSlices / Project. The function documentation explicitly states that the histograms created by these operations are added to the current directory, so this is done explicitly here. --- hist/hist/src/TH2.cxx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hist/hist/src/TH2.cxx b/hist/hist/src/TH2.cxx index 43d773a7c8311..b28753047db88 100644 --- a/hist/hist/src/TH2.cxx +++ b/hist/hist/src/TH2.cxx @@ -854,6 +854,7 @@ void TH2::DoFitSlices(bool onX, } else { hlist[ipar] = new TH1D(name,title, nOutBins, &bins->fArray[firstOutBin-1]); } + hlist[ipar]->SetDirectory(gDirectory); hlist[ipar]->GetXaxis()->SetTitle(outerAxis.GetTitle()); if (arr) (*arr)[ipar] = hlist[ipar]; @@ -866,6 +867,7 @@ void TH2::DoFitSlices(bool onX, } else { hchi2 = new TH1D(name,"chisquare", nOutBins, &bins->fArray[firstOutBin-1]); } + hchi2->SetDirectory(gDirectory); hchi2->GetXaxis()->SetTitle(outerAxis.GetTitle()); if (arr) (*arr)[npar] = hchi2; @@ -2126,7 +2128,8 @@ TProfile *TH2::ProfileY(const char *name, Int_t firstxbin, Int_t lastxbin, Optio //////////////////////////////////////////////////////////////////////////////// /// Internal (protected) method for performing projection on the X or Y axis -/// called by ProjectionX or ProjectionY +/// called by ProjectionX or ProjectionY. +/// The histograms created are added to gDirectory. TH1D *TH2::DoProjection(bool onX, const char *name, Int_t firstbin, Int_t lastbin, Option_t *option) const { @@ -2239,6 +2242,7 @@ TH1D *TH2::DoProjection(bool onX, const char *name, Int_t firstbin, Int_t lastbi else h1 = new TH1D(pname,GetTitle(),lastOutBin-firstOutBin+1,&bins->fArray[firstOutBin-1]); } + h1->SetDirectory(gDirectory); if (opt.Contains("e") || GetSumw2N() ) h1->Sumw2(); } if (pname != name) delete [] pname; From 67845211fbebd678d8fb0e4050692a1b7ed25f00 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 9 Sep 2025 17:02:42 +0200 Subject: [PATCH 22/39] [SPLIT?] Prepare hist and directory for ROOT without implicit ownership. --- roottest/root/hist/misc/runownership.C | 2 ++ .../root/io/directory/testFindObjectAny.C | 27 ++++++++++--------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/roottest/root/hist/misc/runownership.C b/roottest/root/hist/misc/runownership.C index c5261e742d1ce..8fbf502cacc1e 100644 --- a/roottest/root/hist/misc/runownership.C +++ b/roottest/root/hist/misc/runownership.C @@ -28,7 +28,9 @@ void write(const char *filename = "histo.root") { TFile * f = TFile::Open(filename,"RECREATE"); TH1F *histo = new TH1F_inst("h1","h1",10,0,10); histo->Fill(3); + histo->SetDirectory(f); histo = new TH1F_inst("h2","h2",10,0,10); histo->Fill(3); + histo->SetDirectory(f); TCanvas *c1 = new TCanvas("c1"); histo->SetBit(kCanDelete); histo->Draw(); diff --git a/roottest/root/io/directory/testFindObjectAny.C b/roottest/root/io/directory/testFindObjectAny.C index 915471400d970..323b637f8af73 100644 --- a/roottest/root/io/directory/testFindObjectAny.C +++ b/roottest/root/io/directory/testFindObjectAny.C @@ -2,12 +2,12 @@ void doit() { TFile* base = new TFile("f.db","recreate"); TDirectory* a = base->mkdir("a","First Level Dir"); - a->cd(); TH1D* ha = new TH1D("ha","ha",10,0,1); + a->Append(ha, true); TDirectory* aa = a->mkdir("aa","Second Level Dira"); - aa->cd(); TH1D* haa = new TH1D("haa","haa",10,0,1); - + aa->Append(haa, true); + a->ls(); printf(" a: created@ %p found@ %p\n", a,base->FindObjectAny("a")); @@ -92,13 +92,13 @@ void testing(TObject *orig, TObject *found) } } -int testFindObjectAny() -{ - TDirectory* db = gROOT->mkdir("db","db"); - TDirectory* a = db->mkdir("a","a"); - TDirectory* aa = a->mkdir("aa","aa"); - aa->cd(); - TH1D* haa_new = new TH1D("haa","haa",10,0,1); +int testFindObjectAny() +{ + TDirectory* db = gROOT->mkdir("db","db"); + TDirectory* a = db->mkdir("a","a"); + TDirectory* aa = a->mkdir("aa","aa"); + TH1D* haa_new = new TH1D("haa","haa",10,0,1); + aa->Append(haa_new, true); TH1D* haa_find = (TH1D*)db->FindObjectAny("haa"); #ifdef ClingWorkAroundMissingDynamicScope TH1D* haa = haa_find; @@ -108,15 +108,15 @@ int testFindObjectAny() } else if (haa_new != haa_find) { cout << "haa not found correctly!\n"; } - + TFile* base = new TFile("fdb.root","recreate"); #ifdef ClingReinstateRedeclarationAllowed TDirectory* a = base->mkdir("a","First Level Dir"); #else a = base->mkdir("a","First Level Dir"); #endif - a->cd(); TH1D* ha = new TH1D("ha","ha",10,0,1); + a->Append(ha, true); #ifdef ClingReinstateRedeclarationAllowed TDirectory* aa = a->mkdir("aa","Second Level Dira"); #else @@ -128,7 +128,8 @@ int testFindObjectAny() #else TH1D* haa = new TH1D("haa","haa",10,0,1); #endif - + aa->Append(haa, true); + testing( a, base->FindObjectAny("a")); testing( ha, base->FindObjectAny("ha")); testing( ha, a->FindObjectAny("ha")); From 74c62f41b2d39e930cce762231d5ff366ab5ea0c Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 16 Sep 2025 16:43:58 +0200 Subject: [PATCH 23/39] [core] Make TDirectory::Append tolerant to identical objects. When calling TDirectory::Append(obj, replace=true), and obj is already in the directory, return without doing anything. If obj has the same name as an existing object, but is physically different, the usual warning is raised. This will enable workflows such as: auto h = new TH1D(...); directory->Append(h, true); After this change, the above lines work both with implicit object ownership off and on. This pattern will allow writing robust programs with consistent behaviour. --- core/base/src/TDirectory.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/core/base/src/TDirectory.cxx b/core/base/src/TDirectory.cxx index 9bebee0e74a1d..9211d68018290 100644 --- a/core/base/src/TDirectory.cxx +++ b/core/base/src/TDirectory.cxx @@ -205,6 +205,7 @@ void TDirectory::Append(TObject *obj, Bool_t replace /* = kFALSE */) if (replace && obj->GetName() && obj->GetName()[0]) { TObject *old; while (nullptr != (old = GetList()->FindObject(obj->GetName()))) { + if (old == obj) return; Warning("Append","Replacing existing %s: %s (Potential memory leak).", obj->IsA()->GetName(),obj->GetName()); ROOT::DirAutoAdd_t func = old->IsA()->GetDirectoryAutoAdd(); From e1768a22bb286491f7e2926fecf84a85d82a6d10 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 16 Sep 2025 17:38:04 +0200 Subject: [PATCH 24/39] Prepare test/stress for ROOT without implicit object ownership. Explicitly assign histograms to directories. In this way, the tests proceed irrespective of whether or not implicit object ownership is in use. --- test/stress.cxx | 81 ++++++++++++++++++++++++++++--------------------- 1 file changed, 46 insertions(+), 35 deletions(-) diff --git a/test/stress.cxx b/test/stress.cxx index 4db3b572c5b7a..a5f4ed44866ef 100644 --- a/test/stress.cxx +++ b/test/stress.cxx @@ -1,5 +1,6 @@ // @(#)root/test:$Id$ // Author: Rene Brun 05/11/98 +// clang-format off ///////////////////////////////////////////////////////////////// // @@ -577,9 +578,11 @@ void stress6() snprintf(hname,20,"h%d_%dN",i,j); snprintf(htitle,80,"hist for counter:%d in plane:%d North",j,i); hn[j] = new TH1S(hname,htitle,100,0,100); + cdplane->Append(hn[j], true); snprintf(hname,20,"h%d_%dS",i,j); snprintf(htitle,80,"hist for counter:%d in plane:%d South",j,i); hs[j] = new TH1S(hname,htitle,100,0,100); + cdplane->Append(hs[j], true); } // fill counter histograms randomly for (k=0;k<10000;k++) { @@ -671,6 +674,7 @@ void stress7() cutg->SetPoint(7,-1.27161,1.01523); cutg->SetPoint(8,-1.75713,2.46193); TH2F *hpxpy = new TH2F("hpxpy","px vx py with cutg",40,-4,4,40,-4,4); + f.Append(hpxpy, true); ntuple->Draw("px:py>>hpxpy","cutg","goff"); Int_t npxpy = (Int_t)hpxpy->GetEntries(); Int_t npxpyGood = 27918; @@ -1018,40 +1022,40 @@ void stress9tree(TTree *tree, Int_t realTestNum) //We make clones of the generated histograms //We set new names and reset the clones. //We want to have identical histogram limits - TH1F *bNtrack = (TH1F*)hNtrack->Clone(); bNtrack->SetName("bNtrack"); bNtrack->Reset(); - TH1F *bNseg = (TH1F*)hNseg->Clone(); bNseg->SetName("bNseg"); bNseg->Reset(); - TH1F *bTemp = (TH1F*)hTemp->Clone(); bTemp->SetName("bTemp"); bTemp->Reset(); - TH1F *bHmean = (TH1F*)hHmean->Clone(); bHmean->SetName("bHmean"); bHmean->Reset(); - TH1F *bPx = (TH1F*)hPx->Clone(); bPx->SetName("bPx"); bPx->Reset(); - TH1F *bPy = (TH1F*)hPy->Clone(); bPy->SetName("bPy"); bPy->Reset(); - TH1F *bPz = (TH1F*)hPz->Clone(); bPz->SetName("bPz"); bPz->Reset(); - TH1F *bRandom = (TH1F*)hRandom->Clone(); bRandom->SetName("bRandom"); bRandom->Reset(); - TH1F *bMass2 = (TH1F*)hMass2->Clone(); bMass2->SetName("bMass2"); bMass2->Reset(); - TH1F *bBx = (TH1F*)hBx->Clone(); bBx->SetName("bBx"); bBx->Reset(); - TH1F *bBy = (TH1F*)hBy->Clone(); bBy->SetName("bBy"); bBy->Reset(); - TH1F *bXfirst = (TH1F*)hXfirst->Clone(); bXfirst->SetName("bXfirst"); bXfirst->Reset(); - TH1F *bYfirst = (TH1F*)hYfirst->Clone(); bYfirst->SetName("bYfirst"); bYfirst->Reset(); - TH1F *bZfirst = (TH1F*)hZfirst->Clone(); bZfirst->SetName("bZfirst"); bZfirst->Reset(); - TH1F *bXlast = (TH1F*)hXlast->Clone(); bXlast->SetName("bXlast"); bXlast->Reset(); - TH1F *bYlast = (TH1F*)hYlast->Clone(); bYlast->SetName("bYlast"); bYlast->Reset(); - TH1F *bZlast = (TH1F*)hZlast->Clone(); bZlast->SetName("bZlast"); bZlast->Reset(); - TH1F *bCharge = (TH1F*)hCharge->Clone(); bCharge->SetName("bCharge"); bCharge->Reset(); - TH1F *bNpoint = (TH1F*)hNpoint->Clone(); bNpoint->SetName("bNpoint"); bNpoint->Reset(); - TH1F *bValid = (TH1F*)hValid->Clone(); bValid->SetName("bValid"); bValid->Reset(); - - TH1F *bFullMatrix =(TH1F*)hFullMatrix->Clone(); bFullMatrix->SetName("bFullMatrix"); bFullMatrix->Reset(); - TH1F *bColMatrix = (TH1F*)hColMatrix->Clone(); bColMatrix->SetName("bColMatrix"); bColMatrix->Reset(); - TH1F *bRowMatrix = (TH1F*)hRowMatrix->Clone(); bRowMatrix->SetName("bRowMatrix"); bRowMatrix->Reset(); - TH1F *bCellMatrix = (TH1F*)hCellMatrix->Clone(); bCellMatrix->SetName("bCellMatrix"); bCellMatrix->Reset(); - TH1F *bFullOper = (TH1F*)hFullOper->Clone(); bFullOper->SetName("bFullOper"); bFullOper->Reset(); - TH1F *bCellOper = (TH1F*)hCellOper->Clone(); bCellOper->SetName("bCellOper"); bCellOper->Reset(); - TH1F *bColOper = (TH1F*)hColOper->Clone(); bColOper->SetName("bColOper"); bColOper->Reset(); - TH1F *bRowOper = (TH1F*)hRowOper->Clone(); bRowOper->SetName("bRowOper"); bRowOper->Reset(); - TH1F *bMatchRowOper = (TH1F*)hMatchRowOper->Clone(); bMatchRowOper->SetName("bMatchRowOper"); bMatchRowOper->Reset(); - TH1F *bMatchColOper = (TH1F*)hMatchColOper->Clone(); bMatchColOper->SetName("bMatchColOper"); bMatchColOper->Reset(); - TH1F *bRowMatOper = (TH1F*)hRowMatOper->Clone(); bRowMatOper->SetName("bRowMatOper"); bRowMatOper->Reset(); - TH1F *bMatchDiffOper= (TH1F*)hMatchDiffOper->Clone(); bMatchDiffOper->SetName("bMatchDiffOper"); bMatchDiffOper->Reset(); - TH1F *bFullOper2 = (TH1F*)hFullOper2->Clone(); bFullOper2->SetName("bFullOper2"); bFullOper2->Reset(); + TH1F *bNtrack = (TH1F*)hNtrack->Clone("bNtrack"); bNtrack->SetDirectory(hfile); bNtrack->Reset(); + TH1F *bNseg = (TH1F*)hNseg->Clone("bNseg"); bNseg->SetDirectory(hfile); bNseg->Reset(); + TH1F *bTemp = (TH1F*)hTemp->Clone("bTemp"); bTemp->SetDirectory(hfile); bTemp->Reset(); + TH1F *bHmean = (TH1F*)hHmean->Clone("bHmean"); bHmean->SetDirectory(hfile); bHmean->Reset(); + TH1F *bPx = (TH1F*)hPx->Clone("bPx"); bPx->SetDirectory(hfile); bPx->Reset(); + TH1F *bPy = (TH1F*)hPy->Clone("bPy"); bPy->SetDirectory(hfile); bPy->Reset(); + TH1F *bPz = (TH1F*)hPz->Clone("bPz"); bPz->SetDirectory(hfile); bPz->Reset(); + TH1F *bRandom = (TH1F*)hRandom->Clone("bRandom"); bRandom->SetDirectory(hfile); bRandom->Reset(); + TH1F *bMass2 = (TH1F*)hMass2->Clone("bMass2"); bMass2->SetDirectory(hfile); bMass2->Reset(); + TH1F *bBx = (TH1F*)hBx->Clone("bBx"); bBx->SetDirectory(hfile); bBx->Reset(); + TH1F *bBy = (TH1F*)hBy->Clone("bBy"); bBy->SetDirectory(hfile); bBy->Reset(); + TH1F *bXfirst = (TH1F*)hXfirst->Clone("bXfirst"); bXfirst->SetDirectory(hfile); bXfirst->Reset(); + TH1F *bYfirst = (TH1F*)hYfirst->Clone("bYfirst"); bYfirst->SetDirectory(hfile); bYfirst->Reset(); + TH1F *bZfirst = (TH1F*)hZfirst->Clone("bZfirst"); bZfirst->SetDirectory(hfile); bZfirst->Reset(); + TH1F *bXlast = (TH1F*)hXlast->Clone("bXlast"); bXlast->SetDirectory(hfile); bXlast->Reset(); + TH1F *bYlast = (TH1F*)hYlast->Clone("bYlast"); bYlast->SetDirectory(hfile); bYlast->Reset(); + TH1F *bZlast = (TH1F*)hZlast->Clone("bZlast"); bZlast->SetDirectory(hfile); bZlast->Reset(); + TH1F *bCharge = (TH1F*)hCharge->Clone("bCharge"); bCharge->SetDirectory(hfile); bCharge->Reset(); + TH1F *bNpoint = (TH1F*)hNpoint->Clone("bNpoint"); bNpoint->SetDirectory(hfile); bNpoint->Reset(); + TH1F *bValid = (TH1F*)hValid->Clone("bValid"); bValid->SetDirectory(hfile); bValid->Reset(); + + TH1F *bFullMatrix =(TH1F*)hFullMatrix->Clone("bFullMatrix"); bFullMatrix->SetDirectory(hfile); bFullMatrix->Reset(); + TH1F *bColMatrix = (TH1F*)hColMatrix->Clone("bColMatrix"); bColMatrix->SetDirectory(hfile); bColMatrix->Reset(); + TH1F *bRowMatrix = (TH1F*)hRowMatrix->Clone("bRowMatrix"); bRowMatrix->SetDirectory(hfile); bRowMatrix->Reset(); + TH1F *bCellMatrix = (TH1F*)hCellMatrix->Clone("bCellMatrix"); bCellMatrix->SetDirectory(hfile); bCellMatrix->Reset(); + TH1F *bFullOper = (TH1F*)hFullOper->Clone("bFullOper"); bFullOper->SetDirectory(hfile); bFullOper->Reset(); + TH1F *bCellOper = (TH1F*)hCellOper->Clone("bCellOper"); bCellOper->SetDirectory(hfile); bCellOper->Reset(); + TH1F *bColOper = (TH1F*)hColOper->Clone("bColOper"); bColOper->SetDirectory(hfile); bColOper->Reset(); + TH1F *bRowOper = (TH1F*)hRowOper->Clone("bRowOper"); bRowOper->SetDirectory(hfile); bRowOper->Reset(); + TH1F *bMatchRowOper = (TH1F*)hMatchRowOper->Clone("bMatchRowOper"); bMatchRowOper->SetDirectory(hfile); bMatchRowOper->Reset(); + TH1F *bMatchColOper = (TH1F*)hMatchColOper->Clone("bMatchColOper"); bMatchColOper->SetDirectory(hfile); bMatchColOper->Reset(); + TH1F *bRowMatOper = (TH1F*)hRowMatOper->Clone("bRowMatOper"); bRowMatOper->SetDirectory(hfile); bRowMatOper->Reset(); + TH1F *bMatchDiffOper= (TH1F*)hMatchDiffOper->Clone("bMatchDiffOper");bMatchDiffOper->SetDirectory(hfile); bMatchDiffOper->Reset(); + TH1F *bFullOper2 = (TH1F*)hFullOper2->Clone("bFullOper2"); bFullOper2->SetDirectory(hfile); bFullOper2->Reset(); // Loop with user code on all events and fill the b histograms // The code below should produce identical results to the tree->Draw above @@ -1339,9 +1343,16 @@ void stress12(Int_t testid) if (strcmp(key->GetClassName(),"TH1F")) continue; //may be a TList of TStreamerInfo h9 = (TH1F*)f9.Get(key->GetName()); h11 = (TH1F*)f11.Get(key->GetName()); - if (h9 == 0 || h11 == 0) continue; + if (h9 == 0 || h11 == 0) { + std::cerr << "Missing " << key->GetName(); + if (!h9) std::cerr << " in stress_test9.root"; + if (!h11) std::cerr << " in stress_test11.root"; + std::cerr << "\n"; + continue; + } comp = HistCompare(h9,h11); if (comp == 0) ngood++; + else std::cerr << key->GetName() << " not equal\n"; } ntotin += f9.GetBytesRead(); ntotin += f11.GetBytesRead(); From 72f19bcf23cb7db0fe1bc17822109099417b7ae4 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Wed, 17 Sep 2025 15:57:08 +0200 Subject: [PATCH 25/39] [roottest] Fix output file path in nbdiff.py nbconvert was creating an invalid path when creating an output notebook. This seems to result from a behaviour change that doesn't anymore trim a redundant path from a notebook name ("a/b/notebook" was transformed to "notebook" if the input notebook is in a/b). In nbconvert 7.16.6, this became "a/b/a/b/notebook". Since nbconvert 5.0, the notebook is anyway placed next to the input notebook instead of the current working directory, so the output path can be removed from the command. --- roottest/python/JupyROOT/nbdiff.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/roottest/python/JupyROOT/nbdiff.py b/roottest/python/JupyROOT/nbdiff.py index 5931c322e0bcc..df89eb1ef6279 100644 --- a/roottest/python/JupyROOT/nbdiff.py +++ b/roottest/python/JupyROOT/nbdiff.py @@ -154,7 +154,8 @@ def canReproduceNotebook(inNBName, kernelName, needsCompare): tmpDir = addEtcToEnvironment(os.path.dirname(inNBName)) outNBName = inNBName.replace(nbExtension,"_out"+nbExtension) interpName = getInterpreterName() - convCmd = convCmdTmpl %(interpName, kernelName, inNBName, outNBName) + convCmd = convCmdTmpl %(interpName, kernelName, inNBName, os.path.basename(outNBName)) + print("Running", convCmd) exitStatus = os.system(convCmd) # we use system to inherit the environment in os.environ shutil.rmtree(tmpDir) if needsCompare: From 62877030d2ef500e02d000a72b7eab4a1f9ae152 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 23 Sep 2025 16:13:43 +0200 Subject: [PATCH 26/39] [Revise] Make TH1 AddDirectory callback independent of ROOT 7 mode. --- hist/hist/src/TH1.cxx | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/hist/hist/src/TH1.cxx b/hist/hist/src/TH1.cxx index 7d09678ca4842..86c41450e7628 100644 --- a/hist/hist/src/TH1.cxx +++ b/hist/hist/src/TH1.cxx @@ -2776,16 +2776,13 @@ TObject* TH1::Clone(const char* newname) const } //////////////////////////////////////////////////////////////////////////////// -/// Perform the automatic addition of the histogram to the given directory +/// Callback to perform the automatic addition of the histogram to the given directory. /// -/// Note this function is called in place when the semantic requires -/// this object to be added to a directory (I.e. when being read from -/// a TKey or being Cloned) +/// This callback is called when a TKey is read or an object is being Cloned. void TH1::DirectoryAutoAdd(TDirectory *dir) { - Bool_t addStatus = TH1::AddDirectoryStatus(); - if (addStatus) { + if (fgAddDirectory) { SetDirectory(dir); if (dir) { ResetBit(kCanDelete); From 6d1738d724f002e1c5de63efe6ed02f8dbe3ae25 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Thu, 23 Oct 2025 15:01:25 +0200 Subject: [PATCH 27/39] [roottest] Make notebook test independent of ownership model. The test was relying on implicit ownership to transfer a histogram from python to C++, so now the histogram is appended explicitly. --- roottest/python/JupyROOT/ROOT_kernel.ipynb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/roottest/python/JupyROOT/ROOT_kernel.ipynb b/roottest/python/JupyROOT/ROOT_kernel.ipynb index ade90d23a76d5..07446900e549f 100644 --- a/roottest/python/JupyROOT/ROOT_kernel.ipynb +++ b/roottest/python/JupyROOT/ROOT_kernel.ipynb @@ -63,7 +63,7 @@ ], "source": [ "%%cpp -a\n", - "#include \n", + "#include \n", "\n", "template\n", "class B{};\n", @@ -172,7 +172,8 @@ "outputs": [], "source": [ "%%python\n", - "h = ROOT.TH1F(\"s\",\"s\",10,0,1)" + "h = ROOT.TH1F(\"s\",\"s\",10,0,1)\n", + "ROOT.gDirectory.Add(h)" ] }, { From 35eb7ae3f92f2c2570d2edde7c18481e81fac8bc Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Fri, 24 Oct 2025 16:39:06 +0200 Subject: [PATCH 28/39] [Tutorials] Solve ownership problem in combinedFit.C When implicit histogram ownership is off, the co-ownership of a function both by the global list of functions and by a histogram lead to a double delete. --- tutorials/math/fit/combinedFit.C | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tutorials/math/fit/combinedFit.C b/tutorials/math/fit/combinedFit.C index 2fd69ead39d04..84a459043bb1a 100644 --- a/tutorials/math/fit/combinedFit.C +++ b/tutorials/math/fit/combinedFit.C @@ -70,19 +70,21 @@ void combinedFit() TH1D *hB = new TH1D("hB", "histo B", 100, 0, 100); TH1D *hSB = new TH1D("hSB", "histo S+B", 100, 0, 100); - TF1 *fB = new TF1("fB", "expo", 0, 100); + // Create functions (not adding them to ROOT's global list, + // because we want to add them to the histograms later) + TF1 *fB = new TF1("fB", "expo", 0, 100, TF1::EAddToList::kNo); fB->SetParameters(1, -0.05); - hB->FillRandom("fB"); + hB->FillRandom(fB); - TF1 *fS = new TF1("fS", "gaus", 0, 100); + TF1 *fS = new TF1("fS", "gaus", 0, 100, TF1::EAddToList::kNo); fS->SetParameters(1, 30, 5); - hSB->FillRandom("fB", 2000); - hSB->FillRandom("fS", 1000); + hSB->FillRandom(fB, 2000); + hSB->FillRandom(fS, 1000); // perform now global fit - TF1 *fSB = new TF1("fSB", "expo + gaus(2)", 0, 100); + TF1 *fSB = new TF1("fSB", "expo + gaus(2)", 0, 100, TF1::EAddToList::kNo); ROOT::Math::WrappedMultiTF1 wfB(*fB, 1); ROOT::Math::WrappedMultiTF1 wfSB(*fSB, 1); From ed91f242f95f47da90757399b2b36746edc3fd8e Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Fri, 24 Oct 2025 17:34:07 +0200 Subject: [PATCH 29/39] [roottest] Make multicore/fork tutorial robust w.r.t. impl. ownership The tutorial was running code that didn't directly reference the histogram, so it failed when implicit ownership is off. --- roottest/root/multicore/commands1.txt | 2 +- roottest/root/multicore/commands2.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/roottest/root/multicore/commands1.txt b/roottest/root/multicore/commands1.txt index 2a657630fae06..af60b04a8a11d 100644 --- a/roottest/root/multicore/commands1.txt +++ b/roottest/root/multicore/commands1.txt @@ -3,5 +3,5 @@ TH1F h1("h","",100,0,1); std::vector> v; THtml h; //gInterpreter->SetClassAutoparsing(false); -h.LoadAllLibs(); +h1.LoadAllLibs(); diff --git a/roottest/root/multicore/commands2.txt b/roottest/root/multicore/commands2.txt index 30e307155af81..34755018a1c61 100644 --- a/roottest/root/multicore/commands2.txt +++ b/roottest/root/multicore/commands2.txt @@ -4,4 +4,4 @@ std::vector> v; std::set stringset; THtml h; //gInterpreter->SetClassAutoparsing(false); -h.LoadAllLibs(); +h1.LoadAllLibs(); From b2912864801a184e1deb3b80ed258b0beb4e4e4b Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Mon, 27 Oct 2025 17:35:12 +0100 Subject: [PATCH 30/39] [RFile] Prepare RFile test for implicit object ownership off. --- io/io/test/rfile.cxx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/io/io/test/rfile.cxx b/io/io/test/rfile.cxx index 2d40607e6e703..7c52ba9dfa398 100644 --- a/io/io/test/rfile.cxx +++ b/io/io/test/rfile.cxx @@ -156,9 +156,10 @@ TEST(RFile, CheckNoAutoRegistrationWrite) EXPECT_EQ(gDirectory, gROOT); auto hist = std::make_unique("hist", "", 100, -10, 10); file->Put("hist", *hist); - EXPECT_EQ(hist->GetDirectory(), gROOT); + TDirectory const * expectedDir = ROOT::Experimental::IsImplicitObjectOwnershipEnabled() ? gROOT : nullptr; + EXPECT_EQ(hist->GetDirectory(), expectedDir); file->Close(); - EXPECT_EQ(hist->GetDirectory(), gROOT); + EXPECT_EQ(hist->GetDirectory(), expectedDir); hist.reset(); // no double free should happen when ROOT exits } From 415a6488fa5a85c2b1059705e6d29f5ecacac13f Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Mon, 3 Nov 2025 11:15:41 +0100 Subject: [PATCH 31/39] Prepare a stressGraphics test for ROOT7 object ownership. --- test/stressGraphics.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/test/stressGraphics.cxx b/test/stressGraphics.cxx index 2f10790463a50..3f24381ca71a2 100644 --- a/test/stressGraphics.cxx +++ b/test/stressGraphics.cxx @@ -3687,6 +3687,7 @@ void hbars() TH1F *hDiv = (TH1F*)gDirectory->Get("hDiv"); hDiv->SetStats(0); TH1F *hDivFR = (TH1F*)hDiv->Clone("hDivFR"); + gDirectory->Append(hDivFR); T->Draw("Division>>hDivFR","Nation==\"FR\"","goff"); hDiv->SetBarWidth(0.45); hDiv->SetBarOffset(0.1); From 1a0ecc215cea29ca8cbd33c0d631187cc5b81ba2 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Thu, 20 Mar 2025 15:17:54 +0100 Subject: [PATCH 32/39] [core] Add ROOT::Experimental::DisableImplicitObjectOwnership() - Add a silentOff mode for running tests that check the program output. --- core/base/inc/TROOT.h | 5 ++ core/base/src/TROOT.cxx | 102 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) diff --git a/core/base/inc/TROOT.h b/core/base/inc/TROOT.h index 1cebafc5a30dd..227000450bcb7 100644 --- a/core/base/inc/TROOT.h +++ b/core/base/inc/TROOT.h @@ -97,6 +97,11 @@ namespace ROOT { void DisableImplicitMT(); Bool_t IsImplicitMTEnabled(); UInt_t GetThreadPoolSize(); + namespace Experimental { + void EnableImplicitObjectOwnership(); + void DisableImplicitObjectOwnerhsip(); + bool IsImplicitObjectOwnershipEnabled(); + } } class TROOT : public TDirectory { diff --git a/core/base/src/TROOT.cxx b/core/base/src/TROOT.cxx index 137000cd371a7..ef8b1a979a127 100644 --- a/core/base/src/TROOT.cxx +++ b/core/base/src/TROOT.cxx @@ -71,6 +71,7 @@ of a main program creating an interactive version is shown below: #include #include "RConfigure.h" #include "RConfigOptions.h" +#include #include #include #include @@ -471,6 +472,49 @@ namespace Internal { return isImplicitMTEnabled; } + //////////////////////////////////////////////////////////////////////////////// + /// \brief Test if objects such as TTree and TH1-derived classes should be implicitly owned + /// by gDirectory. + /// A default can be set in a .rootrc using "Root.ImplicitOwnership: 1" or setting + /// the environment variable "ROOT_IMPLICIT_OWNERSHIP=0". + static std::atomic_bool &IsImplicitOwnershipEnabled() + { + static std::atomic_bool initCompleted = false; + static std::atomic_bool implicitOwnership = true; + + if (!initCompleted.load(std::memory_order_acquire)) { + R__LOCKGUARD(gROOTMutex); + // test again, because another thread might have raced us here + if (!initCompleted) { + int desiredValue = -1; + bool silent = false; + if (auto env = gSystem->Getenv("ROOT_IMPLICIT_OWNERSHIP"); env) { + if (strcmp(env, "silentOff") == 0) { + implicitOwnership = false; + silent = true; + } else { + try { + desiredValue = std::stoi(env); + } catch (std::invalid_argument& e) { } + if (desiredValue < 0) Error("TROOT", "ROOT_IMPLICIT_OWNERSHIP should be >= 0"); + } + } else if (gEnv) { + desiredValue = gEnv->GetValue("Root.ImplicitOwnership", -1); + } + + if (desiredValue == 0) { + if (!silent) Info("TROOT", "Implicit object ownership switched off by ROOT_IMPLICIT_OWNERSHIP or rootrc"); + implicitOwnership = false; + } else if (desiredValue > 0) { + implicitOwnership = true; + } + + initCompleted = true; + } + } + + return implicitOwnership; + } } // end of Internal sub namespace // back to ROOT namespace @@ -616,6 +660,64 @@ namespace Internal { return 0; #endif } + + namespace Experimental { + //////////////////////////////////////////////////////////////////////////////// + /// \brief Switch ROOT's object ownership model to ROOT 6 mode. + /// + /// In ROOT 6 mode, ROOT will implicitly assign ownership of histograms or TTrees + /// to the current \ref gDirectory, for example to the last TFile that was opened. + /// \code{.cpp} + /// TFile file(...); + /// TTree* tree = new TTree(...); + /// TH1D* histo = new TH1D(...); + /// file.Write(); // Both tree and histogram are in the file now + /// \endcode + /// + /// In ROOT 7 mode, these objects won't register themselves to the current gDirectory, + /// so they are fully owned by the user. To write these to files, the user needs to do + /// one of the following: + /// - Explicitly transfer ownership: + /// \code{.cpp} + /// TFile file(...); + /// TTree* tree = new TTree(...); + /// tree->SetDirectory(&file); + /// \endcode + /// - Keep ownership of the object, but write explicitly: + /// \code{.cpp} + /// TFile file(...); + /// std::unique_ptr histo{new TH1D(...)}; + /// file.WriteObject(histo.get(), "HistogramName"); + /// file.Close(); + /// // histo is still valid + /// \endcode + /// + /// \note This setting has higher priority than TH1::AddDirectoryStatus() and TDirectory::AddDirectoryStatus(). + /// These two will always evaluate to false if implicit ownership is off. + /// + /// \copydetails ROOT::Internal::IsImplicitOwnershipEnabled() + void EnableImplicitObjectOwnerhsip() + { + Internal::IsImplicitOwnershipEnabled() = true; + } + + //////////////////////////////////////////////////////////////////////////////// + /// \brief Switch ROOT's object ownership model to ROOT 7 mode (no ownership). + /// \copydetails ROOT::Experimental::EnableImplicitObjectOwnership() + void DisableImplicitObjectOwnerhsip() + { + Internal::IsImplicitOwnershipEnabled() = false; + } + + //////////////////////////////////////////////////////////////////////////////// + /// Test whether the current directory should take ownership of objects such as + /// TH1-derived classes, TTree, TEntryList etc. + /// \copydetails ROOT::Experimental::EnableImplicitObjectOwnership() + bool IsImplicitObjectOwnershipEnabled() + { + return Internal::IsImplicitOwnershipEnabled(); + } +} } // end of ROOT namespace TROOT *ROOT::Internal::gROOTLocal = ROOT::GetROOT(); From 272a3ea07861ec91f381778672458cf41c2c2dd3 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Fri, 5 Sep 2025 16:46:52 +0200 Subject: [PATCH 33/39] [hist] Propagate implicit object registration settings to TH1 and derived. - Couple TH1::AddDirectoryStatus to ROOT::DisableImplicitObjectOwnership. - Add a manual exception for TTree::Draw(): Histograms and profiles will still register themselves to gDirectory because the registration is an essential part of the feature. --- hist/hist/src/TH1.cxx | 19 ++++++++++++------- tree/treeplayer/src/TSelectorDraw.cxx | 6 ++++++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/hist/hist/src/TH1.cxx b/hist/hist/src/TH1.cxx index 86c41450e7628..ce0dfd253a487 100644 --- a/hist/hist/src/TH1.cxx +++ b/hist/hist/src/TH1.cxx @@ -736,11 +736,13 @@ TH1::TH1(const char *name,const char *title,Int_t nbins,const Double_t *xbins) } //////////////////////////////////////////////////////////////////////////////// -/// Static function: cannot be inlined on Windows/NT. +/// Check whether TH1-derived classes are owned by the current gDirectory. +/// \note ROOT::Experimental::IsImplicitObjectOwnershipEnabled() might lead to this +/// setting being always off, since it has higher precedence. Bool_t TH1::AddDirectoryStatus() { - return fgAddDirectory; + return ROOT::Experimental::IsImplicitObjectOwnershipEnabled() && fgAddDirectory; } //////////////////////////////////////////////////////////////////////////////// @@ -1249,16 +1251,19 @@ Bool_t TH1::Add(const TH1 *h1, const TH1 *h2, Double_t c1, Double_t c2) } //////////////////////////////////////////////////////////////////////////////// -/// Sets the flag controlling the automatic add of histograms in memory +/// Sets the flag controlling the automatic add of histograms in memory. /// /// By default (fAddDirectory = kTRUE), histograms are automatically added -/// to the list of objects in memory. +/// to the current directory (gDirectory). /// Note that one histogram can be removed from its support directory /// by calling h->SetDirectory(nullptr) or h->SetDirectory(dir) to add it /// to the list of objects in the directory dir. /// -/// NOTE that this is a static function. To call it, use; -/// TH1::AddDirectory +/// This is a static function. To call it, use `TH1::AddDirectory` +/// +/// \note When ROOT::Experimental::IsImplicitOwnershipEnabled() is off, AddDirectory is +/// without effect. +/// void TH1::AddDirectory(Bool_t add) { @@ -2720,7 +2725,7 @@ void TH1::Copy(TObject &obj) const // will be added to gDirectory independently of the fDirectory stored. // and if the AddDirectoryStatus() is false it will not be added to // any directory (fDirectory = nullptr) - if (fgAddDirectory && gDirectory) { + if (AddDirectoryStatus() && gDirectory) { gDirectory->Append(&obj); ((TH1&)obj).fFunctions->UseRWLock(); ((TH1&)obj).fDirectory = gDirectory; diff --git a/tree/treeplayer/src/TSelectorDraw.cxx b/tree/treeplayer/src/TSelectorDraw.cxx index e1830a6751658..732087ec69d2c 100644 --- a/tree/treeplayer/src/TSelectorDraw.cxx +++ b/tree/treeplayer/src/TSelectorDraw.cxx @@ -578,6 +578,7 @@ void TSelectorDraw::Begin(TTree *tree) } else { hist = new TH1D(hname, htitle.Data(), fNbins[0], fVmin[0], fVmax[0]); } + hist->SetDirectory(gDirectory); hist->SetLineColor(fTree->GetLineColor()); hist->SetLineWidth(fTree->GetLineWidth()); hist->SetLineStyle(fTree->GetLineStyle()); @@ -661,6 +662,7 @@ void TSelectorDraw::Begin(TTree *tree) } else { hp = new TProfile(hname, htitle.Data(), fNbins[1], fVmin[1], fVmax[1], ""); } + hp->SetDirectory(gDirectory); if (!hkeep) { hp->SetBit(kCanDelete); if (!opt.Contains("goff")) hp->SetDirectory(nullptr); @@ -689,6 +691,7 @@ void TSelectorDraw::Begin(TTree *tree) } else { h2 = new TH2D(hname, htitle.Data(), fNbins[1], fVmin[1], fVmax[1], fNbins[0], fVmin[0], fVmax[0]); } + h2->SetDirectory(gDirectory); h2->SetLineColor(fTree->GetLineColor()); h2->SetLineWidth(fTree->GetLineWidth()); h2->SetLineStyle(fTree->GetLineStyle()); @@ -803,6 +806,7 @@ void TSelectorDraw::Begin(TTree *tree) } else { hp = new TProfile2D(hname, htitle.Data(), fNbins[2], fVmin[2], fVmax[2], fNbins[1], fVmin[1], fVmax[1], ""); } + hp->SetDirectory(gDirectory); if (!hkeep) { hp->SetBit(kCanDelete); if (!opt.Contains("goff")) hp->SetDirectory(nullptr); @@ -826,6 +830,7 @@ void TSelectorDraw::Begin(TTree *tree) h2 = (TH2F*)fOldHistogram; } else { h2 = new TH2F(hname, htitle.Data(), fNbins[1], fVmin[1], fVmax[1], fNbins[0], fVmin[0], fVmax[0]); + h2->SetDirectory(gDirectory); h2->SetLineColor(fTree->GetLineColor()); h2->SetLineWidth(fTree->GetLineWidth()); h2->SetLineStyle(fTree->GetLineStyle()); @@ -859,6 +864,7 @@ void TSelectorDraw::Begin(TTree *tree) } else { h3 = new TH3D(hname, htitle.Data(), fNbins[2], fVmin[2], fVmax[2], fNbins[1], fVmin[1], fVmax[1], fNbins[0], fVmin[0], fVmax[0]); } + h3->SetDirectory(gDirectory); h3->SetLineColor(fTree->GetLineColor()); h3->SetLineWidth(fTree->GetLineWidth()); h3->SetLineStyle(fTree->GetLineStyle()); From 385bd2548ef0d35b2004256eb791286ce8d75b52 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Fri, 5 Sep 2025 17:37:28 +0200 Subject: [PATCH 34/39] [core] Propagate implicit object ownership settings to TDirectory. --- core/base/src/TDirectory.cxx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/core/base/src/TDirectory.cxx b/core/base/src/TDirectory.cxx index 9211d68018290..a70193fa22d4f 100644 --- a/core/base/src/TDirectory.cxx +++ b/core/base/src/TDirectory.cxx @@ -178,6 +178,8 @@ TDirectory::TContext::~TContext() /// ~~~ {.cpp} /// TDirectory::AddDirectory /// ~~~ +/// \note When ROOT::Experimental::IsImplicitObjectOwnershipEnabled() is off, these settings +/// are without effect. void TDirectory::AddDirectory(Bool_t add) { @@ -185,11 +187,13 @@ void TDirectory::AddDirectory(Bool_t add) } //////////////////////////////////////////////////////////////////////////////// -/// Static function: see TDirectory::AddDirectory for more comments. - +/// Check whether objects such as histograms or TGraphs2D should be owned by the current directory. +/// \copydetails AddDirectory(Bool_t) +/// \note ROOT::Experimental::IsImplicitObjectOwnershipEnabled() might lead to this +/// setting being always off, since it has higher precedence. Bool_t TDirectory::AddDirectoryStatus() { - return fgAddDirectory; + return ROOT::Experimental::IsImplicitObjectOwnershipEnabled() && fgAddDirectory; } //////////////////////////////////////////////////////////////////////////////// From 13a2d706c698f3b2fb839fca6f85a21668b1be5e Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Fri, 5 Sep 2025 17:43:00 +0200 Subject: [PATCH 35/39] [RF] Propagate implicit object registration settings to RooPlot. --- roofit/roofitcore/src/RooPlot.cxx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/roofit/roofitcore/src/RooPlot.cxx b/roofit/roofitcore/src/RooPlot.cxx index 89cf8818476eb..617b990615ad7 100644 --- a/roofit/roofitcore/src/RooPlot.cxx +++ b/roofit/roofitcore/src/RooPlot.cxx @@ -56,6 +56,7 @@ object onto a one-dimensional plot. #include "TH1D.h" #include "TBrowser.h" #include "TVirtualPad.h" +#include "TROOT.h" #include "TAttLine.h" #include "TAttFill.h" @@ -73,7 +74,7 @@ object onto a one-dimensional plot. bool RooPlot::_addDirStatus = true ; -bool RooPlot::addDirectoryStatus() { return _addDirStatus; } +bool RooPlot::addDirectoryStatus() { return ROOT::Experimental::IsImplicitObjectOwnershipEnabled() && _addDirStatus; } bool RooPlot::setAddDirectoryStatus(bool flag) { bool ret = flag ; _addDirStatus = flag ; return ret ; } From 9764fc6b7009184f14a2ec3ac4aecd6e8b52e443 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 4 Nov 2025 11:07:15 +0100 Subject: [PATCH 36/39] [REVERT] Test switching off the implicit ownership in the CI. --- .github/workflows/root-ci.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/root-ci.yml b/.github/workflows/root-ci.yml index b50bc971b85eb..677e7280ec200 100644 --- a/.github/workflows/root-ci.yml +++ b/.github/workflows/root-ci.yml @@ -167,7 +167,8 @@ jobs: HOME: /Users/sftnight INCREMENTAL: ${{ !contains(github.event.pull_request.labels.*.name, 'clean build') && !matrix.platform == 'mac15' && !matrix.platform == 'mac26'}} GITHUB_PR_ORIGIN: ${{ github.event.pull_request.head.repo.clone_url }} - run: ".github/workflows/root-ci-config/build_root.py + ROOT_IMPLICIT_OWNERSHIP: silentOff + run: ".github/workflows/root-ci-config/build_root.py --buildtype RelWithDebInfo --incremental $INCREMENTAL --base_ref ${{ github.base_ref }} @@ -526,6 +527,7 @@ jobs: INCREMENTAL: ${{ !contains(github.event.pull_request.labels.*.name, 'clean build') }} GITHUB_PR_ORIGIN: ${{ github.event.pull_request.head.repo.clone_url }} CMAKE_GENERATOR: ${{ matrix.cmake_generator }} + ROOT_IMPLICIT_OWNERSHIP: silentOff run: ".github/workflows/root-ci-config/build_root.py --buildtype RelWithDebInfo --platform ${{ matrix.image }} From 60f880893c5c77a5df0d65d38aeb3227a44cf896 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 4 Nov 2025 14:07:16 +0100 Subject: [PATCH 37/39] Make unfold tutorials ready for implicit object ownership=off. --- tutorials/analysis/unfold/testUnfold5c.C | 13 +++++-------- tutorials/analysis/unfold/testUnfold7b.C | 15 +++++++++------ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/tutorials/analysis/unfold/testUnfold5c.C b/tutorials/analysis/unfold/testUnfold5c.C index 13b657b624997..59ba1f21f1c4e 100644 --- a/tutorials/analysis/unfold/testUnfold5c.C +++ b/tutorials/analysis/unfold/testUnfold5c.C @@ -104,8 +104,6 @@ void testUnfold5c() TUnfoldBinning *detectorBinning,*generatorBinning; - outputFile->cd(); - // read binning schemes in XML format #ifndef READ_BINNING_CINT TDOMParser parser; @@ -122,8 +120,8 @@ void testUnfold5c() delete binningSchemes; #endif - detectorBinning->Write(); - generatorBinning->Write(); + outputFile->WriteTObject(detectorBinning); + outputFile->WriteTObject(generatorBinning); if(detectorBinning) { detectorBinning->PrintStream(cout); @@ -154,10 +152,10 @@ void testUnfold5c() Float_t etaRec,ptRec,discr,etaGen,ptGen; Int_t istriggered,issignal; - outputFile->cd(); - TH1 *histDataReco=detectorBinning->CreateHistogram("histDataReco"); TH1 *histDataTruth=generatorBinning->CreateHistogram("histDataTruth"); + histDataReco->SetDirectory(outputFile); + histDataTruth->SetDirectory(outputFile); TFile *dataFile=new TFile("testUnfold5_data.root"); TTree *dataTree=(TTree *) dataFile->Get("data"); @@ -208,10 +206,9 @@ void testUnfold5c() // Step 4: book and fill histogram of migrations // it receives events from both signal MC and background MC - outputFile->cd(); - TH2 *histMCGenRec=TUnfoldBinning::CreateHistogramOfMigrations (generatorBinning,detectorBinning,"histMCGenRec"); + histMCGenRec->SetDirectory(outputFile); TFile *signalFile=new TFile("testUnfold5_signal.root"); TTree *signalTree=(TTree *) signalFile->Get("signal"); diff --git a/tutorials/analysis/unfold/testUnfold7b.C b/tutorials/analysis/unfold/testUnfold7b.C index 6549e0acbd81d..890831db6fcc2 100644 --- a/tutorials/analysis/unfold/testUnfold7b.C +++ b/tutorials/analysis/unfold/testUnfold7b.C @@ -110,8 +110,6 @@ void testUnfold7b() TUnfoldBinning *fineBinningRoot,*coarseBinningRoot; - outputFile->cd(); - // read binning schemes in XML format TDOMParser parser; @@ -164,6 +162,10 @@ void testUnfold7b() TH1 *histDataBgrC=coarseBinning->CreateHistogram("histDataBgrC"); TH1 *histDataGen=coarseBinning->CreateHistogram("histDataGen"); + for (auto histo : {histDataRecF, histDataRecC, histDataBgrF, histDataBgrC, histDataGen}) { + histo->SetDirectory(outputFile); + } + TFile *dataFile=new TFile("testUnfold7_data.root"); TTree *dataTree=(TTree *) dataFile->Get("data"); @@ -213,8 +215,6 @@ void testUnfold7b() // Step 4: book and fill histogram of migrations // it receives events from both signal MC and background MC - outputFile->cd(); - TH2 *histMcsigGenRecF=TUnfoldBinning::CreateHistogramOfMigrations (coarseBinning,fineBinning,"histMcsigGenRecF"); TH2 *histMcsigGenRecC=TUnfoldBinning::CreateHistogramOfMigrations @@ -222,6 +222,9 @@ void testUnfold7b() TH1 *histMcsigRecF=fineBinning->CreateHistogram("histMcsigRecF"); TH1 *histMcsigRecC=coarseBinning->CreateHistogram("histMcsigRecC"); TH1 *histMcsigGen=coarseBinning->CreateHistogram("histMcsigGen"); + for (auto histo : std::initializer_list{histMcsigGenRecF, histMcsigGenRecC, histMcsigRecF, histMcsigRecC, histMcsigGen}) { + histo->SetDirectory(outputFile); + } TFile *signalFile=new TFile("testUnfold7_signal.root"); TTree *signalTree=(TTree *) signalFile->Get("signal"); @@ -259,10 +262,10 @@ void testUnfold7b() delete signalTree; delete signalFile; - outputFile->cd(); - TH1 *histMcbgrRecF=fineBinning->CreateHistogram("histMcbgrRecF"); TH1 *histMcbgrRecC=coarseBinning->CreateHistogram("histMcbgrRecC"); + histMcbgrRecF->SetDirectory(outputFile); + histMcbgrRecC->SetDirectory(outputFile); TFile *bgrFile=new TFile("testUnfold7_background.root"); TTree *bgrTree=(TTree *) bgrFile->Get("background"); From 2e866e7e138ae2300fc9cc6b143e520ef08646e5 Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Tue, 4 Nov 2025 14:25:20 +0100 Subject: [PATCH 38/39] Add a missing header to make testUnfold7c.C compilable. --- tutorials/analysis/unfold/testUnfold7c.C | 1 + 1 file changed, 1 insertion(+) diff --git a/tutorials/analysis/unfold/testUnfold7c.C b/tutorials/analysis/unfold/testUnfold7c.C index 44ecb7ced678d..bb040d258f259 100644 --- a/tutorials/analysis/unfold/testUnfold7c.C +++ b/tutorials/analysis/unfold/testUnfold7c.C @@ -85,6 +85,7 @@ #include #include #include +#include #include #include #include From 0ca4f8a20f8d4775b9c4b62f7dcfae800034292a Mon Sep 17 00:00:00 2001 From: Stephan Hageboeck Date: Wed, 5 Nov 2025 10:18:47 +0100 Subject: [PATCH 39/39] Prepare mlp for implicit memory management off. --- math/mlp/src/TMLPAnalyzer.cxx | 1 + math/mlp/src/TNeuron.cxx | 1 + 2 files changed, 2 insertions(+) diff --git a/math/mlp/src/TMLPAnalyzer.cxx b/math/mlp/src/TMLPAnalyzer.cxx index 2e787e77f17fd..39271c1e00c7f 100644 --- a/math/mlp/src/TMLPAnalyzer.cxx +++ b/math/mlp/src/TMLPAnalyzer.cxx @@ -197,6 +197,7 @@ void TMLPAnalyzer::GatherInformations() index[i] = val.Atoi(); } TH1D tmp("tmpb", "tmpb", 1, -FLT_MAX, FLT_MAX); + tmp.SetDirectory(gDirectory); data->Draw(Form("%s>>tmpb",formula.Data()),"","goff"); rms[i] = tmp.GetRMS(); } diff --git a/math/mlp/src/TNeuron.cxx b/math/mlp/src/TNeuron.cxx index 3b84844070e5c..17741faa3478a 100644 --- a/math/mlp/src/TNeuron.cxx +++ b/math/mlp/src/TNeuron.cxx @@ -893,6 +893,7 @@ TTreeFormula* TNeuron::UseBranch(TTree* input, const char* formula) } // Computes the default normalization TH1D tmp("tmpb", "tmpb", 1, -FLT_MAX, FLT_MAX); + tmp.SetDirectory(gDirectory); input->Draw(Form("%s>>tmpb",(const char*)f),"","goff"); fNorm[0] = tmp.GetRMS(); if(fNorm[0]<1e-15) fNorm[0]=1.;