From 99e33a40955c7497842adef1aa930cd4d38cdac1 Mon Sep 17 00:00:00 2001 From: adavidzh Date: Thu, 20 Nov 2014 16:23:27 +0100 Subject: [PATCH 1/2] Bug fix for #138. Closes #153. --- interface/CloseCoutSentry.h | 2 ++ interface/utils.h | 5 ++-- src/CascadeMinimizer.cc | 9 ++++--- src/CloseCoutSentry.cc | 10 ++++++++ src/utils.cc | 51 +++++++++++++++++++++++++------------ 5 files changed, 55 insertions(+), 22 deletions(-) diff --git a/interface/CloseCoutSentry.h b/interface/CloseCoutSentry.h index f05a4c46c60..97538d71a01 100644 --- a/interface/CloseCoutSentry.h +++ b/interface/CloseCoutSentry.h @@ -13,6 +13,7 @@ class CloseCoutSentry { // break through any sentry, even the ones above myself (for critical error messages, or debug) static void breakFree() ; FILE *trueStdOut(); + static FILE *trueStdOutGlobal(); private: bool silent_; static int fdOut_, fdErr_; @@ -20,6 +21,7 @@ class CloseCoutSentry { // always clear, even if I was not the one closing it void static reallyClear() ; static FILE *trueStdOut_; + static CloseCoutSentry *owner_; bool stdOutIsMine_; }; diff --git a/interface/utils.h b/interface/utils.h index 651e5daa6ca..61c2eb71f6a 100644 --- a/interface/utils.h +++ b/interface/utils.h @@ -3,6 +3,7 @@ #include #include +#include struct RooDataHist; struct RooAbsData; struct RooAbsPdf; @@ -93,8 +94,8 @@ namespace utils { // Set range of physics model parameters void setModelParameterRanges( const std::string & setPhysicsModelParameterRangeExpression, const RooArgSet & params); - bool checkParameterBoundary( const RooRealVar &); - bool checkParameterBoundaries( const RooArgSet &); + bool isParameterAtBoundary( const RooRealVar &); + bool anyParameterAtBoundaries( const RooArgSet &, int verbosity); void reorderCombinations(std::vector > &, const std::vector &, const std::vector &); std::vector > generateCombinations(const std::vector &vec); diff --git a/src/CascadeMinimizer.cc b/src/CascadeMinimizer.cc index b71a85899de..da7b5fcf5b6 100644 --- a/src/CascadeMinimizer.cc +++ b/src/CascadeMinimizer.cc @@ -278,10 +278,11 @@ bool CascadeMinimizer::minimize(int verbose, bool cascade) nllParams->remove(CascadeMinimizerGlobalConfigs::O().pdfCategories); nllParams->remove(CascadeMinimizerGlobalConfigs::O().parametersOfInterest); - bool boundariesOk = utils::checkParameterBoundaries(*nllParams); - if(!boundariesOk){ - std::cout << " [WARNING] After the fit some parameters are at their boundary (see above)." << std::endl; - std::cout << " [WARNING] Are you sure your model is correct?" << std::endl; + bool boundariesNotOk = utils::anyParameterAtBoundaries(*nllParams, verbose); + if(boundariesNotOk && verbose >= 1){ + fprintf(CloseCoutSentry::trueStdOutGlobal(), + " [WARNING] After the fit some parameters are at their boundary.\n" + " [WARNING] Are you sure your model is correct?\n"); } return ret; diff --git a/src/CloseCoutSentry.cc b/src/CloseCoutSentry.cc index ab239d5807b..47c545a4e70 100644 --- a/src/CloseCoutSentry.cc +++ b/src/CloseCoutSentry.cc @@ -7,6 +7,8 @@ bool CloseCoutSentry::open_ = true; int CloseCoutSentry::fdOut_ = 0; int CloseCoutSentry::fdErr_ = 0; FILE * CloseCoutSentry::trueStdOut_ = 0; +CloseCoutSentry *CloseCoutSentry::owner_ = 0; + CloseCoutSentry::CloseCoutSentry(bool silent) : silent_(silent), stdOutIsMine_(false) @@ -20,6 +22,7 @@ CloseCoutSentry::CloseCoutSentry(bool silent) : } freopen("/dev/null", "w", stdout); freopen("/dev/null", "w", stderr); + owner_ = this; } else { silent_ = false; } @@ -47,6 +50,7 @@ void CloseCoutSentry::reallyClear() sprintf(buf, "/dev/fd/%d", fdOut_); freopen(buf, "w", stdout); sprintf(buf, "/dev/fd/%d", fdErr_); freopen(buf, "w", stderr); open_ = true; + owner_ = 0; fdOut_ = fdErr_ = 0; } } @@ -56,6 +60,12 @@ void CloseCoutSentry::breakFree() reallyClear(); } +FILE *CloseCoutSentry::trueStdOutGlobal() +{ + if (!owner_) return stdout; + return owner_->trueStdOut(); +} + FILE *CloseCoutSentry::trueStdOut() { if (open_) return stdout; diff --git a/src/utils.cc b/src/utils.cc index d32209634bb..8896edc292f 100644 --- a/src/utils.cc +++ b/src/utils.cc @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -414,9 +415,11 @@ void utils::copyAttributes(const RooAbsArg &from, RooAbsArg &to) { if (!attribs.empty()) { for (std::set::const_iterator it = attribs.begin(), ed = attribs.end(); it != ed; ++it) to.setAttribute(it->c_str()); } - const std::map strattribs = from.stringAttributes(); + const std::map + strattribs = from.stringAttributes(); if (!strattribs.empty()) { - for (std::map::const_iterator it = strattribs.begin(), ed = strattribs.end(); it != ed; ++it) to.setStringAttribute(it->first.c_str(), it->second.c_str()); + for (std::map + ::const_iterator it = strattribs.begin(), ed = strattribs.end(); it != ed; ++it) to.setStringAttribute(it->first.c_str(), it->second.c_str()); } } @@ -695,7 +698,8 @@ std::vector > utils::generateCombinations(const std::vector timesFoundAtBoundary; + bool isAnyBad = true; RooLinkedListIter iter = params.iterator(); int i = 0; for (RooRealVar *a = (RooRealVar *) iter.Next(); a != 0; a = (RooRealVar *) iter.Next(), ++i) { - bool isBad = checkParameterBoundary(*a); - isNoneBad &= isBad; + + bool isBad = isParameterAtBoundary(*a); + + if(isBad){ + std::string varName((*a).GetName()); + + if( verbosity >= 9 || (timesFoundAtBoundary[varName] < 3 && verbosity > -1) ){ + fprintf(CloseCoutSentry::trueStdOutGlobal()," [WARNING] Found [%s] at boundary. \n", (*a).GetName()); + std::cout << " "; (*a).Print(); + } + + timesFoundAtBoundary[varName]++; + } + + isAnyBad |= isBad; } - return isNoneBad; + // for( std::unordered_map::value_type e : timesFoundAtBoundary ){ + // printf("e %s -> %i\n", e.first.c_str(), e.second); + // } + + return isAnyBad; } - From 5062fa53561c95daf26336e2bbda3469a69db1ce Mon Sep 17 00:00:00 2001 From: adavidzh Date: Thu, 20 Nov 2014 17:36:15 +0100 Subject: [PATCH 2/2] Invert initaliser after having inverted logic. --- src/utils.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils.cc b/src/utils.cc index 8896edc292f..c5db9ad5365 100644 --- a/src/utils.cc +++ b/src/utils.cc @@ -722,7 +722,7 @@ bool utils::isParameterAtBoundary( const RooRealVar ¶m ){ bool utils::anyParameterAtBoundaries( const RooArgSet ¶ms, int verbosity ){ static std::unordered_map timesFoundAtBoundary; - bool isAnyBad = true; + bool isAnyBad = false; RooLinkedListIter iter = params.iterator(); int i = 0; for (RooRealVar *a = (RooRealVar *) iter.Next(); a != 0; a = (RooRealVar *) iter.Next(), ++i) {