Skip to content

[RF] xroofit improvements #12103

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
guitargeek merged 5 commits into from
Jan 26, 2023
Merged

[RF] xroofit improvements #12103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6197ae8
xroofit improvements
will-cern Jan 23, 2023
93404ae
revert headers
will-cern Jan 23, 2023
9db7db1
remove the banner for xroofit
will-cern Jan 23, 2023
9da23bb
remove shadowing variable
will-cern Jan 24, 2023
54895c8
respond to PR comments
will-cern Jan 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion roofit/xroofit/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ ROOT_STANDARD_LIBRARY_PACKAGE(RooFitXRooFit
RooFit/xRooFit/xRooNode.h
RooFit/xRooFit/xRooBrowser.h
SOURCES
src/Banner.cxx
src/Asymptotics.cxx
src/xRooBrowser.cxx
src/xRooFit.cxx
Expand Down
4 changes: 4 additions & 0 deletions roofit/xroofit/inc/RooFit/xRooFit/xRooNode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,10 @@ class xRooNode : public TNamed, public std::vector<std::shared_ptr<xRooNode>> {
std::vector<double> GetBinErrors(int binStart = 1, int binEnd = 0, const xRooNode &fr = "") const;
std::pair<double, double> IntegralAndError(const xRooNode &fr = "", const char *rangeName = nullptr) const;

// methods to access default content and error
double GetContent() const { return GetBinContent(0); }
double GetError() const { return GetBinError(0); }

xRooNLLVar nll(const xRooNode &_data, std::initializer_list<RooCmdArg> nllOpts) const;
xRooNLLVar nll(const xRooNode &_data, const RooLinkedList &nllOpts) const;
xRooNLLVar nll(const xRooNode &_data = "") const; // uses xRooFit::createNLLOption for nllOpts
Expand Down
29 changes: 0 additions & 29 deletions roofit/xroofit/src/Banner.cxx
View file
Open in desktop

This file was deleted.

8 changes: 4 additions & 4 deletions roofit/xroofit/src/xRooBrowser.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@
#include "TEnv.h"
#include "TFile.h"
#include "RooWorkspace.h"
#define protected public
#include "TRootBrowser.h"
#undef protected
#include "TGMenu.h"
#include "TGFileDialog.h"
#include "TObjString.h"

#define GETPOPUPMENU(b,m) ((TGPopupMenu*)(*(void**)(((unsigned char*)b) + b->Class()->GetDataMemberOffset(#m))))

BEGIN_XROOFIT_NAMESPACE

xRooBrowser::xRooBrowser(xRooNode *o) : TBrowser("RooBrowser", o, "RooFit Browser"), fTopNode(o)
Expand Down Expand Up @@ -61,8 +61,8 @@ xRooBrowser::xRooBrowser(xRooNode *o) : TBrowser("RooBrowser", o, "RooFit Browse

// override file menu event handling so that can intercept "Open"
if (auto rb = dynamic_cast<TRootBrowser *>(GetBrowserImp())) {
rb->Disconnect(rb->fMenuFile, "Activated(Int_t)", rb, "HandleMenu(Int_t)");
rb->fMenuFile->Connect("Activated(Int_t)", ClassName(), this, "HandleMenu(Int_t)");
rb->Disconnect(GETPOPUPMENU(rb,fMenuFile), "Activated(Int_t)", rb, "HandleMenu(Int_t)");
GETPOPUPMENU(rb,fMenuFile)->Connect("Activated(Int_t)", ClassName(), this, "HandleMenu(Int_t)");
}
}

Expand Down
57 changes: 44 additions & 13 deletions roofit/xroofit/src/xRooNLLVar.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,16 @@
* listed in LICENSE (http://roofit.sourceforge.net/license.txt)
*/

#include "RVersion.h"

#if ROOT_VERSION_CODE < ROOT_VERSION(6, 27, 00)
#define protected public
#endif
#include "RooFitResult.h"
#include "RooNLLVar.h"
#ifdef protected
#undef protected
#endif

#include "xRooFit/xRooFit.h"

Expand All @@ -40,9 +46,26 @@

#include "Math/GenAlgoOptions.h"

#if ROOT_VERSION_CODE < ROOT_VERSION(6, 27, 00)
#define private public
#define GETWS(a) a->_myws
#define GETWSSETS(w) w->_namedSets
#else
#define GETWS(a) a->workspace()
#define GETWSSETS(w) w->sets()
#endif
#include "RooWorkspace.h"
#ifdef private
#undef private
#endif

#if ROOT_VERSION_CODE < ROOT_VERSION(6, 27, 00)
#define protected public
#endif
#include "RooStats/HypoTestResult.h"
#ifdef protected
#undef protected
#endif

#include "TMultiGraph.h"
#include "TCanvas.h"
Expand All @@ -53,9 +76,6 @@
#include "TH1D.h"
#include "TLegend.h"

#define protected public
#include "RooStats/HypoTestResult.h"
#undef protected

BEGIN_XROOFIT_NAMESPACE

Expand Down Expand Up @@ -289,15 +309,16 @@ void xRooNLLVar::reinitialize()
}
}
// before creating, clear away caches if any if pdf is in ws
if (fPdf->_myws) {
if (GETWS(fPdf)) {
std::set<std::string> setNames;
for (auto &a : static_cast<RooWorkspace *>(fPdf->_myws)->_namedSets) {
for (auto &a : GETWSSETS(GETWS(fPdf))) {
if (TString(a.first.c_str()).BeginsWith("CACHE_")) {
setNames.insert(a.first);
}
}
for (auto &a : setNames)
fPdf->_myws->removeSet(a.c_str());
for (auto &a : setNames) {
GETWS(fPdf)->removeSet(a.c_str());
}
}
std::set<std::string> attribs;
if (std::shared_ptr<RooAbsReal>::get())
Expand All @@ -310,8 +331,8 @@ void xRooNLLVar::reinitialize()
for (auto &a : attribs)
std::shared_ptr<RooAbsReal>::get()->setAttribute(a.c_str());
// create parent on next line to avoid triggering workspace initialization code in constructor of xRooNode
if (fPdf->_myws) {
xRooNode(*fPdf->_myws, std::make_shared<xRooNode>()).sterilize();
if (GETWS(fPdf)) {
xRooNode(*GETWS(fPdf), std::make_shared<xRooNode>()).sterilize();
} // there seems to be a nasty bug somewhere that can make the cache become invalid, so clear it here
if (oldName != "")
std::shared_ptr<RooAbsReal>::get()->SetName(oldName);
Expand Down Expand Up @@ -341,9 +362,9 @@ xRooNLLVar::generate(bool expected, int seed)
RooArgList l;
l.add((fFuncVars) ? *fFuncVars : *std::unique_ptr<RooAbsCollection>(fPdf->getParameters(*fData)));
fr->setConstParList(l);
fr->_constPars->setAttribAll("global", false);
const_cast<RooArgList&>(fr->constPars()).setAttribAll("global", false);
if (fGlobs)
std::unique_ptr<RooAbsCollection>(fr->_constPars->selectCommon(*fGlobs))->setAttribAll("global", true);
std::unique_ptr<RooAbsCollection>(fr->constPars().selectCommon(*fGlobs))->setAttribAll("global", true);
return xRooFit::generateFrom(*fPdf, fr, expected, seed);
}

Expand Down Expand Up @@ -395,9 +416,9 @@ xRooNLLVar::xRooFitResult xRooNLLVar::minimize(const std::shared_ptr<ROOT::Fit::

// before returning, flag which of the constPars were actually global observables
if (out) {
out->_constPars->setAttribAll("global", false);
const_cast<RooArgList&>(out->constPars()).setAttribAll("global", false);
if (fGlobs)
std::unique_ptr<RooAbsCollection>(out->_constPars->selectCommon(*fGlobs))->setAttribAll("global", true);
std::unique_ptr<RooAbsCollection>(out->constPars().selectCommon(*fGlobs))->setAttribAll("global", true);
}
return xRooFitResult(std::make_shared<xRooNode>(out, fPdf));
}
Expand Down Expand Up @@ -1816,8 +1837,13 @@ RooStats::HypoTestResult xRooNLLVar::xRooHypoPoint::result()
out.SetNullDetailedOutput(new RooDataSet("nullDetails", "nullDetails", nullDetails));
out.GetNullDetailedOutput()->add(nullDetails);
} else {
#if ROOT_VERSION_CODE < ROOT_VERSION(6, 27, 00)
out.fNullPValue = pNull_asymp().first;
out.fNullPValueError = pNull_asymp().second;
#else
out.SetNullPValue(pNull_asymp().first);
out.SetNullPValueError(pNull_asymp().second);
#endif
}

RooArgList altDetails;
Expand All @@ -1841,8 +1867,13 @@ RooStats::HypoTestResult xRooNLLVar::xRooHypoPoint::result()
out.SetAltDetailedOutput(new RooDataSet("altDetails", "altDetails", altDetails));
out.GetAltDetailedOutput()->add(altDetails);
} else {
#if ROOT_VERSION_CODE < ROOT_VERSION(6, 27, 00)
out.fAlternatePValue = pAlt_asymp().first;
out.fAlternatePValueError = pAlt_asymp().second;
#else
out.SetAltPValue(pAlt_asymp().first);
out.SetAltPValueError(pAlt_asymp().second);
#endif
}

if (setReadonly) {
Expand Down
Loading