From 722e42cbda365e3d09ab8a907cdd65c51617098f Mon Sep 17 00:00:00 2001 From: Viviane Rochon Montplaisir Date: Thu, 19 Dec 2019 12:35:47 -0500 Subject: [PATCH] Small fixes after feedback from expert users --- examples/basic/batch/example1/bb.cpp | 4 +- src/Algos/MainStep.cpp | 4 +- src/Algos/NelderMead/NMReflective.cpp | 6 +-- src/Algos/Projection.cpp | 11 +++-- .../SgtelibModel/SgtelibModelFilterCache.cpp | 28 +++++------- .../SgtelibModelMegaIteration.cpp | 3 +- src/Cache/CacheSet.cpp | 2 +- src/Eval/BBOutput.cpp | 45 +++++++++++-------- src/Eval/BBOutput.hpp | 11 ++--- src/Eval/Eval.cpp | 14 ++++-- src/Eval/EvalPoint.cpp | 28 ++++++++++-- src/Math/Double.cpp | 24 +++++----- 12 files changed, 104 insertions(+), 76 deletions(-) diff --git a/examples/basic/batch/example1/bb.cpp b/examples/basic/batch/example1/bb.cpp index d44b19ce..acce5812 100644 --- a/examples/basic/batch/example1/bb.cpp +++ b/examples/basic/batch/example1/bb.cpp @@ -111,12 +111,12 @@ int main (int argc, char **argv) f -= (sum1 -2*prod1) / std::abs(sqrt(sum3)); } // Scale - if (!isnan(f)) + if (!std::isnan(f)) { f *= 1e-5; } - eval_ok = !isnan(f); + eval_ok = !std::isnan(f); g3 = - (f + 2000); } diff --git a/src/Algos/MainStep.cpp b/src/Algos/MainStep.cpp index ace143f9..cc1f025d 100644 --- a/src/Algos/MainStep.cpp +++ b/src/Algos/MainStep.cpp @@ -459,7 +459,7 @@ void NOMAD::MainStep::printNumThreads() const bool NOMAD::MainStep::detectPhaseOne() { bool hasEBConstraints = false; - bool hasNoFeas = true; + bool hasNoFeas = !NOMAD::CacheBase::getInstance()->hasFeas(); auto bbOutputTypeList = _allParams->getEvalParams()->getAttributeValue("BB_OUTPUT_TYPE"); if (std::find(bbOutputTypeList.begin(), bbOutputTypeList.end(), NOMAD::BBOutputType::EB) @@ -468,8 +468,6 @@ bool NOMAD::MainStep::detectPhaseOne() hasEBConstraints = true; } - hasNoFeas = !NOMAD::CacheBase::getInstance()->hasFeas(); - return hasEBConstraints && hasNoFeas; } diff --git a/src/Algos/NelderMead/NMReflective.cpp b/src/Algos/NelderMead/NMReflective.cpp index 180b0be2..58f1573c 100644 --- a/src/Algos/NelderMead/NMReflective.cpp +++ b/src/Algos/NelderMead/NMReflective.cpp @@ -195,10 +195,10 @@ void NOMAD::NMReflective::generateTrialPoints () // Determine the centroid of Y std::set::iterator it; std::set::iterator itBeforeEnd = _nmY->end(); - itBeforeEnd--; + --itBeforeEnd; int i=0; NOMAD::Point yc(n,0); - for ( it = _nmY->begin() ; it != itBeforeEnd ; it++,i++) + for (it = _nmY->begin() ; it != itBeforeEnd ; ++it, i++) { AddOutputInfo("y" + std::to_string(i) + ": " + (*it).display() ); for (size_t k = 0 ; k < n ; ++k ) @@ -483,7 +483,7 @@ bool NOMAD::NMReflective::insertInYBest( const NOMAD::Point & x1 , const NOMAD:: bool x2EvalOK = ( X2.getEvalStatus(evalType) == NOMAD::EvalStatusType::EVAL_OK ); std::set::iterator itYn = _nmY->end(); - itYn--; + --itYn; AddOutputDebug("Insertion/Deletion of points in Y: "); diff --git a/src/Algos/Projection.cpp b/src/Algos/Projection.cpp index 03cffe3c..094920ab 100644 --- a/src/Algos/Projection.cpp +++ b/src/Algos/Projection.cpp @@ -178,7 +178,10 @@ void NOMAD::Projection::projectPoint(const NOMAD::EvalPoint& oraclePoint) AddOutputInfo(subStepName, true, false); NOMAD::EvalPointSet trySet; - size_t nbCachePoints; + + NOMAD::CacheInterface cacheInterface(this); + std::vector evalPointList; + size_t nbCachePoints = cacheInterface.getAllPoints(evalPointList); for (auto index : _indexSet) { @@ -186,10 +189,6 @@ void NOMAD::Projection::projectPoint(const NOMAD::EvalPoint& oraclePoint) auto perturbation = computePerturbation(oraclePoint, index); // Loop on points of the cache - NOMAD::CacheInterface cacheInterface(this); - std::vector evalPointList; - nbCachePoints = cacheInterface.getAllPoints(evalPointList); - for (auto xRef : evalPointList) { auto xTry = buildProjectionTrialPoint(xRef, perturbation); @@ -199,7 +198,7 @@ void NOMAD::Projection::projectPoint(const NOMAD::EvalPoint& oraclePoint) s = std::to_string(_indexSet.size()) + " perturbation vectors"; NOMAD::OutputQueue::Add(s, _displayLevel); - s = std::to_string(nbCachePoints) + " perturbation vectors"; + s = std::to_string(nbCachePoints) + " cache points"; NOMAD::OutputQueue::Add(s, _displayLevel); s = std::to_string(trySet.size()) + " projection candidates"; NOMAD::OutputQueue::Add(s, _displayLevel); diff --git a/src/Algos/SgtelibModel/SgtelibModelFilterCache.cpp b/src/Algos/SgtelibModel/SgtelibModelFilterCache.cpp index eda7113e..f739f1c8 100644 --- a/src/Algos/SgtelibModel/SgtelibModelFilterCache.cpp +++ b/src/Algos/SgtelibModel/SgtelibModelFilterCache.cpp @@ -113,7 +113,7 @@ bool NOMAD::SgtelibModelFilterCache::runImp() // Get used methods from parameter const size_t methodMax = 10; - bool useMethod[methodMax]; + bool useMethod[methodMax] = { false }; size_t nbMethods = 0; std::string filterParam = _runParams->getAttributeValue("SGTELIB_MODEL_FILTER"); for (size_t i = 0; i < methodMax; i++) @@ -123,10 +123,6 @@ bool NOMAD::SgtelibModelFilterCache::runImp() useMethod[i] = true; nbMethods++; } - else - { - useMethod[i] = false; - } } // Display info @@ -389,7 +385,6 @@ int NOMAD::SgtelibModelFilterCache::applyMethod(NOMAD::FilterSelectionMethod met double dmin = 0; double dmax = 0; int nmax = 0; - int ni = -1; double deltaMNorm = 0; if (_modelAlgo->getDeltaMNorm().isDefined()) { @@ -431,6 +426,7 @@ int NOMAD::SgtelibModelFilterCache::applyMethod(NOMAD::FilterSelectionMethod met case NOMAD::FilterSelectionMethod::METHOD_BEST_MIN_DIST: // Select the best point but with a minimum distance to points already selected + // dmin is 0 at this point s = "dmin = " + NOMAD::Double(dmin).display(); NOMAD::OutputQueue::Add(s, _displayLevel); for (size_t i = 0; i < nbSgte; i++) @@ -445,15 +441,15 @@ int NOMAD::SgtelibModelFilterCache::applyMethod(NOMAD::FilterSelectionMethod met iSelect = static_cast(i); } } - } - if (-1 != iSelect) - { - s = "d = " + NOMAD::Double(_DTX[iSelect]).display(); - NOMAD::OutputQueue::Add(s, _displayLevel); - s = "h select = " + NOMAD::Double(hmin).display(); - NOMAD::OutputQueue::Add(s, _displayLevel); + if (-1 != iSelect) + { + s = "d = " + NOMAD::Double(_DTX[iSelect]).display(); + NOMAD::OutputQueue::Add(s, _displayLevel); + s = "h select = " + NOMAD::Double(hmin).display(); + NOMAD::OutputQueue::Add(s, _displayLevel); - dmin = _DTX[iSelect] + deltaMNorm; + dmin = std::max(dmin, _DTX[iSelect] + deltaMNorm); + } } break; @@ -486,7 +482,7 @@ int NOMAD::SgtelibModelFilterCache::applyMethod(NOMAD::FilterSelectionMethod met { if ( (!_keep[i]) && (_distIsolation[i] > 0) ) { - ni = _nIsolation[i]; + int ni = _nIsolation[i]; // If criteria undef, then compute. if (-1 == ni) { @@ -519,7 +515,7 @@ int NOMAD::SgtelibModelFilterCache::applyMethod(NOMAD::FilterSelectionMethod met { if ( (!_keep[i]) && (_DTX[i] > 0) ) { - ni = _nDensity[i]; + int ni = _nDensity[i]; // If criteria undef, then compute. if (-1 == ni) { diff --git a/src/Algos/SgtelibModel/SgtelibModelMegaIteration.cpp b/src/Algos/SgtelibModel/SgtelibModelMegaIteration.cpp index 415e3c4a..941753d6 100644 --- a/src/Algos/SgtelibModel/SgtelibModelMegaIteration.cpp +++ b/src/Algos/SgtelibModel/SgtelibModelMegaIteration.cpp @@ -97,8 +97,7 @@ bool NOMAD::SgtelibModelMegaIteration::runImp() else { // DEBUG - ensure OPPORTUNISM is off. - NOMAD::EvcInterface evcInterface(this); - auto evcParams = evcInterface.getEvaluatorControl()->getEvaluatorControlParams(); + auto evcParams = NOMAD::EvcInterface::getEvaluatorControl()->getEvaluatorControlParams(); auto previousOpportunism = evcParams->getAttributeValue("OPPORTUNISTIC_EVAL"); if (previousOpportunism) { diff --git a/src/Cache/CacheSet.cpp b/src/Cache/CacheSet.cpp index 69ee277f..8b5fed81 100644 --- a/src/Cache/CacheSet.cpp +++ b/src/Cache/CacheSet.cpp @@ -227,7 +227,7 @@ bool NOMAD::CacheSet::smartInsert(const NOMAD::EvalPoint &evalPoint, } bool inserted = false; - bool doEval = true; + bool doEval; std::pair ret; // Return of the insert() #ifdef _OPENMP omp_set_lock(&_cacheLock); diff --git a/src/Eval/BBOutput.cpp b/src/Eval/BBOutput.cpp index 26b12d59..f5fe5a93 100644 --- a/src/Eval/BBOutput.cpp +++ b/src/Eval/BBOutput.cpp @@ -98,14 +98,15 @@ NOMAD::Double NOMAD::BBOutput::getObjective(const NOMAD::BBOutputTypeList &bbOut NOMAD::ArrayOfString array(_rawBBO); NOMAD::Double obj; - checkSizeMatch(bbOutputType, array); - - for (size_t i = 0; i < array.size(); i++) + if (checkSizeMatch(bbOutputType)) { - if (NOMAD::BBOutputType::OBJ == bbOutputType[i]) + for (size_t i = 0; i < array.size(); i++) { - obj.atof(array[i]); - break; + if (NOMAD::BBOutputType::OBJ == bbOutputType[i]) + { + obj.atof(array[i]); + break; + } } } return obj; @@ -116,17 +117,18 @@ NOMAD::ArrayOfDouble NOMAD::BBOutput::getConstraints(const NOMAD::BBOutputTypeLi NOMAD::ArrayOfString array(_rawBBO); NOMAD::ArrayOfDouble constraints; - checkSizeMatch(bbOutputType, array); - - for (size_t i = 0; i < array.size(); i++) + if (checkSizeMatch(bbOutputType)) { - if ( NOMAD::BBOutputTypeIsConstraint(bbOutputType[i]) ) + for (size_t i = 0; i < array.size(); i++) { - NOMAD::Double d; - d.atof(array[i]); - size_t constrSize = constraints.size(); - constraints.resize(constrSize + 1); - constraints[constrSize] = d; + if ( NOMAD::BBOutputTypeIsConstraint(bbOutputType[i]) ) + { + NOMAD::Double d; + d.atof(array[i]); + size_t constrSize = constraints.size(); + constraints.resize(constrSize + 1); + constraints[constrSize] = d; + } } } @@ -151,10 +153,12 @@ NOMAD::ArrayOfDouble NOMAD::BBOutput::getBBOAsArrayOfDouble() const // Helper function. // Verify that the given output type list has the same size as the raw output. -// Throw an exception if this is not the case. -void NOMAD::BBOutput::checkSizeMatch(const NOMAD::BBOutputTypeList &bbOutputType, - const NOMAD::ArrayOfString &array) const +// Show an error, and return false, if this is not the case. +bool NOMAD::BBOutput::checkSizeMatch(const NOMAD::BBOutputTypeList &bbOutputType) const { + bool ret = true; + NOMAD::ArrayOfString array(_rawBBO); + if (bbOutputType.size() != array.size()) { std::string err = "Error: Parameter BB_OUTPUT_TYPE has " + NOMAD::itos(bbOutputType.size()); @@ -171,8 +175,11 @@ void NOMAD::BBOutput::checkSizeMatch(const NOMAD::BBOutputTypeList &bbOutputType } err += ":\n"; err += _rawBBO; - throw NOMAD::Exception(__FILE__, __LINE__, err); + std::cerr << err << std::endl; + ret = false; } + + return ret; } diff --git a/src/Eval/BBOutput.hpp b/src/Eval/BBOutput.hpp index 05b20b83..4e3eea35 100644 --- a/src/Eval/BBOutput.hpp +++ b/src/Eval/BBOutput.hpp @@ -150,17 +150,14 @@ class BBOutput { /// Display void display (std::ostream & out) const; -private: - /// Helper functions that can trigger exception. /** - Exception is triggered if the BBOutputList and - the ArrayOfString have inconsistent size. + Verify if the BBOutputList and the ArrayOfString have consistent size. + A warning is displayed if this is not the case. \param bbOutputType The list of blackbox output type -- \b IN. \param array The array of string -- \b IN. - + \return \c true if the sizes match */ - void checkSizeMatch(const BBOutputTypeList &bbOutputType, - const ArrayOfString &array) const; + bool checkSizeMatch(const BBOutputTypeList &bbOutputType) const; }; diff --git a/src/Eval/Eval.cpp b/src/Eval/Eval.cpp index 54a8985a..0841b5f7 100644 --- a/src/Eval/Eval.cpp +++ b/src/Eval/Eval.cpp @@ -89,12 +89,11 @@ NOMAD::Eval::Eval(std::shared_ptr params, auto bbOutputType = params->getAttributeValue("BB_OUTPUT_TYPE"); - // Note: if bbOutput is not eval_ok, then _f and _h end up undefined Doubles. _f = computeF(bbOutputType); // Set H - setH (_computeH(*this, bbOutputType) ); + setH (_computeH(*this, bbOutputType)); _toBeRecomputed = false; if (_bbOutput.getEvalOk() && _f.isDefined()) @@ -349,8 +348,15 @@ void NOMAD::Eval::setBBOutputAndRecompute(const NOMAD::BBOutput& bbOutput, const NOMAD::BBOutputTypeList& bbOutputType) { setBBOutput(bbOutput); - setF(computeF(bbOutputType)); - setH(_computeH(*this, bbOutputType)); + if (!bbOutput.checkSizeMatch(bbOutputType)) + { + _evalStatus = NOMAD::EvalStatusType::EVAL_ERROR; + } + else + { + setF(computeF(bbOutputType)); + setH(_computeH(*this, bbOutputType)); + } toRecompute(false); } diff --git a/src/Eval/EvalPoint.cpp b/src/Eval/EvalPoint.cpp index 3a1710a5..7bdded60 100644 --- a/src/Eval/EvalPoint.cpp +++ b/src/Eval/EvalPoint.cpp @@ -505,6 +505,7 @@ void NOMAD::EvalPoint::setBBO(const std::string &bbo, { switch (evalType) { + // Create new Eval case NOMAD::EvalType::SGTE: _evalSgte = NOMAD::EvalUPtr(new NOMAD::Eval()); break; @@ -516,7 +517,14 @@ void NOMAD::EvalPoint::setBBO(const std::string &bbo, eval = getEval(evalType); } - eval->setBBO(bbo, bboutputtypes, evalOk); + if (nullptr == eval) + { + throw NOMAD::Exception(__FILE__, __LINE__, "EvalPoint::setBBO: Could not create new Eval"); + } + else + { + eval->setBBO(bbo, bboutputtypes, evalOk); + } } @@ -551,7 +559,14 @@ void NOMAD::EvalPoint::setBBO(const NOMAD::BBOutput& bbo, } eval = getEval(evalType); } - eval->setBBOutput(bbo); + if (nullptr == eval) + { + throw NOMAD::Exception(__FILE__, __LINE__, "EvalPoint::setBBO: Could not create new Eval"); + } + else + { + eval->setBBOutput(bbo); + } } @@ -589,7 +604,14 @@ void NOMAD::EvalPoint::setEvalStatus(const NOMAD::EvalStatusType &evalStatus, eval = getEval(evalType); } - eval->setEvalStatus(evalStatus); + if (nullptr == eval) + { + throw NOMAD::Exception(__FILE__, __LINE__, "EvalPoint::setEvalStatus: Could not create new Eval"); + } + else + { + eval->setEvalStatus(evalStatus); + } } diff --git a/src/Math/Double.cpp b/src/Math/Double.cpp index a5ca4295..51724241 100644 --- a/src/Math/Double.cpp +++ b/src/Math/Double.cpp @@ -781,25 +781,29 @@ std::size_t NOMAD::Double::nbDecimals( ) const { std::size_t nbDec; - if ( _value < _epsilon ) + if (_value < _epsilon) { std::string str = "Error: nbDecimals of number smaller than EPSILON is not supported"; throw NOMAD::Exception(__FILE__, __LINE__, str); } - NOMAD::Double rem ( _value ); - int dec; - while ( true ) + NOMAD::Double rem( _value ); + int dec = std::floor(log10(rem.todouble())); + rem -= pow(10, dec); + while (rem._value >= _epsilon) { - dec = std::floor( log10 ( rem.todouble() ) ); - rem -= pow( 10, dec ) ; - if ( rem._value < _epsilon ) - break; + dec = std::floor(log10(rem.todouble())); + rem -= pow(10, dec); } - if ( dec > 0 ) + if (dec > 0) + { nbDec = 0; + } else - nbDec = -dec ; + { + nbDec = -dec; + } + return nbDec; }