From 54421edffcaf166be47b4b54c87b24edcb376700 Mon Sep 17 00:00:00 2001 From: "E. G. Patrick Bos" Date: Tue, 29 Jun 2021 14:41:00 +0200 Subject: [PATCH 1/3] add RooAbsMinimizerFcn This class will serve as the new base class for RooMinimizerFcn and other minimizer function implementations to be added later. It handles the "logistics" of synchronizing parameter data between Minuit and RooFit, allowing the derived classes to focus only on function evaluation and optimized performance thereof. --- roofit/roofitcore/CMakeLists.txt | 2 + roofit/roofitcore/inc/RooAbsMinimizerFcn.h | 131 +++++ roofit/roofitcore/src/RooAbsMinimizerFcn.cxx | 526 +++++++++++++++++++ 3 files changed, 659 insertions(+) create mode 100644 roofit/roofitcore/inc/RooAbsMinimizerFcn.h create mode 100644 roofit/roofitcore/src/RooAbsMinimizerFcn.cxx diff --git a/roofit/roofitcore/CMakeLists.txt b/roofit/roofitcore/CMakeLists.txt index bdbd5e1b804b4..83d4cfd9a2c0c 100644 --- a/roofit/roofitcore/CMakeLists.txt +++ b/roofit/roofitcore/CMakeLists.txt @@ -31,6 +31,7 @@ ROOT_STANDARD_LIBRARY_PACKAGE(RooFitCore RooAbsIntegrator.h RooAbsLValue.h RooAbsMCStudyModule.h + RooAbsMinimizerFcn.h RooAbsMoment.h RooAbsNumGenerator.h RooAbsOptTestStatistic.h @@ -253,6 +254,7 @@ ROOT_STANDARD_LIBRARY_PACKAGE(RooFitCore src/RooAbsIntegrator.cxx src/RooAbsLValue.cxx src/RooAbsMCStudyModule.cxx + src/RooAbsMinimizerFcn.cxx src/RooAbsMoment.cxx src/RooAbsNumGenerator.cxx src/RooAbsOptTestStatistic.cxx diff --git a/roofit/roofitcore/inc/RooAbsMinimizerFcn.h b/roofit/roofitcore/inc/RooAbsMinimizerFcn.h new file mode 100644 index 0000000000000..bad09efceff01 --- /dev/null +++ b/roofit/roofitcore/inc/RooAbsMinimizerFcn.h @@ -0,0 +1,131 @@ +/***************************************************************************** + * Project: RooFit * + * Package: RooFitCore * + * @(#)root/roofitcore:$Id$ + * Authors: * + * AL, Alfio Lazzaro, INFN Milan, alfio.lazzaro@mi.infn.it * + * PB, Patrick Bos, Netherlands eScience Center, p.bos@esciencecenter.nl * + * * + * * + * Redistribution and use in source and binary forms, * + * with or without modification, are permitted according to the terms * + * listed in LICENSE (http://roofit.sourceforge.net/license.txt) * + *****************************************************************************/ + +#ifndef __ROOFIT_NOROOMINIMIZER + +#ifndef ROO_ABS_MINIMIZER_FCN +#define ROO_ABS_MINIMIZER_FCN + +#include "Math/IFunction.h" +#include "Fit/ParameterSettings.h" +#include "Fit/FitResult.h" + +#include "TMatrixDSym.h" + +#include "RooAbsReal.h" +#include "RooArgList.h" +#include "RooRealVar.h" + +#include +#include +#include + +// forward declaration +class RooMinimizer; + +class RooAbsMinimizerFcn { + +public: + RooAbsMinimizerFcn(RooArgList paramList, RooMinimizer *context, bool verbose = false); + RooAbsMinimizerFcn(const RooAbsMinimizerFcn &other); + virtual ~RooAbsMinimizerFcn(); + + // inform Minuit through its parameter_settings vector of RooFit parameter properties + Bool_t synchronize_parameter_settings(std::vector ¶meters, Bool_t optConst, Bool_t verbose); + // same, but can be overridden to e.g. also include gradient strategy synchronization in subclasses: + virtual Bool_t Synchronize(std::vector ¶meters, Bool_t optConst, Bool_t verbose); + + // used for export to RooFitResult from Minimizer: + RooArgList *GetFloatParamList(); + RooArgList *GetConstParamList(); + RooArgList *GetInitFloatParamList(); + RooArgList *GetInitConstParamList(); + Int_t GetNumInvalidNLL() const; + + // need access from Minimizer: + void SetEvalErrorWall(Bool_t flag); + /// Try to recover from invalid function values. When invalid function values are encountered, + /// a penalty term is returned to the minimiser to make it back off. This sets the strength of this penalty. + /// \note A strength of zero is equivalent to a constant penalty (= the gradient vanishes, ROOT < 6.24). + /// Positive values lead to a gradient pointing away from the undefined regions. Use ~10 to force the minimiser + /// away from invalid function values. + void SetRecoverFromNaNStrength(double strength) { _recoverFromNaNStrength = strength; } + void SetPrintEvalErrors(Int_t numEvalErrors); + Double_t &GetMaxFCN(); + Int_t evalCounter() const; + void zeroEvalCount(); + /// Return a possible offset that's applied to the function to separate invalid function values from valid ones. + double getOffset() const { return _funcOffset; } + void SetVerbose(Bool_t flag = kTRUE); + + // put Minuit results back into RooFit objects: + void BackProp(const ROOT::Fit::FitResult &results); + + // used in several Minimizer functions: + virtual std::string getFunctionName() const = 0; + virtual std::string getFunctionTitle() const = 0; + + // set different external covariance matrix + void ApplyCovarianceMatrix(TMatrixDSym &V); + + Bool_t SetLogFile(const char *inLogfile); + std::ofstream *GetLogFile() { return _logfile; } + + unsigned int get_nDim() const { return _nDim; } + + virtual void setOptimizeConst(Int_t flag) = 0; + + bool getOptConst(); + std::vector get_parameter_values() const; + + Bool_t SetPdfParamVal(int index, double value) const; + +protected: + virtual void optimizeConstantTerms(bool constStatChange, bool constValChange) = 0; + + // used in BackProp (Minuit results -> RooFit) and ApplyCovarianceMatrix + void SetPdfParamErr(Int_t index, Double_t value); + void ClearPdfParamAsymErr(Int_t index); + void SetPdfParamErr(Int_t index, Double_t loVal, Double_t hiVal); + + void printEvalErrors() const; + + // members + RooMinimizer *_context; + + // the following four are mutable because DoEval is const (in child classes) + // Reset the *largest* negative log-likelihood value we have seen so far: + mutable double _maxFCN = -std::numeric_limits::infinity(); + mutable double _funcOffset{0.}; + double _recoverFromNaNStrength{10.}; + mutable int _numBadNLL = 0; + mutable int _printEvalErrors = 10; + mutable int _evalCounter{0}; + + unsigned int _nDim = 0; + + Bool_t _optConst = kFALSE; + + RooArgList *_floatParamList; + RooArgList *_constParamList; + RooArgList *_initFloatParamList; + RooArgList *_initConstParamList; + + std::ofstream *_logfile = nullptr; + bool _doEvalErrorWall{true}; + bool _verbose; +}; + +#endif +#endif diff --git a/roofit/roofitcore/src/RooAbsMinimizerFcn.cxx b/roofit/roofitcore/src/RooAbsMinimizerFcn.cxx new file mode 100644 index 0000000000000..6a1dc7b293cdb --- /dev/null +++ b/roofit/roofitcore/src/RooAbsMinimizerFcn.cxx @@ -0,0 +1,526 @@ +/***************************************************************************** + * Project: RooFit * + * Package: RooFitCore * + * @(#)root/roofitcore:$Id$ + * Authors: * + * AL, Alfio Lazzaro, INFN Milan, alfio.lazzaro@mi.infn.it * + * PB, Patrick Bos, Netherlands eScience Center, p.bos@esciencecenter.nl * + * * + * * + * Redistribution and use in source and binary forms, * + * with or without modification, are permitted according to the terms * + * listed in LICENSE (http://roofit.sourceforge.net/license.txt) * + *****************************************************************************/ + +#ifndef __ROOFIT_NOROOMINIMIZER + +////////////////////////////////////////////////////////////////////////////// +// +// RooAbsMinimizerFcn is an interface class to the ROOT::Math function +// for minimization. It contains only the "logistics" of synchronizing +// between Minuit and RooFit. Its subclasses implement actual interfacing +// to Minuit by subclassing IMultiGenFunction or IMultiGradFunction. +// + +#include "RooAbsMinimizerFcn.h" + +#include "RooAbsArg.h" +#include "RooAbsPdf.h" +#include "RooArgSet.h" +#include "RooRealVar.h" +#include "RooAbsRealLValue.h" +#include "RooMsgService.h" +#include "RooMinimizer.h" +#include "RooNaNPacker.h" + +#include "TMatrixDSym.h" + +#include +#include + +using namespace std; + +RooAbsMinimizerFcn::RooAbsMinimizerFcn(RooArgList paramList, RooMinimizer *context, bool verbose) + : _context(context), _verbose(verbose) +{ + // Examine parameter list + _floatParamList = (RooArgList *)paramList.selectByAttrib("Constant", kFALSE); + if (_floatParamList->getSize() > 1) { + _floatParamList->sort(); + } + _floatParamList->setName("floatParamList"); + + _constParamList = (RooArgList *)paramList.selectByAttrib("Constant", kTRUE); + if (_constParamList->getSize() > 1) { + _constParamList->sort(); + } + _constParamList->setName("constParamList"); + + // Remove all non-RooRealVar parameters from list (MINUIT cannot handle them) + for (unsigned int i = 0; i < _floatParamList->size(); ) { // Note: Counting loop, since removing from collection! + const RooAbsArg* arg = (*_floatParamList).at(i); + if (!arg->IsA()->InheritsFrom(RooAbsRealLValue::Class())) { + oocoutW(_context,Minimization) << "RooMinimizerFcn::RooMinimizerFcn: removing parameter " + << arg->GetName() << " from list because it is not of type RooRealVar" << endl; + _floatParamList->remove(*arg); + } else { + ++i; + } + } + + _nDim = _floatParamList->getSize(); + + // Save snapshot of initial lists + _initFloatParamList = (RooArgList *)_floatParamList->snapshot(kFALSE); + _initConstParamList = (RooArgList *)_constParamList->snapshot(kFALSE); +} + +RooAbsMinimizerFcn::RooAbsMinimizerFcn(const RooAbsMinimizerFcn &other) + : _context(other._context), _maxFCN(other._maxFCN), + _funcOffset(other._funcOffset), + _recoverFromNaNStrength(other._recoverFromNaNStrength), + _numBadNLL(other._numBadNLL), + _printEvalErrors(other._printEvalErrors), _evalCounter(other._evalCounter), + _nDim(other._nDim), _optConst(other._optConst), _logfile(other._logfile), _doEvalErrorWall(other._doEvalErrorWall), _verbose(other._verbose) +{ + _floatParamList = new RooArgList(*other._floatParamList); + _constParamList = new RooArgList(*other._constParamList); + _initFloatParamList = (RooArgList *)other._initFloatParamList->snapshot(kFALSE); + _initConstParamList = (RooArgList *)other._initConstParamList->snapshot(kFALSE); +} + +RooAbsMinimizerFcn::~RooAbsMinimizerFcn() +{ + delete _floatParamList; + delete _initFloatParamList; + delete _constParamList; + delete _initConstParamList; +} + +/// Internal function to synchronize TMinimizer with current +/// information in RooAbsReal function parameters +Bool_t RooAbsMinimizerFcn::synchronize_parameter_settings(std::vector ¶meters, Bool_t optConst, Bool_t verbose) +{ + Bool_t constValChange(kFALSE); + Bool_t constStatChange(kFALSE); + + Int_t index(0); + + // Handle eventual migrations from constParamList -> floatParamList + for (index = 0; index < _constParamList->getSize(); index++) { + + RooRealVar *par = dynamic_cast(_constParamList->at(index)); + if (!par) + continue; + + RooRealVar *oldpar = dynamic_cast(_initConstParamList->at(index)); + if (!oldpar) + continue; + + // Test if constness changed + if (!par->isConstant()) { + + // Remove from constList, add to floatList + _constParamList->remove(*par); + _floatParamList->add(*par); + _initFloatParamList->addClone(*oldpar); + _initConstParamList->remove(*oldpar); + constStatChange = kTRUE; + _nDim++; + + if (verbose) { + oocoutI(_context, Minimization) + << "RooAbsMinimizerFcn::synchronize: parameter " << par->GetName() << " is now floating." << endl; + } + } + + // Test if value changed + if (par->getVal() != oldpar->getVal()) { + constValChange = kTRUE; + if (verbose) { + oocoutI(_context, Minimization) + << "RooAbsMinimizerFcn::synchronize: value of constant parameter " << par->GetName() << " changed from " + << oldpar->getVal() << " to " << par->getVal() << endl; + } + } + } + + // Update reference list + *_initConstParamList = *_constParamList; + + // Synchronize MINUIT with function state + // Handle floatParamList + for (index = 0; index < _floatParamList->getSize(); index++) { + RooRealVar *par = dynamic_cast(_floatParamList->at(index)); + + if (!par) + continue; + + Double_t pstep(0); + Double_t pmin(0); + Double_t pmax(0); + + if (!par->isConstant()) { + + // Verify that floating parameter is indeed of type RooRealVar + if (!par->IsA()->InheritsFrom(RooRealVar::Class())) { + oocoutW(_context, Minimization) << "RooAbsMinimizerFcn::fit: Error, non-constant parameter " + << par->GetName() << " is not of type RooRealVar, skipping" << endl; + _floatParamList->remove(*par); + index--; + _nDim--; + continue; + } + // make sure the parameter are in dirty state to enable + // a real NLL computation when the minimizer calls the function the first time + // (see issue #7659) + par->setValueDirty(); + + // Set the limits, if not infinite + if (par->hasMin()) + pmin = par->getMin(); + if (par->hasMax()) + pmax = par->getMax(); + + // Calculate step size + pstep = par->getError(); + if (pstep <= 0) { + // Floating parameter without error estitimate + if (par->hasMin() && par->hasMax()) { + pstep = 0.1 * (pmax - pmin); + + // Trim default choice of error if within 2 sigma of limit + if (pmax - par->getVal() < 2 * pstep) { + pstep = (pmax - par->getVal()) / 2; + } else if (par->getVal() - pmin < 2 * pstep) { + pstep = (par->getVal() - pmin) / 2; + } + + // If trimming results in zero error, restore default + if (pstep == 0) { + pstep = 0.1 * (pmax - pmin); + } + + } else { + pstep = 1; + } + if (verbose) { + oocoutW(_context, Minimization) + << "RooAbsMinimizerFcn::synchronize: WARNING: no initial error estimate available for " + << par->GetName() << ": using " << pstep << endl; + } + } + } else { + pmin = par->getVal(); + pmax = par->getVal(); + } + + // new parameter + if (index >= Int_t(parameters.size())) { + + if (par->hasMin() && par->hasMax()) { + parameters.emplace_back(par->GetName(), par->getVal(), pstep, pmin, pmax); + } else { + parameters.emplace_back(par->GetName(), par->getVal(), pstep); + if (par->hasMin()) + parameters.back().SetLowerLimit(pmin); + else if (par->hasMax()) + parameters.back().SetUpperLimit(pmax); + } + + continue; + } + + Bool_t oldFixed = parameters[index].IsFixed(); + Double_t oldVar = parameters[index].Value(); + Double_t oldVerr = parameters[index].StepSize(); + Double_t oldVlo = parameters[index].LowerLimit(); + Double_t oldVhi = parameters[index].UpperLimit(); + + if (par->isConstant() && !oldFixed) { + + // Parameter changes floating -> constant : update only value if necessary + if (oldVar != par->getVal()) { + parameters[index].SetValue(par->getVal()); + if (verbose) { + oocoutI(_context, Minimization) + << "RooAbsMinimizerFcn::synchronize: value of parameter " << par->GetName() << " changed from " + << oldVar << " to " << par->getVal() << endl; + } + } + parameters[index].Fix(); + constStatChange = kTRUE; + if (verbose) { + oocoutI(_context, Minimization) + << "RooAbsMinimizerFcn::synchronize: parameter " << par->GetName() << " is now fixed." << endl; + } + + } else if (par->isConstant() && oldFixed) { + + // Parameter changes constant -> constant : update only value if necessary + if (oldVar != par->getVal()) { + parameters[index].SetValue(par->getVal()); + constValChange = kTRUE; + + if (verbose) { + oocoutI(_context, Minimization) + << "RooAbsMinimizerFcn::synchronize: value of fixed parameter " << par->GetName() << " changed from " + << oldVar << " to " << par->getVal() << endl; + } + } + + } else { + // Parameter changes constant -> floating + if (!par->isConstant() && oldFixed) { + parameters[index].Release(); + constStatChange = kTRUE; + + if (verbose) { + oocoutI(_context, Minimization) + << "RooAbsMinimizerFcn::synchronize: parameter " << par->GetName() << " is now floating." << endl; + } + } + + // Parameter changes constant -> floating : update all if necessary + if (oldVar != par->getVal() || oldVlo != pmin || oldVhi != pmax || oldVerr != pstep) { + parameters[index].SetValue(par->getVal()); + parameters[index].SetStepSize(pstep); + if (par->hasMin() && par->hasMax()) + parameters[index].SetLimits(pmin, pmax); + else if (par->hasMin()) + parameters[index].SetLowerLimit(pmin); + else if (par->hasMax()) + parameters[index].SetUpperLimit(pmax); + } + + // Inform user about changes in verbose mode + if (verbose) { + // if ierr<0, par was moved from the const list and a message was already printed + + if (oldVar != par->getVal()) { + oocoutI(_context, Minimization) + << "RooAbsMinimizerFcn::synchronize: value of parameter " << par->GetName() << " changed from " + << oldVar << " to " << par->getVal() << endl; + } + if (oldVlo != pmin || oldVhi != pmax) { + oocoutI(_context, Minimization) + << "RooAbsMinimizerFcn::synchronize: limits of parameter " << par->GetName() << " changed from [" + << oldVlo << "," << oldVhi << "] to [" << pmin << "," << pmax << "]" << endl; + } + + // If oldVerr=0, then parameter was previously fixed + if (oldVerr != pstep && oldVerr != 0) { + oocoutI(_context, Minimization) + << "RooAbsMinimizerFcn::synchronize: error/step size of parameter " << par->GetName() + << " changed from " << oldVerr << " to " << pstep << endl; + } + } + } + } + + if (optConst) { + optimizeConstantTerms(constStatChange, constValChange); + } + + return 0; +} + +Bool_t +RooAbsMinimizerFcn::Synchronize(std::vector ¶meters, Bool_t optConst, Bool_t verbose) { + return synchronize_parameter_settings(parameters, optConst, verbose); +} + +/// Modify PDF parameter error by ordinal index (needed by MINUIT) +void RooAbsMinimizerFcn::SetPdfParamErr(Int_t index, Double_t value) +{ + static_cast(_floatParamList->at(index))->setError(value); +} + +/// Modify PDF parameter error by ordinal index (needed by MINUIT) +void RooAbsMinimizerFcn::ClearPdfParamAsymErr(Int_t index) +{ + static_cast(_floatParamList->at(index))->removeAsymError(); +} + +/// Modify PDF parameter error by ordinal index (needed by MINUIT) +void RooAbsMinimizerFcn::SetPdfParamErr(Int_t index, Double_t loVal, Double_t hiVal) +{ + static_cast(_floatParamList->at(index))->setAsymError(loVal, hiVal); +} + +/// Transfer MINUIT fit results back into RooFit objects. +void RooAbsMinimizerFcn::BackProp(const ROOT::Fit::FitResult &results) +{ + for (unsigned int index = 0; index < _nDim; index++) { + Double_t value = results.Value(index); + SetPdfParamVal(index, value); + + // Set the parabolic error + Double_t err = results.Error(index); + SetPdfParamErr(index, err); + + Double_t eminus = results.LowerError(index); + Double_t eplus = results.UpperError(index); + + if (eplus > 0 || eminus < 0) { + // Store the asymmetric error, if it is available + SetPdfParamErr(index, eminus, eplus); + } else { + // Clear the asymmetric error + ClearPdfParamAsymErr(index); + } + } +} + +/// Change the file name for logging of a RooMinimizer of all MINUIT steppings +/// through the parameter space. If inLogfile is null, the current log file +/// is closed and logging is stopped. +Bool_t RooAbsMinimizerFcn::SetLogFile(const char *inLogfile) +{ + if (_logfile) { + oocoutI(_context, Minimization) << "RooAbsMinimizerFcn::setLogFile: closing previous log file" << endl; + _logfile->close(); + delete _logfile; + _logfile = 0; + } + _logfile = new ofstream(inLogfile); + if (!_logfile->good()) { + oocoutI(_context, Minimization) << "RooAbsMinimizerFcn::setLogFile: cannot open file " << inLogfile << endl; + _logfile->close(); + delete _logfile; + _logfile = 0; + } + + return kFALSE; +} + +/// Apply results of given external covariance matrix. i.e. propagate its errors +/// to all RRV parameter representations and give this matrix instead of the +/// HESSE matrix at the next save() call +void RooAbsMinimizerFcn::ApplyCovarianceMatrix(TMatrixDSym &V) +{ + for (unsigned int i = 0; i < _nDim; i++) { + // Skip fixed parameters + if (_floatParamList->at(i)->isConstant()) { + continue; + } + SetPdfParamErr(i, sqrt(V(i, i))); + } +} + +/// Set value of parameter i. +Bool_t RooAbsMinimizerFcn::SetPdfParamVal(int index, double value) const +{ + auto par = static_cast(&(*_floatParamList)[index]); + + if (par->getVal()!=value) { + if (_verbose) cout << par->GetName() << "=" << value << ", " ; + + par->setVal(value); + return kTRUE; + } + + return kFALSE; +} + + +/// Print information about why evaluation failed. +/// Using _printEvalErrors, the number of errors printed can be steered. +/// Negative values disable printing. +void RooAbsMinimizerFcn::printEvalErrors() const { + if (_printEvalErrors < 0) + return; + + std::ostringstream msg; + if (_doEvalErrorWall) { + msg << "RooMinimizerFcn: Minimized function has error status." << endl + << "Returning maximum FCN so far (" << _maxFCN + << ") to force MIGRAD to back out of this region. Error log follows.\n"; + } else { + msg << "RooMinimizerFcn: Minimized function has error status but is ignored.\n"; + } + + msg << "Parameter values: " ; + for (const auto par : *_floatParamList) { + auto var = static_cast(par); + msg << "\t" << var->GetName() << "=" << var->getVal() ; + } + msg << std::endl; + + RooAbsReal::printEvalErrors(msg, _printEvalErrors); + ooccoutW(_context,Minimization) << msg.str() << endl; +} + + +//////////////////////////////////////////////////////////////////////////////// +/// Logistics + +RooArgList *RooAbsMinimizerFcn::GetFloatParamList() +{ + return _floatParamList; +} +RooArgList *RooAbsMinimizerFcn::GetConstParamList() +{ + return _constParamList; +} +RooArgList *RooAbsMinimizerFcn::GetInitFloatParamList() +{ + return _initFloatParamList; +} +RooArgList *RooAbsMinimizerFcn::GetInitConstParamList() +{ + return _initConstParamList; +} + +void RooAbsMinimizerFcn::SetEvalErrorWall(Bool_t flag) +{ + _doEvalErrorWall = flag; +} +void RooAbsMinimizerFcn::SetPrintEvalErrors(Int_t numEvalErrors) +{ + _printEvalErrors = numEvalErrors; +} + +Double_t &RooAbsMinimizerFcn::GetMaxFCN() +{ + return _maxFCN; +} +Int_t RooAbsMinimizerFcn::GetNumInvalidNLL() const +{ + return _numBadNLL; +} + +Int_t RooAbsMinimizerFcn::evalCounter() const +{ + return _evalCounter; +} +void RooAbsMinimizerFcn::zeroEvalCount() +{ + _evalCounter = 0; +} + +void RooAbsMinimizerFcn::SetVerbose(Bool_t flag) +{ + _verbose = flag; +} + +bool RooAbsMinimizerFcn::getOptConst() +{ + return _optConst; +} + +std::vector RooAbsMinimizerFcn::get_parameter_values() const +{ + // TODO: make a cache for this somewhere so it doesn't have to be recreated on each call + std::vector values; + values.reserve(_nDim); + + for (std::size_t index = 0; index < _nDim; ++index) { + RooRealVar *par = (RooRealVar *)_floatParamList->at(index); + values.push_back(par->getVal()); + } + + return values; +} + +#endif From bfd74005cab9b88ff8e8c3ee7b82574a34cdcddd Mon Sep 17 00:00:00 2001 From: "E. G. Patrick Bos" Date: Tue, 29 Jun 2021 14:50:40 +0200 Subject: [PATCH 2/3] make RooMinimizerFcn subclass RooAbsMinimizerFcn This commit changes RooMinimizerFcn to become a derived class of RooAbsMinimizerFcn. This significantly reduces the size of the RooMinimizerFcn implementation, because most of it was logistics of synchronizing between RooFit and Minuit, which is now handled in RooAbsMinimizerFcn. Some further additions, in preparation of upcoming PRs: - The new optimizeConstantTerms function was factored out of Synchronize so it can be overridden by RooAbsMinimizerFcn classes that handle optimization differently. - In a later PR, RooMinimizer will be refactored so that it no longer holds _func itself anymore, and hence cannot get to its name/title or optimization options. To handle this, RooMinimizerFcn now has three added functions for RooMinimizer's use: - getFunctionName/Title - setOptimizeConst --- roofit/roofitcore/inc/RooMinimizer.h | 3 + roofit/roofitcore/inc/RooMinimizerFcn.h | 88 +--- roofit/roofitcore/src/RooMinimizer.cxx | 5 + roofit/roofitcore/src/RooMinimizerFcn.cxx | 482 +++------------------- 4 files changed, 78 insertions(+), 500 deletions(-) diff --git a/roofit/roofitcore/inc/RooMinimizer.h b/roofit/roofitcore/inc/RooMinimizer.h index 71615642cd624..4b9e8d0711d94 100644 --- a/roofit/roofitcore/inc/RooMinimizer.h +++ b/roofit/roofitcore/inc/RooMinimizer.h @@ -77,6 +77,9 @@ class RooMinimizer : public TObject { void setProfile(Bool_t flag=kTRUE) { _profile = flag ; } Bool_t setLogFile(const char* logf=0) { return fitterFcn()->SetLogFile(logf); } + // necessary from RooAbsMinimizerFcn subclasses + Int_t getPrintLevel() const; + void setMinimizerType(const char* type) ; static void cleanup() ; diff --git a/roofit/roofitcore/inc/RooMinimizerFcn.h b/roofit/roofitcore/inc/RooMinimizerFcn.h index 456589b189c3b..ea69209ddd33e 100644 --- a/roofit/roofitcore/inc/RooMinimizerFcn.h +++ b/roofit/roofitcore/inc/RooMinimizerFcn.h @@ -1,9 +1,10 @@ /***************************************************************************** * Project: RooFit * - * Package: RooFitCore * + * Package: RooFitCore * * @(#)root/roofitcore:$Id$ * Authors: * * AL, Alfio Lazzaro, INFN Milan, alfio.lazzaro@mi.infn.it * + * PB, Patrick Bos, Netherlands eScience Center, p.bos@esciencecenter.nl * * * * * * Redistribution and use in source and binary forms, * @@ -24,83 +25,34 @@ #include #include -class RooMinimizer; +#include + template class TMatrixTSym; using TMatrixDSym = TMatrixTSym; -class RooMinimizerFcn : public ROOT::Math::IBaseFunctionMultiDim { - - public: - - RooMinimizerFcn(RooAbsReal *funct, RooMinimizer *context, - bool verbose = false); - RooMinimizerFcn(const RooMinimizerFcn& other); - virtual ~RooMinimizerFcn(); - - virtual ROOT::Math::IBaseFunctionMultiDim* Clone() const; - virtual unsigned int NDim() const { return _nDim; } - - RooArgList* GetFloatParamList() { return _floatParamList; } - RooArgList* GetConstParamList() { return _constParamList; } - RooArgList* GetInitFloatParamList() { return _initFloatParamList; } - RooArgList* GetInitConstParamList() { return _initConstParamList; } - - void SetEvalErrorWall(Bool_t flag) { _doEvalErrorWall = flag ; } - /// Try to recover from invalid function values. When invalid function values are encountered, - /// a penalty term is returned to the minimiser to make it back off. This sets the strength of this penalty. - /// \note A strength of zero is equivalent to a constant penalty (= the gradient vanishes, ROOT < 6.24). - /// Positive values lead to a gradient pointing away from the undefined regions. Use ~10 to force the minimiser - /// away from invalid function values. - void SetRecoverFromNaNStrength(double strength) { _recoverFromNaNStrength = strength; } - void SetPrintEvalErrors(Int_t numEvalErrors) { _printEvalErrors = numEvalErrors ; } - Bool_t SetLogFile(const char* inLogfile); - std::ofstream* GetLogFile() { return _logfile; } - void SetVerbose(Bool_t flag=kTRUE) { _verbose = flag ; } - - Double_t& GetMaxFCN() { return _maxFCN; } - Int_t GetNumInvalidNLL() const { return _numBadNLL; } - - Bool_t Synchronize(std::vector& parameters, - Bool_t optConst, Bool_t verbose); - void BackProp(const ROOT::Fit::FitResult &results); - void ApplyCovarianceMatrix(TMatrixDSym& V); - - Int_t evalCounter() const { return _evalCounter ; } - void zeroEvalCount() { _evalCounter = 0 ; } - /// Return a possible offset that's applied to the function to separate invalid function values from valid ones. - double getOffset() const { return _funcOffset; } - - private: - void SetPdfParamErr(Int_t index, Double_t value); - void ClearPdfParamAsymErr(Int_t index); - void SetPdfParamErr(Int_t index, Double_t loVal, Double_t hiVal); - - Bool_t SetPdfParamVal(int index, double value) const; - void printEvalErrors() const; +// forward declaration +class RooMinimizer; - virtual double DoEval(const double * x) const; +class RooMinimizerFcn : public RooAbsMinimizerFcn, public ROOT::Math::IBaseFunctionMultiDim { +public: + RooMinimizerFcn(RooAbsReal *funct, RooMinimizer *context, bool verbose = false); + RooMinimizerFcn(const RooMinimizerFcn &other); + virtual ~RooMinimizerFcn(); - RooAbsReal *_funct; - const RooMinimizer *_context; + ROOT::Math::IBaseFunctionMultiDim *Clone() const override; + unsigned int NDim() const override { return get_nDim(); } - mutable double _maxFCN; - mutable double _funcOffset{0.}; - double _recoverFromNaNStrength{10.}; - mutable int _numBadNLL; - mutable int _printEvalErrors; - mutable int _evalCounter{0}; - int _nDim; + std::string getFunctionName() const override; + std::string getFunctionTitle() const override; - RooArgList* _floatParamList; - RooArgList* _constParamList; - RooArgList* _initFloatParamList; - RooArgList* _initConstParamList; + void setOptimizeConst(Int_t flag) override; - std::ofstream *_logfile; - bool _doEvalErrorWall{true}; - bool _verbose; +private: + double DoEval(const double *x) const override; + void optimizeConstantTerms(bool constStatChange, bool constValChange) override; + RooAbsReal *_funct; }; #endif diff --git a/roofit/roofitcore/src/RooMinimizer.cxx b/roofit/roofitcore/src/RooMinimizer.cxx index b3c32e8cff1b5..08b9d344aaf12 100644 --- a/roofit/roofitcore/src/RooMinimizer.cxx +++ b/roofit/roofitcore/src/RooMinimizer.cxx @@ -957,3 +957,8 @@ RooFitResult* RooMinimizer::lastMinuitFit(const RooArgList& varList) return res; } + +Int_t RooMinimizer::getPrintLevel() const +{ + return _printLevel; +} diff --git a/roofit/roofitcore/src/RooMinimizerFcn.cxx b/roofit/roofitcore/src/RooMinimizerFcn.cxx index 8bcf3413b00c5..ce587f5c9b2dd 100644 --- a/roofit/roofitcore/src/RooMinimizerFcn.cxx +++ b/roofit/roofitcore/src/RooMinimizerFcn.cxx @@ -4,6 +4,7 @@ * @(#)root/roofitcore:$Id$ * Authors: * * AL, Alfio Lazzaro, INFN Milan, alfio.lazzaro@mi.infn.it * + * PB, Patrick Bos, Netherlands eScience Center, p.bos@esciencecenter.nl * * * * * * Redistribution and use in source and binary forms, * @@ -39,81 +40,18 @@ using namespace std; RooMinimizerFcn::RooMinimizerFcn(RooAbsReal *funct, RooMinimizer* context, bool verbose) : - _funct(funct), _context(context), - // Reset the *largest* negative log-likelihood value we have seen so far - _maxFCN(-std::numeric_limits::infinity()), _numBadNLL(0), - _printEvalErrors(10), - _nDim(0), _logfile(0), - _verbose(verbose) -{ - - // Examine parameter list - RooArgSet* paramSet = _funct->getParameters(RooArgSet()); - RooArgList paramList(*paramSet); - delete paramSet; - - _floatParamList = (RooArgList*) paramList.selectByAttrib("Constant",kFALSE); - if (_floatParamList->getSize()>1) { - _floatParamList->sort(); - } - _floatParamList->setName("floatParamList"); - - _constParamList = (RooArgList*) paramList.selectByAttrib("Constant",kTRUE); - if (_constParamList->getSize()>1) { - _constParamList->sort(); - } - _constParamList->setName("constParamList"); - - // Remove all non-RooRealVar parameters from list (MINUIT cannot handle them) - for (unsigned int i = 0; i < _floatParamList->size(); ) { // Note: Counting loop, since removing from collection! - const RooAbsArg* arg = (*_floatParamList).at(i); - if (!arg->IsA()->InheritsFrom(RooAbsRealLValue::Class())) { - oocoutW(_context,Minimization) << "RooMinimizerFcn::RooMinimizerFcn: removing parameter " - << arg->GetName() << " from list because it is not of type RooRealVar" << endl; - _floatParamList->remove(*arg); - } else { - ++i; - } - } - - _nDim = _floatParamList->getSize(); + RooAbsMinimizerFcn(*funct->getParameters(RooArgSet()), context, verbose), _funct(funct) +{} - // Save snapshot of initial lists - _initFloatParamList = (RooArgList*) _floatParamList->snapshot(kFALSE) ; - _initConstParamList = (RooArgList*) _constParamList->snapshot(kFALSE) ; -} - - -RooMinimizerFcn::RooMinimizerFcn(const RooMinimizerFcn& other) : ROOT::Math::IBaseFunctionMultiDim(other), - _funct(other._funct), - _context(other._context), - _maxFCN(other._maxFCN), - _funcOffset(other._funcOffset), - _recoverFromNaNStrength(other._recoverFromNaNStrength), - _numBadNLL(other._numBadNLL), - _printEvalErrors(other._printEvalErrors), - _evalCounter(other._evalCounter), - _nDim(other._nDim), - _logfile(other._logfile), - _doEvalErrorWall(other._doEvalErrorWall), - _verbose(other._verbose) -{ - _floatParamList = new RooArgList(*other._floatParamList) ; - _constParamList = new RooArgList(*other._constParamList) ; - _initFloatParamList = (RooArgList*) other._initFloatParamList->snapshot(kFALSE) ; - _initConstParamList = (RooArgList*) other._initConstParamList->snapshot(kFALSE) ; -} +RooMinimizerFcn::RooMinimizerFcn(const RooMinimizerFcn& other) : RooAbsMinimizerFcn(other), ROOT::Math::IBaseFunctionMultiDim(other), + _funct(other._funct) +{} RooMinimizerFcn::~RooMinimizerFcn() -{ - delete _floatParamList; - delete _initFloatParamList; - delete _constParamList; - delete _initConstParamList; -} +{} ROOT::Math::IBaseFunctionMultiDim* RooMinimizerFcn::Clone() const @@ -121,374 +59,44 @@ ROOT::Math::IBaseFunctionMultiDim* RooMinimizerFcn::Clone() const return new RooMinimizerFcn(*this) ; } - -/// Internal function to synchronize TMinimizer with current -/// information in RooAbsReal function parameters -Bool_t RooMinimizerFcn::Synchronize(std::vector& parameters, - Bool_t optConst, Bool_t verbose) +void RooMinimizerFcn::setOptimizeConst(Int_t flag) { - Bool_t constValChange(kFALSE) ; - Bool_t constStatChange(kFALSE) ; - - Int_t index(0) ; - - // Handle eventual migrations from constParamList -> floatParamList - for(index= 0; index < _constParamList->getSize() ; index++) { - - RooRealVar *par= dynamic_cast(_constParamList->at(index)) ; - if (!par) continue ; - - RooRealVar *oldpar= dynamic_cast(_initConstParamList->at(index)) ; - if (!oldpar) continue ; - - // Test if constness changed - if (!par->isConstant()) { - - // Remove from constList, add to floatList - _constParamList->remove(*par) ; - _floatParamList->add(*par) ; - _initFloatParamList->addClone(*oldpar) ; - _initConstParamList->remove(*oldpar) ; - constStatChange=kTRUE ; - _nDim++ ; - - if (verbose) { - oocoutI(_context,Minimization) << "RooMinimizerFcn::synchronize: parameter " - << par->GetName() << " is now floating." << endl ; - } - } - - // Test if value changed - if (par->getVal()!= oldpar->getVal()) { - constValChange=kTRUE ; - if (verbose) { - oocoutI(_context,Minimization) << "RooMinimizerFcn::synchronize: value of constant parameter " - << par->GetName() - << " changed from " << oldpar->getVal() << " to " - << par->getVal() << endl ; - } - } - - } - - // Update reference list - *_initConstParamList = *_constParamList ; - - // Synchronize MINUIT with function state - // Handle floatParamList - for(index= 0; index < _floatParamList->getSize(); index++) { - RooRealVar *par= dynamic_cast(_floatParamList->at(index)) ; - - if (!par) continue ; - - Double_t pstep(0) ; - Double_t pmin(0) ; - Double_t pmax(0) ; - - if(!par->isConstant()) { - - // Verify that floating parameter is indeed of type RooRealVar - if (!par->IsA()->InheritsFrom(RooRealVar::Class())) { - oocoutW(_context,Minimization) << "RooMinimizerFcn::fit: Error, non-constant parameter " - << par->GetName() - << " is not of type RooRealVar, skipping" << endl ; - _floatParamList->remove(*par); - index--; - _nDim--; - continue ; - } - // make sure the parameter are in dirty state to enable - // a real NLL computation when the minimizer calls the function the first time - // (see issue #7659) - par->setValueDirty(); - - // Set the limits, if not infinite - if (par->hasMin() ) - pmin = par->getMin(); - if (par->hasMax() ) - pmax = par->getMax(); - - // Calculate step size - pstep = par->getError(); - if(pstep <= 0) { - // Floating parameter without error estitimate - if (par->hasMin() && par->hasMax()) { - pstep= 0.1*(pmax-pmin); - - // Trim default choice of error if within 2 sigma of limit - if (pmax - par->getVal() < 2*pstep) { - pstep = (pmax - par->getVal())/2 ; - } else if (par->getVal() - pmin < 2*pstep) { - pstep = (par->getVal() - pmin )/2 ; - } - - // If trimming results in zero error, restore default - if (pstep==0) { - pstep= 0.1*(pmax-pmin); - } - - } else { - pstep=1 ; - } - if(verbose) { - oocoutW(_context,Minimization) << "RooMinimizerFcn::synchronize: WARNING: no initial error estimate available for " - << par->GetName() << ": using " << pstep << endl; - } - } - } else { - pmin = par->getVal() ; - pmax = par->getVal() ; - } - - // new parameter - if (index>=Int_t(parameters.size())) { - - if (par->hasMin() && par->hasMax()) { - parameters.push_back(ROOT::Fit::ParameterSettings(par->GetName(), - par->getVal(), - pstep, - pmin,pmax)); - } - else { - parameters.push_back(ROOT::Fit::ParameterSettings(par->GetName(), - par->getVal(), - pstep)); - if (par->hasMin() ) - parameters.back().SetLowerLimit(pmin); - else if (par->hasMax() ) - parameters.back().SetUpperLimit(pmax); - } - - continue; - - } - - Bool_t oldFixed = parameters[index].IsFixed(); - Double_t oldVar = parameters[index].Value(); - Double_t oldVerr = parameters[index].StepSize(); - Double_t oldVlo = parameters[index].LowerLimit(); - Double_t oldVhi = parameters[index].UpperLimit(); - - if (par->isConstant() && !oldFixed) { - - // Parameter changes floating -> constant : update only value if necessary - if (oldVar!=par->getVal()) { - parameters[index].SetValue(par->getVal()); - if (verbose) { - oocoutI(_context,Minimization) << "RooMinimizerFcn::synchronize: value of parameter " - << par->GetName() << " changed from " << oldVar - << " to " << par->getVal() << endl ; - } - } - parameters[index].Fix(); - constStatChange=kTRUE ; - if (verbose) { - oocoutI(_context,Minimization) << "RooMinimizerFcn::synchronize: parameter " - << par->GetName() << " is now fixed." << endl ; - } - - } else if (par->isConstant() && oldFixed) { - - // Parameter changes constant -> constant : update only value if necessary - if (oldVar!=par->getVal()) { - parameters[index].SetValue(par->getVal()); - constValChange=kTRUE ; - - if (verbose) { - oocoutI(_context,Minimization) << "RooMinimizerFcn::synchronize: value of fixed parameter " - << par->GetName() << " changed from " << oldVar - << " to " << par->getVal() << endl ; - } - } - - } else { - // Parameter changes constant -> floating - if (!par->isConstant() && oldFixed) { - parameters[index].Release(); - constStatChange=kTRUE ; - - if (verbose) { - oocoutI(_context,Minimization) << "RooMinimizerFcn::synchronize: parameter " - << par->GetName() << " is now floating." << endl ; - } - } - - // Parameter changes constant -> floating : update all if necessary - if (oldVar!=par->getVal() || oldVlo!=pmin || oldVhi != pmax || oldVerr!=pstep) { - parameters[index].SetValue(par->getVal()); - parameters[index].SetStepSize(pstep); - if (par->hasMin() && par->hasMax() ) - parameters[index].SetLimits(pmin,pmax); - else if (par->hasMin() ) - parameters[index].SetLowerLimit(pmin); - else if (par->hasMax() ) - parameters[index].SetUpperLimit(pmax); - } - - // Inform user about changes in verbose mode - if (verbose) { - // if ierr<0, par was moved from the const list and a message was already printed - - if (oldVar!=par->getVal()) { - oocoutI(_context,Minimization) << "RooMinimizerFcn::synchronize: value of parameter " - << par->GetName() << " changed from " << oldVar << " to " - << par->getVal() << endl ; - } - if (oldVlo!=pmin || oldVhi!=pmax) { - oocoutI(_context,Minimization) << "RooMinimizerFcn::synchronize: limits of parameter " - << par->GetName() << " changed from [" << oldVlo << "," << oldVhi - << "] to [" << pmin << "," << pmax << "]" << endl ; - } - - // If oldVerr=0, then parameter was previously fixed - if (oldVerr!=pstep && oldVerr!=0) { - oocoutI(_context,Minimization) << "RooMinimizerFcn::synchronize: error/step size of parameter " - << par->GetName() << " changed from " << oldVerr << " to " << pstep << endl ; - } - } - } - } - - if (optConst) { - if (constStatChange) { - - RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::CollectErrors) ; - - oocoutI(_context,Minimization) << "RooMinimizerFcn::synchronize: set of constant parameters changed, rerunning const optimizer" << endl ; - _funct->constOptimizeTestStatistic(RooAbsArg::ConfigChange) ; - } else if (constValChange) { - oocoutI(_context,Minimization) << "RooMinimizerFcn::synchronize: constant parameter values changed, rerunning const optimizer" << endl ; - _funct->constOptimizeTestStatistic(RooAbsArg::ValueChange) ; - } - - RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::PrintErrors) ; - - } - - return 0 ; - -} - -/// Modify PDF parameter error by ordinal index (needed by MINUIT) -void RooMinimizerFcn::SetPdfParamErr(Int_t index, Double_t value) -{ - static_cast(_floatParamList->at(index))->setError(value); -} - -/// Modify PDF parameter error by ordinal index (needed by MINUIT) -void RooMinimizerFcn::ClearPdfParamAsymErr(Int_t index) -{ - static_cast(_floatParamList->at(index))->removeAsymError(); + RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::CollectErrors); + + if (_optConst && !flag) { + if (_context->getPrintLevel() > -1) + oocoutI(_context, Minimization) << "RooMinimizerFcn::setOptimizeConst: deactivating const optimization" << endl; + _funct->constOptimizeTestStatistic(RooAbsArg::DeActivate, true); + _optConst = flag; + } else if (!_optConst && flag) { + if (_context->getPrintLevel() > -1) + oocoutI(_context, Minimization) << "RooMinimizerFcn::setOptimizeConst: activating const optimization" << endl; + _funct->constOptimizeTestStatistic(RooAbsArg::Activate, flag > 1); + _optConst = flag; + } else if (_optConst && flag) { + if (_context->getPrintLevel() > -1) + oocoutI(_context, Minimization) << "RooMinimizerFcn::setOptimizeConst: const optimization already active" << endl; + } else { + if (_context->getPrintLevel() > -1) + oocoutI(_context, Minimization) << "RooMinimizerFcn::setOptimizeConst: const optimization wasn't active" << endl; + } + + RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::PrintErrors); } -/// Modify PDF parameter error by ordinal index (needed by MINUIT) -void RooMinimizerFcn::SetPdfParamErr(Int_t index, Double_t loVal, Double_t hiVal) -{ - static_cast(_floatParamList->at(index))->setAsymError(loVal,hiVal); -} +void RooMinimizerFcn::optimizeConstantTerms(bool constStatChange, bool constValChange) { + if (constStatChange) { -/// Transfer MINUIT fit results back into RooFit objects. -void RooMinimizerFcn::BackProp(const ROOT::Fit::FitResult &results) -{ - for (Int_t index= 0; index < _nDim; index++) { - Double_t value = results.Value(index); - SetPdfParamVal(index, value); - - // Set the parabolic error - Double_t err = results.Error(index); - SetPdfParamErr(index, err); - - Double_t eminus = results.LowerError(index); - Double_t eplus = results.UpperError(index); - - if(eplus > 0 || eminus < 0) { - // Store the asymmetric error, if it is available - SetPdfParamErr(index, eminus,eplus); - } else { - // Clear the asymmetric error - ClearPdfParamAsymErr(index) ; - } - } -} - -/// Change the file name for logging of a RooMinimizer of all MINUIT steppings -/// through the parameter space. If inLogfile is null, the current log file -/// is closed and logging is stopped. -Bool_t RooMinimizerFcn::SetLogFile(const char* inLogfile) -{ - if (_logfile) { - oocoutI(_context,Minimization) << "RooMinimizerFcn::setLogFile: closing previous log file" << endl ; - _logfile->close() ; - delete _logfile ; - _logfile = 0 ; - } - _logfile = new ofstream(inLogfile) ; - if (!_logfile->good()) { - oocoutI(_context,Minimization) << "RooMinimizerFcn::setLogFile: cannot open file " << inLogfile << endl ; - _logfile->close() ; - delete _logfile ; - _logfile= 0; - } - - return kFALSE ; -} - -/// Apply results of given external covariance matrix. i.e. propagate its errors -/// to all RRV parameter representations and give this matrix instead of the -/// HESSE matrix at the next save() call -void RooMinimizerFcn::ApplyCovarianceMatrix(TMatrixDSym& V) -{ - for (Int_t i=0 ; i<_nDim ; i++) { - // Skip fixed parameters - if (_floatParamList->at(i)->isConstant()) { - continue ; - } - SetPdfParamErr(i, sqrt(V(i,i))) ; - } - -} - -/// Set value of parameter i. -Bool_t RooMinimizerFcn::SetPdfParamVal(int index, double value) const -{ - auto par = static_cast(&(*_floatParamList)[index]); - - if (par->getVal()!=value) { - if (_verbose) cout << par->GetName() << "=" << value << ", " ; - - par->setVal(value); - return kTRUE; - } - - return kFALSE; -} - - -/// Print information about why evaluation failed. -/// Using _printEvalErrors, the number of errors printed can be steered. -/// Negative values disable printing. -void RooMinimizerFcn::printEvalErrors() const { - if (_printEvalErrors < 0) - return; - - std::ostringstream msg; - if (_doEvalErrorWall) { - msg << "RooMinimizerFcn: Minimized function has error status." << endl - << "Returning maximum FCN so far (" << _maxFCN - << ") to force MIGRAD to back out of this region. Error log follows.\n"; - } else { - msg << "RooMinimizerFcn: Minimized function has error status but is ignored.\n"; - } + RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::CollectErrors) ; - msg << "Parameter values: " ; - for (const auto par : *_floatParamList) { - auto var = static_cast(par); - msg << "\t" << var->GetName() << "=" << var->getVal() ; - } - msg << std::endl; + oocoutI(_context,Minimization) << "RooMinimizerFcn::optimizeConstantTerms: set of constant parameters changed, rerunning const optimizer" << endl ; + _funct->constOptimizeTestStatistic(RooAbsArg::ConfigChange, true) ; + } else if (constValChange) { + oocoutI(_context,Minimization) << "RooMinimizerFcn::optimizeConstantTerms: constant parameter values changed, rerunning const optimizer" << endl ; + _funct->constOptimizeTestStatistic(RooAbsArg::ValueChange, true) ; + } - RooAbsReal::printEvalErrors(msg, _printEvalErrors); - ooccoutW(_context,Minimization) << msg.str() << endl; + RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::PrintErrors) ; } @@ -496,7 +104,7 @@ void RooMinimizerFcn::printEvalErrors() const { double RooMinimizerFcn::DoEval(const double *x) const { // Set the parameter values for this iteration - for (int index = 0; index < _nDim; index++) { + for (unsigned index = 0; index < _nDim; index++) { if (_logfile) (*_logfile) << x[index] << " " ; SetPdfParamVal(index,x[index]); } @@ -539,3 +147,13 @@ double RooMinimizerFcn::DoEval(const double *x) const { return fvalue; } + +std::string RooMinimizerFcn::getFunctionName() const +{ + return _funct->GetName(); +} + +std::string RooMinimizerFcn::getFunctionTitle() const +{ + return _funct->GetTitle(); +} From 61dbdae79aba4c2b0a05d6a1bfa90673fa3397f2 Mon Sep 17 00:00:00 2001 From: "E. G. Patrick Bos" Date: Wed, 7 Jul 2021 12:07:35 +0200 Subject: [PATCH 3/3] remove obsolete parts RooMinimizer + refactor optimizeConst + address review _func and _optConst had already been moved into RooAbsMinimizerFcn, since they are logically part of the function, not the minimizer. This commit removes them from RooMinimizer. The necessary functionality for using the MinimizerFcn's members from RooMinimizer had already been added, but mistakenly were left out of the previous commits. As a consequence of this move, setOffsetting also had to be added to the MinimizerFcn. For backwards compatibility, we leave in RooMinimizer::setOffsetting and pass on the call to the MinimizerFcn. RooMinimizer::optimizeConst also still had a duplicate implementation. Now it forwards to RooAbsMinimizerFcn::setOptimizeConst. Note that we also refactored RooAbsMinimizerFcn::setOptimizeConst and ::optimizeConstantTerms here. They were previously virtual but, in fact, we duplicated 95% of their implementations in all derived classes. Both are now implemented non-virtually, and in turn call a (much smaller) new virtual function RooAbsMinimizerFcn::setOptimizeConstOnFunction, which must be implemented in the derived classes to pass on the constant term optimization settings to the actual function to be minimized. Furthermore, some PR review issues are addressed in this commit: - Remove obsolete __ROOFIT_NOROOMINIMIZER preprocessor flags. - Rename snake_case methods into dromedaryCase. - Use unique_ptr for parameter lists. - Add doxygen for new functions. - Bumped RooMinimizer ClassDef version. - Added TClass include to fix compile issue on some systems. --- roofit/roofitcore/inc/RooAbsMinimizerFcn.h | 43 +++++----- roofit/roofitcore/inc/RooMinimizer.h | 5 +- roofit/roofitcore/inc/RooMinimizerFcn.h | 7 +- roofit/roofitcore/src/RooAbsMinimizerFcn.cxx | 87 +++++++++++++------- roofit/roofitcore/src/RooMinimizer.cxx | 57 +++++-------- roofit/roofitcore/src/RooMinimizerFcn.cxx | 46 ++--------- 6 files changed, 113 insertions(+), 132 deletions(-) diff --git a/roofit/roofitcore/inc/RooAbsMinimizerFcn.h b/roofit/roofitcore/inc/RooAbsMinimizerFcn.h index bad09efceff01..8880ec73c27e3 100644 --- a/roofit/roofitcore/inc/RooAbsMinimizerFcn.h +++ b/roofit/roofitcore/inc/RooAbsMinimizerFcn.h @@ -12,8 +12,6 @@ * listed in LICENSE (http://roofit.sourceforge.net/license.txt) * *****************************************************************************/ -#ifndef __ROOFIT_NOROOMINIMIZER - #ifndef ROO_ABS_MINIMIZER_FCN #define ROO_ABS_MINIMIZER_FCN @@ -30,6 +28,7 @@ #include #include #include +#include // unique_ptr // forward declaration class RooMinimizer; @@ -39,14 +38,14 @@ class RooAbsMinimizerFcn { public: RooAbsMinimizerFcn(RooArgList paramList, RooMinimizer *context, bool verbose = false); RooAbsMinimizerFcn(const RooAbsMinimizerFcn &other); - virtual ~RooAbsMinimizerFcn(); + virtual ~RooAbsMinimizerFcn() = default; - // inform Minuit through its parameter_settings vector of RooFit parameter properties - Bool_t synchronize_parameter_settings(std::vector ¶meters, Bool_t optConst, Bool_t verbose); - // same, but can be overridden to e.g. also include gradient strategy synchronization in subclasses: + /// Informs Minuit through its parameter_settings vector of RooFit parameter properties. + Bool_t synchronizeParameterSettings(std::vector ¶meters, Bool_t optConst, Bool_t verbose); + /// Like synchronizeParameterSettings, Synchronize informs Minuit through its parameter_settings vector of RooFit parameter properties, + /// but Synchronize can be overridden to e.g. also include gradient strategy synchronization in subclasses. virtual Bool_t Synchronize(std::vector ¶meters, Bool_t optConst, Bool_t verbose); - // used for export to RooFitResult from Minimizer: RooArgList *GetFloatParamList(); RooArgList *GetConstParamList(); RooArgList *GetInitFloatParamList(); @@ -69,30 +68,37 @@ class RooAbsMinimizerFcn { double getOffset() const { return _funcOffset; } void SetVerbose(Bool_t flag = kTRUE); - // put Minuit results back into RooFit objects: + /// Put Minuit results back into RooFit objects. void BackProp(const ROOT::Fit::FitResult &results); - // used in several Minimizer functions: + /// RooMinimizer sometimes needs the name of the minimized function. Implement this in the derived class. virtual std::string getFunctionName() const = 0; + /// RooMinimizer sometimes needs the title of the minimized function. Implement this in the derived class. virtual std::string getFunctionTitle() const = 0; - // set different external covariance matrix + /// Set different external covariance matrix void ApplyCovarianceMatrix(TMatrixDSym &V); Bool_t SetLogFile(const char *inLogfile); std::ofstream *GetLogFile() { return _logfile; } - unsigned int get_nDim() const { return _nDim; } + unsigned int getNDim() const { return _nDim; } - virtual void setOptimizeConst(Int_t flag) = 0; + void setOptimizeConst(Int_t flag); bool getOptConst(); - std::vector get_parameter_values() const; + std::vector getParameterValues() const; Bool_t SetPdfParamVal(int index, double value) const; + /// Enable or disable offsetting on the function to be minimized, which enhances numerical precision. + virtual void setOffsetting(Bool_t flag) = 0; + protected: - virtual void optimizeConstantTerms(bool constStatChange, bool constValChange) = 0; + void optimizeConstantTerms(bool constStatChange, bool constValChange); + /// This function must be overridden in the derived class to pass on constant term optimization configuration + /// to the function to be minimized. For a RooAbsArg, this would be RooAbsArg::constOptimizeTestStatistic. + virtual void setOptimizeConstOnFunction(RooAbsArg::ConstOpCode opcode, Bool_t doAlsoTrackingOpt) = 0; // used in BackProp (Minuit results -> RooFit) and ApplyCovarianceMatrix void SetPdfParamErr(Int_t index, Double_t value); @@ -117,10 +123,10 @@ class RooAbsMinimizerFcn { Bool_t _optConst = kFALSE; - RooArgList *_floatParamList; - RooArgList *_constParamList; - RooArgList *_initFloatParamList; - RooArgList *_initConstParamList; + std::unique_ptr _floatParamList; + std::unique_ptr _constParamList; + std::unique_ptr _initFloatParamList; + std::unique_ptr _initConstParamList; std::ofstream *_logfile = nullptr; bool _doEvalErrorWall{true}; @@ -128,4 +134,3 @@ class RooAbsMinimizerFcn { }; #endif -#endif diff --git a/roofit/roofitcore/inc/RooMinimizer.h b/roofit/roofitcore/inc/RooMinimizer.h index 4b9e8d0711d94..db20a9f936cc1 100644 --- a/roofit/roofitcore/inc/RooMinimizer.h +++ b/roofit/roofitcore/inc/RooMinimizer.h @@ -77,7 +77,6 @@ class RooMinimizer : public TObject { void setProfile(Bool_t flag=kTRUE) { _profile = flag ; } Bool_t setLogFile(const char* logf=0) { return fitterFcn()->SetLogFile(logf); } - // necessary from RooAbsMinimizerFcn subclasses Int_t getPrintLevel() const; void setMinimizerType(const char* type) ; @@ -112,9 +111,7 @@ class RooMinimizer : public TObject { Int_t _printLevel ; Int_t _status ; - Bool_t _optConst ; Bool_t _profile ; - RooAbsReal* _func ; Bool_t _verbose ; TStopwatch _timer ; @@ -132,7 +129,7 @@ class RooMinimizer : public TObject { RooMinimizer(const RooMinimizer&) ; - ClassDef(RooMinimizer,0) // RooFit interface to ROOT::Fit::Fitter + ClassDef(RooMinimizer,1) // RooFit interface to ROOT::Fit::Fitter } ; diff --git a/roofit/roofitcore/inc/RooMinimizerFcn.h b/roofit/roofitcore/inc/RooMinimizerFcn.h index ea69209ddd33e..0d3626528e983 100644 --- a/roofit/roofitcore/inc/RooMinimizerFcn.h +++ b/roofit/roofitcore/inc/RooMinimizerFcn.h @@ -41,16 +41,17 @@ class RooMinimizerFcn : public RooAbsMinimizerFcn, public ROOT::Math::IBaseFunct virtual ~RooMinimizerFcn(); ROOT::Math::IBaseFunctionMultiDim *Clone() const override; - unsigned int NDim() const override { return get_nDim(); } + unsigned int NDim() const override { return getNDim(); } std::string getFunctionName() const override; std::string getFunctionTitle() const override; - void setOptimizeConst(Int_t flag) override; + void setOptimizeConstOnFunction(RooAbsArg::ConstOpCode opcode, Bool_t doAlsoTrackingOpt) override; + + void setOffsetting(Bool_t flag) override; private: double DoEval(const double *x) const override; - void optimizeConstantTerms(bool constStatChange, bool constValChange) override; RooAbsReal *_funct; }; diff --git a/roofit/roofitcore/src/RooAbsMinimizerFcn.cxx b/roofit/roofitcore/src/RooAbsMinimizerFcn.cxx index 6a1dc7b293cdb..233f3759c0daf 100644 --- a/roofit/roofitcore/src/RooAbsMinimizerFcn.cxx +++ b/roofit/roofitcore/src/RooAbsMinimizerFcn.cxx @@ -12,8 +12,6 @@ * listed in LICENSE (http://roofit.sourceforge.net/license.txt) * *****************************************************************************/ -#ifndef __ROOFIT_NOROOMINIMIZER - ////////////////////////////////////////////////////////////////////////////// // // RooAbsMinimizerFcn is an interface class to the ROOT::Math function @@ -33,6 +31,7 @@ #include "RooMinimizer.h" #include "RooNaNPacker.h" +#include "TClass.h" #include "TMatrixDSym.h" #include @@ -44,13 +43,13 @@ RooAbsMinimizerFcn::RooAbsMinimizerFcn(RooArgList paramList, RooMinimizer *conte : _context(context), _verbose(verbose) { // Examine parameter list - _floatParamList = (RooArgList *)paramList.selectByAttrib("Constant", kFALSE); + _floatParamList.reset((RooArgList *)paramList.selectByAttrib("Constant", kFALSE)); if (_floatParamList->getSize() > 1) { _floatParamList->sort(); } _floatParamList->setName("floatParamList"); - _constParamList = (RooArgList *)paramList.selectByAttrib("Constant", kTRUE); + _constParamList.reset((RooArgList *)paramList.selectByAttrib("Constant", kTRUE)); if (_constParamList->getSize() > 1) { _constParamList->sort(); } @@ -71,8 +70,8 @@ RooAbsMinimizerFcn::RooAbsMinimizerFcn(RooArgList paramList, RooMinimizer *conte _nDim = _floatParamList->getSize(); // Save snapshot of initial lists - _initFloatParamList = (RooArgList *)_floatParamList->snapshot(kFALSE); - _initConstParamList = (RooArgList *)_constParamList->snapshot(kFALSE); + _initFloatParamList.reset((RooArgList *)_floatParamList->snapshot(kFALSE)); + _initConstParamList.reset((RooArgList *)_constParamList->snapshot(kFALSE)); } RooAbsMinimizerFcn::RooAbsMinimizerFcn(const RooAbsMinimizerFcn &other) @@ -81,25 +80,17 @@ RooAbsMinimizerFcn::RooAbsMinimizerFcn(const RooAbsMinimizerFcn &other) _recoverFromNaNStrength(other._recoverFromNaNStrength), _numBadNLL(other._numBadNLL), _printEvalErrors(other._printEvalErrors), _evalCounter(other._evalCounter), - _nDim(other._nDim), _optConst(other._optConst), _logfile(other._logfile), _doEvalErrorWall(other._doEvalErrorWall), _verbose(other._verbose) -{ - _floatParamList = new RooArgList(*other._floatParamList); - _constParamList = new RooArgList(*other._constParamList); - _initFloatParamList = (RooArgList *)other._initFloatParamList->snapshot(kFALSE); - _initConstParamList = (RooArgList *)other._initConstParamList->snapshot(kFALSE); -} + _nDim(other._nDim), _optConst(other._optConst), + _floatParamList(new RooArgList(*other._floatParamList)), _constParamList(new RooArgList(*other._constParamList)), + _initFloatParamList((RooArgList *)other._initFloatParamList->snapshot(kFALSE)), + _initConstParamList((RooArgList *)other._initConstParamList->snapshot(kFALSE)), + _logfile(other._logfile), _doEvalErrorWall(other._doEvalErrorWall), _verbose(other._verbose) +{} -RooAbsMinimizerFcn::~RooAbsMinimizerFcn() -{ - delete _floatParamList; - delete _initFloatParamList; - delete _constParamList; - delete _initConstParamList; -} /// Internal function to synchronize TMinimizer with current /// information in RooAbsReal function parameters -Bool_t RooAbsMinimizerFcn::synchronize_parameter_settings(std::vector ¶meters, Bool_t optConst, Bool_t verbose) +Bool_t RooAbsMinimizerFcn::synchronizeParameterSettings(std::vector ¶meters, Bool_t optConst, Bool_t verbose) { Bool_t constValChange(kFALSE); Bool_t constStatChange(kFALSE); @@ -327,7 +318,7 @@ Bool_t RooAbsMinimizerFcn::synchronize_parameter_settings(std::vector ¶meters, Bool_t optConst, Bool_t verbose) { - return synchronize_parameter_settings(parameters, optConst, verbose); + return synchronizeParameterSettings(parameters, optConst, verbose); } /// Modify PDF parameter error by ordinal index (needed by MINUIT) @@ -457,19 +448,19 @@ void RooAbsMinimizerFcn::printEvalErrors() const { RooArgList *RooAbsMinimizerFcn::GetFloatParamList() { - return _floatParamList; + return _floatParamList.get(); } RooArgList *RooAbsMinimizerFcn::GetConstParamList() { - return _constParamList; + return _constParamList.get(); } RooArgList *RooAbsMinimizerFcn::GetInitFloatParamList() { - return _initFloatParamList; + return _initFloatParamList.get(); } RooArgList *RooAbsMinimizerFcn::GetInitConstParamList() { - return _initConstParamList; + return _initConstParamList.get(); } void RooAbsMinimizerFcn::SetEvalErrorWall(Bool_t flag) @@ -504,12 +495,52 @@ void RooAbsMinimizerFcn::SetVerbose(Bool_t flag) _verbose = flag; } +void RooAbsMinimizerFcn::setOptimizeConst(Int_t flag) +{ + RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::CollectErrors); + + if (_optConst && !flag) { + if (_context->getPrintLevel() > -1) + oocoutI(_context, Minimization) << "RooAbsMinimizerFcn::setOptimizeConst: deactivating const optimization" << endl; + setOptimizeConstOnFunction(RooAbsArg::DeActivate, true); + _optConst = flag; + } else if (!_optConst && flag) { + if (_context->getPrintLevel() > -1) + oocoutI(_context, Minimization) << "RooAbsMinimizerFcn::setOptimizeConst: activating const optimization" << endl; + setOptimizeConstOnFunction(RooAbsArg::Activate, flag > 1); + _optConst = flag; + } else if (_optConst && flag) { + if (_context->getPrintLevel() > -1) + oocoutI(_context, Minimization) << "RooAbsMinimizerFcn::setOptimizeConst: const optimization already active" << endl; + } else { + if (_context->getPrintLevel() > -1) + oocoutI(_context, Minimization) << "RooAbsMinimizerFcn::setOptimizeConst: const optimization wasn't active" << endl; + } + + RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::PrintErrors); +} + +void RooAbsMinimizerFcn::optimizeConstantTerms(bool constStatChange, bool constValChange) { + if (constStatChange) { + + RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::CollectErrors) ; + + oocoutI(_context,Minimization) << "RooAbsMinimizerFcn::optimizeConstantTerms: set of constant parameters changed, rerunning const optimizer" << endl ; + setOptimizeConstOnFunction(RooAbsArg::ConfigChange, true) ; + } else if (constValChange) { + oocoutI(_context,Minimization) << "RooAbsMinimizerFcn::optimizeConstantTerms: constant parameter values changed, rerunning const optimizer" << endl ; + setOptimizeConstOnFunction(RooAbsArg::ValueChange, true) ; + } + + RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::PrintErrors) ; +} + bool RooAbsMinimizerFcn::getOptConst() { return _optConst; } -std::vector RooAbsMinimizerFcn::get_parameter_values() const +std::vector RooAbsMinimizerFcn::getParameterValues() const { // TODO: make a cache for this somewhere so it doesn't have to be recreated on each call std::vector values; @@ -522,5 +553,3 @@ std::vector RooAbsMinimizerFcn::get_parameter_values() const return values; } - -#endif diff --git a/roofit/roofitcore/src/RooMinimizer.cxx b/roofit/roofitcore/src/RooMinimizer.cxx index 08b9d344aaf12..a579d85bbdd8b 100644 --- a/roofit/roofitcore/src/RooMinimizer.cxx +++ b/roofit/roofitcore/src/RooMinimizer.cxx @@ -111,8 +111,6 @@ RooMinimizer::RooMinimizer(RooAbsReal& function) // Store function reference _extV = 0 ; - _func = &function ; - _optConst = kFALSE ; _verbose = kFALSE ; _profile = kFALSE ; _profileStart = kFALSE ; @@ -121,7 +119,7 @@ RooMinimizer::RooMinimizer(RooAbsReal& function) if (_theFitter) delete _theFitter ; _theFitter = new ROOT::Fit::Fitter; - _fcn = new RooMinimizerFcn(_func,this,_verbose); + _fcn = new RooMinimizerFcn(&function,this,_verbose); _theFitter->Config().SetMinimizer(_minimizerType.c_str()); setEps(1.0); // default tolerance // default max number of calls @@ -132,11 +130,11 @@ RooMinimizer::RooMinimizer(RooAbsReal& function) setPrintLevel(-1) ; // Use +0.5 for 1-sigma errors - setErrorLevel(_func->defaultErrorLevel()) ; + setErrorLevel(function.defaultErrorLevel()) ; // Declare our parameters to MINUIT _fcn->Synchronize(_theFitter->Config().ParamsSettings(), - _optConst,_verbose) ; + _fcn->getOptConst(),_verbose) ; // Now set default verbosity if (RooMsgService::instance().silentMode()) { @@ -232,7 +230,7 @@ void RooMinimizer::setEps(Double_t eps) void RooMinimizer::setOffsetting(Bool_t flag) { - _func->enableOffsetting(flag) ; + _fcn->setOffsetting(flag); } @@ -286,7 +284,7 @@ RooFitResult* RooMinimizer::fit(const char* options) // Initial configuration if (opts.Contains("v")) setVerbose(1) ; if (opts.Contains("t")) setProfile(1) ; - if (opts.Contains("l")) setLogFile(Form("%s.log",_func->GetName())) ; + if (opts.Contains("l")) setLogFile(Form("%s.log",_fcn->getFunctionName().c_str())) ; if (opts.Contains("c")) optimizeConst(1) ; // Fitting steps @@ -310,7 +308,7 @@ RooFitResult* RooMinimizer::fit(const char* options) Int_t RooMinimizer::minimize(const char* type, const char* alg) { _fcn->Synchronize(_theFitter->Config().ParamsSettings(), - _optConst,_verbose) ; + _fcn->getOptConst(),_verbose) ; _minimizerType = type; _theFitter->Config().SetMinimizer(type,alg); @@ -342,7 +340,7 @@ Int_t RooMinimizer::minimize(const char* type, const char* alg) Int_t RooMinimizer::migrad() { _fcn->Synchronize(_theFitter->Config().ParamsSettings(), - _optConst,_verbose) ; + _fcn->getOptConst(),_verbose) ; profileStart() ; RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::CollectErrors) ; RooAbsReal::clearEvalErrorLog() ; @@ -378,7 +376,7 @@ Int_t RooMinimizer::hesse() else { _fcn->Synchronize(_theFitter->Config().ParamsSettings(), - _optConst,_verbose) ; + _fcn->getOptConst(),_verbose) ; profileStart() ; RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::CollectErrors) ; RooAbsReal::clearEvalErrorLog() ; @@ -415,7 +413,7 @@ Int_t RooMinimizer::minos() else { _fcn->Synchronize(_theFitter->Config().ParamsSettings(), - _optConst,_verbose) ; + _fcn->getOptConst(),_verbose) ; profileStart() ; RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::CollectErrors) ; RooAbsReal::clearEvalErrorLog() ; @@ -453,7 +451,7 @@ Int_t RooMinimizer::minos(const RooArgSet& minosParamList) else if (minosParamList.getSize()>0) { _fcn->Synchronize(_theFitter->Config().ParamsSettings(), - _optConst,_verbose) ; + _fcn->getOptConst(),_verbose) ; profileStart() ; RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::CollectErrors) ; RooAbsReal::clearEvalErrorLog() ; @@ -505,7 +503,7 @@ Int_t RooMinimizer::minos(const RooArgSet& minosParamList) Int_t RooMinimizer::seek() { _fcn->Synchronize(_theFitter->Config().ParamsSettings(), - _optConst,_verbose) ; + _fcn->getOptConst(),_verbose) ; profileStart() ; RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::CollectErrors) ; RooAbsReal::clearEvalErrorLog() ; @@ -534,7 +532,7 @@ Int_t RooMinimizer::seek() Int_t RooMinimizer::simplex() { _fcn->Synchronize(_theFitter->Config().ParamsSettings(), - _optConst,_verbose) ; + _fcn->getOptConst(),_verbose) ; profileStart() ; RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::CollectErrors) ; RooAbsReal::clearEvalErrorLog() ; @@ -563,7 +561,7 @@ Int_t RooMinimizer::simplex() Int_t RooMinimizer::improve() { _fcn->Synchronize(_theFitter->Config().ParamsSettings(), - _optConst,_verbose) ; + _fcn->getOptConst(),_verbose) ; profileStart() ; RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::CollectErrors) ; RooAbsReal::clearEvalErrorLog() ; @@ -600,24 +598,7 @@ Int_t RooMinimizer::setPrintLevel(Int_t newLevel) void RooMinimizer::optimizeConst(Int_t flag) { - RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::CollectErrors) ; - - if (_optConst && !flag){ - if (_printLevel>-1) coutI(Minimization) << "RooMinimizer::optimizeConst: deactivating const optimization" << endl ; - _func->constOptimizeTestStatistic(RooAbsArg::DeActivate) ; - _optConst = flag ; - } else if (!_optConst && flag) { - if (_printLevel>-1) coutI(Minimization) << "RooMinimizer::optimizeConst: activating const optimization" << endl ; - _func->constOptimizeTestStatistic(RooAbsArg::Activate,flag>1) ; - _optConst = flag ; - } else if (_optConst && flag) { - if (_printLevel>-1) coutI(Minimization) << "RooMinimizer::optimizeConst: const optimization already active" << endl ; - } else { - if (_printLevel>-1) coutI(Minimization) << "RooMinimizer::optimizeConst: const optimization wasn't active" << endl ; - } - - RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::PrintErrors) ; - + _fcn->setOptimizeConst(flag); } @@ -639,8 +620,8 @@ RooFitResult* RooMinimizer::save(const char* userName, const char* userTitle) } TString name,title ; - name = userName ? userName : Form("%s", _func->GetName()) ; - title = userTitle ? userTitle : Form("%s", _func->GetTitle()) ; + name = userName ? userName : Form("%s", _fcn->getFunctionName().c_str()) ; + title = userTitle ? userTitle : Form("%s", _fcn->getFunctionTitle().c_str()) ; RooFitResult* fitRes = new RooFitResult(name,title) ; // Move eventual fixed parameters in floatList to constList @@ -727,7 +708,7 @@ RooPlot* RooMinimizer::contour(RooRealVar& var1, RooRealVar& var2, coutE(Minimization) << "RooMinimizer::contour(" << GetName() << ") ERROR: " << var1.GetName() << " is not a floating parameter of " - << _func->GetName() << endl ; + << _fcn->getFunctionName() << endl ; return 0; } @@ -736,7 +717,7 @@ RooPlot* RooMinimizer::contour(RooRealVar& var1, RooRealVar& var2, coutE(Minimization) << "RooMinimizer::contour(" << GetName() << ") ERROR: " << var2.GetName() << " is not a floating parameter of PDF " - << _func->GetName() << endl ; + << _fcn->getFunctionName() << endl ; return 0; } @@ -783,7 +764,7 @@ RooPlot* RooMinimizer::contour(RooRealVar& var1, RooRealVar& var2, ycoor[npoints] = ycoor[0]; TGraph* graph = new TGraph(npoints+1,xcoor,ycoor); - graph->SetName(Form("contour_%s_n%f",_func->GetName(),n[ic])) ; + graph->SetName(Form("contour_%s_n%f",_fcn->getFunctionName().c_str(),n[ic])) ; graph->SetLineStyle(ic+1) ; graph->SetLineWidth(2) ; graph->SetLineColor(kBlue) ; diff --git a/roofit/roofitcore/src/RooMinimizerFcn.cxx b/roofit/roofitcore/src/RooMinimizerFcn.cxx index ce587f5c9b2dd..8a3cd0e3416af 100644 --- a/roofit/roofitcore/src/RooMinimizerFcn.cxx +++ b/roofit/roofitcore/src/RooMinimizerFcn.cxx @@ -12,7 +12,6 @@ * listed in LICENSE (http://roofit.sourceforge.net/license.txt) * *****************************************************************************/ - ////////////////////////////////////////////////////////////////////////////// /// \class RooMinimizerFcn /// RooMinimizerFcn is an interface to the ROOT::Math::IBaseFunctionMultiDim, @@ -59,47 +58,11 @@ ROOT::Math::IBaseFunctionMultiDim* RooMinimizerFcn::Clone() const return new RooMinimizerFcn(*this) ; } -void RooMinimizerFcn::setOptimizeConst(Int_t flag) +void RooMinimizerFcn::setOptimizeConstOnFunction(RooAbsArg::ConstOpCode opcode, Bool_t doAlsoTrackingOpt) { - RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::CollectErrors); - - if (_optConst && !flag) { - if (_context->getPrintLevel() > -1) - oocoutI(_context, Minimization) << "RooMinimizerFcn::setOptimizeConst: deactivating const optimization" << endl; - _funct->constOptimizeTestStatistic(RooAbsArg::DeActivate, true); - _optConst = flag; - } else if (!_optConst && flag) { - if (_context->getPrintLevel() > -1) - oocoutI(_context, Minimization) << "RooMinimizerFcn::setOptimizeConst: activating const optimization" << endl; - _funct->constOptimizeTestStatistic(RooAbsArg::Activate, flag > 1); - _optConst = flag; - } else if (_optConst && flag) { - if (_context->getPrintLevel() > -1) - oocoutI(_context, Minimization) << "RooMinimizerFcn::setOptimizeConst: const optimization already active" << endl; - } else { - if (_context->getPrintLevel() > -1) - oocoutI(_context, Minimization) << "RooMinimizerFcn::setOptimizeConst: const optimization wasn't active" << endl; - } - - RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::PrintErrors); -} - -void RooMinimizerFcn::optimizeConstantTerms(bool constStatChange, bool constValChange) { - if (constStatChange) { - - RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::CollectErrors) ; - - oocoutI(_context,Minimization) << "RooMinimizerFcn::optimizeConstantTerms: set of constant parameters changed, rerunning const optimizer" << endl ; - _funct->constOptimizeTestStatistic(RooAbsArg::ConfigChange, true) ; - } else if (constValChange) { - oocoutI(_context,Minimization) << "RooMinimizerFcn::optimizeConstantTerms: constant parameter values changed, rerunning const optimizer" << endl ; - _funct->constOptimizeTestStatistic(RooAbsArg::ValueChange, true) ; - } - - RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::PrintErrors) ; + _funct->constOptimizeTestStatistic(opcode, doAlsoTrackingOpt); } - /// Evaluate function given the parameters in `x`. double RooMinimizerFcn::DoEval(const double *x) const { @@ -157,3 +120,8 @@ std::string RooMinimizerFcn::getFunctionTitle() const { return _funct->GetTitle(); } + +void RooMinimizerFcn::setOffsetting(Bool_t flag) +{ + _funct->enableOffsetting(flag); +}