Skip to content

Commit 69b3549

Browse files
guitargeekguitargeek
committed
Improve logic for freezing and unfreezing disassiotiated parameters
This functionality is intended to be upstreamed to RooFit, and therefore refactored in one portable function that can be moved to the RooFit helpers. The major change is that the function that freezes the parameters now returns a set with all parameters where the constant change was toggled. This allows us to easily un-freeze these parameters later, without having to carry around in the global state the full list of original floating parameters.
1 parent 2f81f45 commit 69b3549

File tree

3 files changed

+74
-76
lines changed

3 files changed

+74
-76
lines changed

‎interface/CascadeMinimizer.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,6 @@ class CascadeMinimizerGlobalConfigs{
125125
RooArgList nuisanceParameters;
126126
RooArgList allFloatingParameters;
127127
RooArgList parametersOfInterest;
128-
RooArgList allRooMultiPdfParams;
129128
RooArgList allRooMultiPdfs;
130129

131130
static CascadeMinimizerGlobalConfigs& O(){

‎src/CascadeMinimizer.cc‎

Lines changed: 74 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -63,76 +63,72 @@ void CascadeMinimizer::remakeMinimizer() {
6363

6464
namespace {
6565

66-
bool setAllConstant(const RooAbsCollection &coll, bool constant) {
67-
bool changed = false;
68-
for (RooAbsArg *a : coll) {
69-
RooRealVar *v = dynamic_cast<RooRealVar *>(a);
70-
RooCategory *cv = dynamic_cast<RooCategory *>(a);
71-
if (v && (v->isConstant() != constant)) {
72-
changed = true;
73-
v->setConstant(constant);
74-
} else if (cv && (cv->isConstant() != constant)) {
75-
changed = true;
76-
cv->setConstant(constant);
77-
}
66+
RooArgSet freezeDisassiotiatedParams(RooArgSet const &multiPdfs, bool isVerbose = false) {
67+
RooArgSet changedSet;
68+
69+
if (isVerbose) {
70+
multiPdfs.Print();
7871
}
79-
return changed;
80-
}
8172

82-
bool freezeAllDisassociatedRooMultiPdfParameters(const RooArgSet &multiPdfs,
83-
const RooArgSet &allRooMultiPdfParams,
84-
bool freeze) {
85-
static bool freezeDisassParams_verb = runtimedef::get(std::string("MINIMIZER_freezeDisassociatedParams_verbose"));
73+
RooArgSet multiPdfParams;
8674

87-
RooArgSet multiPdfParams(allRooMultiPdfParams);
75+
for (RooAbsArg *pdf : multiPdfs) {
76+
std::unique_ptr<RooArgSet> pdfPars{pdf->getParameters(static_cast<const RooArgSet *>(nullptr))};
77+
for (RooAbsArg *a : *pdfPars) {
78+
if(RooRealVar *v = dynamic_cast<RooRealVar *>(a)) {
79+
if (!v->isConstant())
80+
multiPdfParams.add(*v);
81+
}
82+
}
83+
}
84+
85+
if (isVerbose) {
86+
multiPdfParams.Print();
87+
}
8888

8989
// For each multiPdf, get the active pdf and remove its parameters
9090
// from this list of params and then freeze the remaining ones
9191

92-
for (RooAbsArg *P : multiPdfs) {
92+
for (RooAbsArg *P : CascadeMinimizerGlobalConfigs::O().allRooMultiPdfs) {
9393
RooMultiPdf *mpdf = dynamic_cast<RooMultiPdf *>(P);
9494
RooAbsPdf *pdf = (RooAbsPdf *)mpdf->getCurrentPdf();
95-
if (freezeDisassParams_verb)
95+
if (isVerbose)
9696
std::cout << " Current active PDF - " << pdf->GetName() << std::endl;
9797
std::unique_ptr<RooArgSet> pdfPars(pdf->getParameters((const RooArgSet *)0));
9898
RooStats::RemoveConstantParameters(&*pdfPars); // make sure still to ignore user set constants
9999
multiPdfParams.remove(*pdfPars);
100100
}
101101

102-
if (multiPdfParams.getSize() > 0) {
103-
if (freezeDisassParams_verb) {
104-
std::cout << " Going to " << (freeze ? " freeze " : " float ") << " the following (disassociated) parameters"
105-
<< std::endl;
106-
multiPdfParams.Print("V");
107-
}
108-
setAllConstant(multiPdfParams, freeze);
109-
110-
//std::cout << " Current state of all MultiPdfParams -> " << std::endl;
111-
//std::unique_ptr<TIterator> iter_par(allRooMultiPdfParams.createIterator());
112-
//for (RooAbsArg *P = (RooAbsArg *) iter_par->Next(); P != 0; P = (RooAbsArg *) iter_par->Next()){
113-
// std::cout << P->GetName() << ", constant=" << P->isConstant() << std::endl;
114-
//}
115-
return true;
102+
if (!multiPdfParams.empty() && isVerbose) {
103+
std::cout << " Going to freeze the following (disassociated) parameters" << std::endl;
104+
multiPdfParams.Print("V");
116105
}
117106

118-
return false;
107+
for (RooAbsArg *a : multiPdfParams) {
108+
RooRealVar *v = dynamic_cast<RooRealVar *>(a);
109+
RooCategory *cv = dynamic_cast<RooCategory *>(a);
110+
if (v && !v->isConstant()) {
111+
v->setConstant();
112+
changedSet.add(*v);
113+
} else if (cv && !cv->isConstant()) {
114+
cv->setConstant();
115+
changedSet.add(*v);
116+
}
117+
}
118+
return changedSet;
119119
}
120120

121-
bool freezeDiscParams(const bool freeze) {
121+
RooArgSet freezeDiscParams() {
122122
static bool freezeDisassParams = runtimedef::get(std::string("MINIMIZER_freezeDisassociatedParams"));
123123
static bool freezeDisassParams_verb = runtimedef::get(std::string("MINIMIZER_freezeDisassociatedParams_verbose"));
124-
if (freezeDisassParams) {
125-
if (freezeDisassParams_verb) {
126-
CascadeMinimizerGlobalConfigs::O().allRooMultiPdfs.Print();
127-
CascadeMinimizerGlobalConfigs::O().allRooMultiPdfParams.Print();
128-
}
129-
bool ret = freezeAllDisassociatedRooMultiPdfParameters((CascadeMinimizerGlobalConfigs::O().allRooMultiPdfs),
130-
(CascadeMinimizerGlobalConfigs::O().allRooMultiPdfParams),
131-
freeze);
132-
return ret;
133-
} else {
134-
return false;
124+
125+
if (!freezeDisassParams) {
126+
return {};
135127
}
128+
129+
RooArgSet const &multiPdfs = CascadeMinimizerGlobalConfigs::O().allRooMultiPdfs;
130+
131+
return freezeDisassiotiatedParams(multiPdfs, freezeDisassParams_verb);
136132
}
137133

138134
} // namespace
@@ -231,7 +227,7 @@ bool CascadeMinimizer::improveOnce(int verbose, bool noHesse)
231227
if (!minimizer_.get()) remakeMinimizer();
232228

233229
// freeze non active parameters if MINIMIZER_freezeDisassociatedParams enabled
234-
freezeDiscParams(true);
230+
RooArgSet frozenSet = freezeDiscParams();
235231

236232
if (maxcalls) {
237233
minimizer_->setMaxFunctionCalls(maxcalls);
@@ -278,7 +274,7 @@ bool CascadeMinimizer::improveOnce(int verbose, bool noHesse)
278274
}
279275

280276
// restore original params
281-
freezeDiscParams(false);
277+
utils::setAllConstant(frozenSet,false);
282278

283279
return outcome;
284280
}
@@ -313,12 +309,12 @@ bool CascadeMinimizer::minos(const RooArgSet & params , int verbose ) {
313309

314310
//TStopwatch tw;
315311
// freeze parameters not active under current indexes if MINIMIZER_freezeDisassociatedParams enabled
316-
freezeDiscParams(true);
312+
RooArgSet frozenSet = freezeDiscParams();
317313
// need to re-run Migrad before running minos
318314
minimizer_->minimize(myType.c_str(), "Migrad");
319315
int iret = minimizer_->minos(params);
320316
if (verbose>0 ) CombineLogger::instance().log("CascadeMinimizer.cc",__LINE__,std::string(Form("Minos finished with status=%d",iret)),__func__);
321-
freezeDiscParams(false);
317+
utils::setAllConstant(frozenSet,false);
322318

323319
//std::cout << "Run Minos in "; tw.Print(); std::cout << std::endl;
324320

@@ -357,9 +353,9 @@ bool CascadeMinimizer::hesse(int verbose ) {
357353
}
358354
}
359355

360-
freezeDiscParams(true);
356+
RooArgSet frozenSet = freezeDiscParams();
361357
int iret = minimizer_->hesse();
362-
freezeDiscParams(false);
358+
utils::setAllConstant(frozenSet,false);
363359

364360
if (setZeroPoint_) {
365361
cacheutils::CachingSimNLL *simnll = dynamic_cast<cacheutils::CachingSimNLL *>(&nll_);
@@ -385,18 +381,30 @@ bool CascadeMinimizer::iterativeMinimize(double &minimumNLL,int verbose, bool ca
385381
//std::cout << " Had to improve further since tolerance is not yet reached " << nll_.getVal() << std::endl;
386382
}
387383

384+
// Figure out all floating RooMultiPdf parameters before freezing any
385+
RooArgSet allRooMultiPdfParams;
386+
for (RooAbsArg *pdf : CascadeMinimizerGlobalConfigs::O().allRooMultiPdfs) {
387+
std::unique_ptr<RooArgSet> pdfPars{pdf->getParameters(static_cast<const RooArgSet *>(nullptr))};
388+
for (RooAbsArg *a : *pdfPars) {
389+
if(RooRealVar *v = dynamic_cast<RooRealVar *>(a)) {
390+
if (!v->isConstant())
391+
allRooMultiPdfParams.add(*v);
392+
}
393+
}
394+
}
395+
388396
// First freeze all parameters that have nothing to do with the current active pdfs
389-
freezeDiscParams(true);
397+
RooArgSet frozenSet = freezeDiscParams();
390398

391399
// Next remove the POIs and constrained nuisances - this is to set up for the fast loop over the Index combinations
392400
RooArgSet nuisances = CascadeMinimizerGlobalConfigs::O().allFloatingParameters;
393-
nuisances.remove(CascadeMinimizerGlobalConfigs::O().allRooMultiPdfParams);
401+
nuisances.remove(allRooMultiPdfParams);
394402

395403
RooArgSet poi = CascadeMinimizerGlobalConfigs::O().parametersOfInterest;
396404
RooArgSet frozen;
397405

398-
if (nuisances.getSize() >0) frozen.add(nuisances);
399-
if (poi.getSize() >0) frozen.add(poi);
406+
if (!nuisances.empty()) frozen.add(nuisances);
407+
if (!poi.empty()) frozen.add(poi);
400408

401409
RooStats::RemoveConstantParameters(&frozen);
402410
utils::setAllConstant(frozen,true);
@@ -426,7 +434,7 @@ bool CascadeMinimizer::iterativeMinimize(double &minimumNLL,int verbose, bool ca
426434
tw.Stop(); if (verbose > 2) std::cout << "Full fit done in " << tw.RealTime() << std::endl;
427435

428436
// unfreeze from *
429-
freezeDiscParams(false);
437+
utils::setAllConstant(frozenSet,false);
430438

431439
return ret;
432440
}
@@ -441,7 +449,8 @@ bool CascadeMinimizer::minimize(int verbose, bool cascade)
441449
RooMsgService::instance().setGlobalKillBelow(RooFit::FATAL);
442450
}
443451

444-
freezeDiscParams(true); // We should do anyway this since there can also be some indeces which are frozen
452+
// We should do anyway this since there can also be some indices that are frozen
453+
RooArgSet frozenSet = freezeDiscParams();
445454

446455
bool doMultipleMini = (CascadeMinimizerGlobalConfigs::O().pdfCategories.getSize()>0);
447456
if (runtimedef::get(std::string("MINIMIZER_skipDiscreteIterations"))) doMultipleMini=false;
@@ -456,7 +465,6 @@ bool CascadeMinimizer::minimize(int verbose, bool cascade)
456465
RooArgSet frozen(nuisances);
457466
RooStats::RemoveConstantParameters(&frozen);
458467
utils::setAllConstant(frozen,true);
459-
freezeDiscParams(true);
460468

461469
remakeMinimizer();
462470
minimizer_->setPrintLevel(verbose-2);
@@ -468,7 +476,6 @@ bool CascadeMinimizer::minimize(int verbose, bool cascade)
468476
minimizer_->minimize(ROOT::Math::MinimizerOptions::DefaultMinimizerType().c_str(), ROOT::Math::MinimizerOptions::DefaultMinimizerAlgo().c_str());
469477
if (simnll) simnll->clearZeroPoint();
470478
utils::setAllConstant(frozen,false);
471-
freezeDiscParams(false);
472479
remakeMinimizer();
473480
}
474481

@@ -518,10 +525,10 @@ bool CascadeMinimizer::minimize(int verbose, bool cascade)
518525

519526
// Run one last fully floating fit to maintain RooFitResult in case freezeDisassociatedParams is ON
520527
if(runtimedef::get(std::string("MINIMIZER_freezeDisassociatedParams"))){
521-
freezeDiscParams(true);
528+
RooArgSet frozenSet = freezeDiscParams();
522529
ret = improve(verbose,cascade,true);
523530
minimumNLL = nll_.getVal();
524-
freezeDiscParams(false);
531+
utils::setAllConstant(frozenSet,false);
525532
}
526533
}
527534
}
@@ -539,7 +546,7 @@ bool CascadeMinimizer::minimize(int verbose, bool cascade)
539546
// " [WARNING] Are you sure your model is correct?\n");
540547
CombineLogger::instance().log("CascadeMinimizer.cc",__LINE__,"[WARNING] After fit, some parameters are found at the boundary (within ~1sigma)",__func__);
541548
}
542-
freezeDiscParams(false);
549+
utils::setAllConstant(frozenSet,false);
543550
return ret;
544551
}
545552

@@ -671,7 +678,7 @@ bool CascadeMinimizer::multipleMinimize(const RooArgSet &reallyCleanParameters,
671678
simnll->setMaskNonDiscreteChannels(true);
672679
}
673680
// Remove parameters which are not associated to the current PDF (only works if using --X-rtd MINIMIZER_freezeDisassociatedParams)
674-
freezeDiscParams(true);
681+
RooArgSet frozenSet = freezeDiscParams();
675682

676683
// FIXME can be made smarter than this
677684
if (mode_ == Unconstrained && poiOnlyFit_) {
@@ -684,7 +691,7 @@ bool CascadeMinimizer::multipleMinimize(const RooArgSet &reallyCleanParameters,
684691
for (int id=0;id<numIndeces;id++) ((RooCategory*)(pdfCategoryIndeces.at(id)))->setConstant(false);
685692
simnll->setMaskNonDiscreteChannels(false);
686693
}
687-
freezeDiscParams(false);
694+
utils::setAllConstant(frozenSet,false);
688695

689696

690697
fitCounter++;

‎src/Combine.cc‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,7 +1189,6 @@ void Combine::addDiscreteNuisances(RooWorkspace *w){
11891189
RooArgSet *discreteParameters = (RooArgSet*) w->genobj("discreteParams");
11901190

11911191
CascadeMinimizerGlobalConfigs::O().pdfCategories = RooArgList();
1192-
CascadeMinimizerGlobalConfigs::O().allRooMultiPdfParams = RooArgList();
11931192

11941193
if (discreteParameters != 0) {
11951194
for (RooAbsArg *arg : *discreteParameters) {
@@ -1223,13 +1222,6 @@ void Combine::addDiscreteNuisances(RooWorkspace *w){
12231222
utils::getClients(CascadeMinimizerGlobalConfigs::O().pdfCategories,(w->allPdfs()),clients);
12241223
for (RooAbsArg *arg : clients) {
12251224
(CascadeMinimizerGlobalConfigs::O().allRooMultiPdfs).add(*(dynamic_cast<RooMultiPdf*>(arg)));
1226-
RooAbsPdf *pdf = dynamic_cast<RooAbsPdf*>(arg);
1227-
std::unique_ptr<RooArgSet> pdfPars{pdf->getParameters((const RooArgSet*)0)};
1228-
for (RooAbsArg *a : *pdfPars) {
1229-
RooRealVar *v = dynamic_cast<RooRealVar *>(a);
1230-
if (!v) continue;
1231-
if (! (v->isConstant())) (CascadeMinimizerGlobalConfigs::O().allRooMultiPdfParams).add(*v) ;
1232-
}
12331225
}
12341226
}
12351227

0 commit comments

Comments
 (0)