Less intrusive fix for #717

In the original fix I was making convert() throw exceptions.
However, we use a lot failed conversions when reading inputs.
Since catching an exception is much more expensive than inspecting
a returned bool, this was slowing down a lot some parts of the code

In 0ed75e9 I solved the issue
by allowing to throw only a few functions.

Here I take a radically different approach: a new function (convertNoexcept)
can be used like to old one, and returns a bool. The function
convert just calls convertNoexcept and, if false, throws.

In this manner there is no overhead expected at all with respect to the
previous version of the code.

In addition, the modifications are less intrusive. It is sufficient to
replace convert with convertNoexcept in all the places where the returned
bool was checked

regtest/basic/rt-make-3/main.cpp

@@ -26,27 +26,27 @@ int main(){
   in.close();
 
   int i=0;
-  plumed_assert(Tools::convert("2*3",i));
+  plumed_assert(Tools::convertNoexcept("2*3",i));
   plumed_assert(i==6);
 
   i=0;
-  plumed_assert(Tools::convert("(10+3)/13",i));
+  plumed_assert(Tools::convertNoexcept("(10+3)/13",i));
   plumed_assert(i==1);
 
   i=0;
-  plumed_assert(Tools::convert("exp(log(7))",i));
+  plumed_assert(Tools::convertNoexcept("exp(log(7))",i));
   plumed_assert(i==7);
 
   i=0;
-  plumed_assert(!Tools::convert("10.1",i));
+  plumed_assert(!Tools::convertNoexcept("10.1",i));
 
   long int l=0;
-  plumed_assert(Tools::convert("2397083434877565865",l));
+  plumed_assert(Tools::convertNoexcept("2397083434877565865",l));
   plumed_assert(l==2397083434877565865L);
 
   l=0;
   // this version is using lepton conversions and should fail:
-  plumed_assert(!Tools::convert("1*2397083434877565865",l));
+  plumed_assert(!Tools::convertNoexcept("1*2397083434877565865",l));
 
   return 0;
 }

regtest/basic/rt-make-exceptions/main.cpp

@@ -121,6 +121,7 @@ int main(){
   test_line(ofs,plumed,"COMBINE ARG=d,d1 COEFFICIENTS=3");
   test_line(ofs,plumed,"COMBINE ARG=d,d1 COEFFICIENTS=3,3 PARAMETERS=1");
   test_line(ofs,plumed,"COMBINE ARG=d,d1 COEFFICIENTS=3,3 PARAMETERS=1,2 POWERS=4");
+  test_line(ofs,plumed,"METAD ARG=d PACE=1 SIGMA=5 HEIGHT=1 GRID_MIN=bla GRID_MAX=100");
 
 // these should not fail
   plumed.cmd("readInputLine","m1: METAD ARG=d PACE=1 SIGMA=5 HEIGHT=1 FILE=H1 FMT=%9.5f");

regtest/basic/rt-make-exceptions/output.reference

@@ -74,6 +74,8 @@ readInputLine COMBINE ARG=d,d1 COEFFICIENTS=3,3 PARAMETERS=1
 +++ catched
 readInputLine COMBINE ARG=d,d1 COEFFICIENTS=3,3 PARAMETERS=1,2 POWERS=4
 +++ catched
+readInputLine METAD ARG=d PACE=1 SIGMA=5 HEIGHT=1 GRID_MIN=bla GRID_MAX=100
++++ catched
 something random here NULL
 +++ catched
 setStep NULL

src/core/Action.h

@@ -321,7 +321,7 @@ void Action::parse(const std::string&key,T&t) {
   // If it isn't read and it is compulsory see if a default value was specified
   if ( !found && (keywords.style(key,"compulsory") || keywords.style(key,"hidden")) ) {
     if( keywords.getDefaultValue(key,def) ) {
-      if( def.length()==0 || !Tools::convert(def,t) ) {
+      if( def.length()==0 || !Tools::convertNoexcept(def,t) ) {
         log.printf("ERROR in action %s with label %s : keyword %s has weird default value",name.c_str(),label.c_str(),key.c_str() );
         this->exit(1);
       }
@@ -371,7 +371,7 @@ void Action::parseVector(const std::string&key,std::vector<T>&t) {
   // If it isn't read and it is compulsory see if a default value was specified
   if ( !found && (keywords.style(key,"compulsory") || keywords.style(key,"hidden")) ) {
     if( keywords.getDefaultValue(key,def) ) {
-      if( def.length()==0 || !Tools::convert(def,val) ) {
+      if( def.length()==0 || !Tools::convertNoexcept(def,val) ) {
         log.printf("ERROR in action %s with label %s : keyword %s has weird default value",name.c_str(),label.c_str(),key.c_str() );
         this->exit(1);
       } else {

src/core/ActionAtomistic.cpp

@@ -168,7 +168,7 @@ void ActionAtomistic::interpretAtomList(std::vector<std::string>& strings, std::
   Tools::interpretRanges(strings); t.resize(0);
   for(unsigned i=0; i<strings.size(); ++i) {
     AtomNumber atom;
-    bool ok=Tools::convert(strings[i],atom); // this is converting strings to AtomNumbers
+    bool ok=Tools::convertNoexcept(strings[i],atom); // this is converting strings to AtomNumbers
     if(ok) t.push_back(atom);
 // here we check if this is a special symbol for MOLINFO
     if( !ok && strings[i].compare(0,1,"@")==0 ) {

src/core/CLTool.h

@@ -104,7 +104,7 @@ bool CLTool::parse(const std::string&key,T&t) {
   plumed_massert(keywords.exists(key),"keyword " + key + " has not been registered");
   if(keywords.style(key,"compulsory") ) {
     if(inputData.count(key)==0) error("missing data for keyword " + key);
-    bool check=Tools::convert(inputData[key],t);
+    bool check=Tools::convertNoexcept(inputData[key],t);
     if(!check) error("data input for keyword " + key + " has wrong type");
     return true;
   }
@@ -143,7 +143,7 @@ bool CLTool::parseVector(const std::string&key,std::vector<T>&t) {
   T val;
   if ( keywords.style(key,"compulsory") && t.size()==0 ) {
     if( keywords.getDefaultValue(key,def) ) {
-      if( def.length()==0 || !Tools::convert(def,val) ) {
+      if( def.length()==0 || !Tools::convertNoexcept(def,val) ) {
         plumed_merror("ERROR in keyword "+key+ " has weird default value" );
       } else {
         for(unsigned i=0; i<t.size(); ++i) t[i]=val;

src/core/PlumedMain.cpp

@@ -594,7 +594,7 @@ void PlumedMain::cmd(const std::string & word,const TypesafePtr & val) {
       {
         double v;
         plumed_assert(words.size()==2);
-        if(Tools::convert(words[1],v)) atoms.double2MD(v,val);
+        if(Tools::convertNoexcept(words[1],v)) atoms.double2MD(v,val);
       }
       break;
       default:

src/core/Value.cpp

@@ -107,9 +107,9 @@ void Value::setNotPeriodic() {
 
 void Value::setDomain(const std::string& pmin,const std::string& pmax) {
   str_min=pmin;
-  if( !Tools::convert(str_min,min) ) action->error("could not convert period string " + str_min + " to real");
+  if( !Tools::convertNoexcept(str_min,min) ) action->error("could not convert period string " + str_min + " to real");
   str_max=pmax;
-  if( !Tools::convert(str_max,max) ) action->error("could not convert period string " + str_max + " to read");
+  if( !Tools::convertNoexcept(str_max,max) ) action->error("could not convert period string " + str_max + " to read");
   setupPeriodicity();
 }
 

src/gridtools/GridVessel.cpp

@@ -92,8 +92,12 @@ void GridVessel::setBounds( const std::vector<std::string>& smin, const std::vec
   npoints=1; bounds_set=true;
   for(unsigned i=0; i<dimension; ++i) {
     str_min[i]=smin[i]; str_max[i]=smax[i];
-    Tools::convert( str_min[i], min[i] );
-    Tools::convert( str_max[i], max[i] );
+    // GB: I ignore the result here not to break test multicolvar/rt-dens-average
+    // where these functions were called with str_min[i] and str_max[i] as empty string
+    // To be checked.
+    Tools::convertNoexcept( str_min[i], min[i] );
+    Tools::convertNoexcept( str_max[i], max[i] );
+
     if( spacing.size()==dimension && binsin.size()==dimension ) {
       if( spacing[i]==0 ) nbin[i] = binsin[i];
       else {

src/opes/OPESmetad.cpp

@@ -342,7 +342,7 @@ OPESmetad<mode>::OPESmetad(const ActionOptions& ao)
   else
   {
     if(biasfactor_str.length()>0)
-      plumed_massert(Tools::convert(biasfactor_str,biasfactor_),error_in_input1+"BIASFACTOR"+error_in_input2);
+      plumed_massert(Tools::convertNoexcept(biasfactor_str,biasfactor_),error_in_input1+"BIASFACTOR"+error_in_input2);
     plumed_massert(biasfactor_>1,"BIASFACTOR must be greater than one (use 'inf' for uniform target)");
     bias_prefactor_=1-1./biasfactor_;
   }
@@ -359,7 +359,7 @@ OPESmetad<mode>::OPESmetad(const ActionOptions& ao)
   parseVector("SIGMA",sigma_str);
   sigma0_.resize(ncv_);
   double dummy;
-  if(sigma_str.size()==1 && !Tools::convert(sigma_str[0],dummy))
+  if(sigma_str.size()==1 && !Tools::convertNoexcept(sigma_str[0],dummy))
   {
     plumed_massert(sigma_str[0]=="ADAPTIVE" || sigma_str[0]=="adaptive",error_in_input1+"SIGMA"+error_in_input2);
     plumed_massert(!std::isinf(biasfactor_),"cannot use BIASFACTOR=inf with adaptive SIGMA");
@@ -377,7 +377,7 @@ OPESmetad<mode>::OPESmetad(const ActionOptions& ao)
     plumed_massert(adaptive_sigma_stride_==0,"if SIGMA is not ADAPTIVE you cannot set an ADAPTIVE_SIGMA_STRIDE");
     for(unsigned i=0; i<ncv_; i++)
     {
-      plumed_massert(Tools::convert(sigma_str[i],sigma0_[i]),error_in_input1+"SIGMA"+error_in_input2);
+      plumed_massert(Tools::convertNoexcept(sigma_str[i],sigma0_[i]),error_in_input1+"SIGMA"+error_in_input2);
       if(mode::explore)
         sigma0_[i]*=std::sqrt(biasfactor_); //the sigma of the target is broader F_t(s)=1/gamma*F(s)
     }

src/tools/PDB.cpp

@@ -338,7 +338,8 @@ bool PDB::readFromFilepointer(FILE *fp,bool naturalUnits,double scale) {
     }
     if(record=="ATOM" || record=="HETATM") {
       between_ters=true;
-      AtomNumber a; unsigned resno;
+      AtomNumber a;
+      unsigned resno=0; // GB: when resnum string is not present, we set res number to zero
       double o,b;
       Vector p;
       {
@@ -354,7 +355,13 @@ bool PDB::readFromFilepointer(FILE *fp,bool naturalUnits,double scale) {
         a.setSerial(result);
       }
 
-      Tools::convert(resnum,resno);
+      // allow skipping residue number
+      {
+        auto trimmed=resnum;
+        Tools::trim(trimmed);
+        if(trimmed.length()>0) Tools::convert(trimmed,resno);
+      }
+
       Tools::convert(occ,o);
       Tools::convert(bet,b);
       Tools::convert(x,p[0]);

src/tools/Tools.cpp

@@ -46,23 +46,23 @@ bool Tools::convertToAny(const std::string & str,T & t) {
   return remaining.length()==0;
 }
 
-bool Tools::convert(const std::string & str,int & t) {
+bool Tools::convertNoexcept(const std::string & str,int & t) {
   return convertToInt(str,t);
 }
 
-bool Tools::convert(const std::string & str,long int & t) {
+bool Tools::convertNoexcept(const std::string & str,long int & t) {
   return convertToInt(str,t);
 }
 
-bool Tools::convert(const std::string & str,unsigned & t) {
+bool Tools::convertNoexcept(const std::string & str,unsigned & t) {
   return convertToInt(str,t);
 }
 
-bool Tools::convert(const std::string & str,long unsigned & t) {
+bool Tools::convertNoexcept(const std::string & str,long unsigned & t) {
   return convertToInt(str,t);
 }
 
-bool Tools::convert(const std::string & str,AtomNumber &a) {
+bool Tools::convertNoexcept(const std::string & str,AtomNumber &a) {
   // Note: AtomNumber's are NOT converted as int, so as to
   // avoid using lepton conversions.
   unsigned i;
@@ -145,19 +145,19 @@ bool Tools::convertToReal(const std::string & str,T & t) {
   return false;
 }
 
-bool Tools::convert(const std::string & str,float & t) {
+bool Tools::convertNoexcept(const std::string & str,float & t) {
   return convertToReal(str,t);
 }
 
-bool Tools::convert(const std::string & str,double & t) {
+bool Tools::convertNoexcept(const std::string & str,double & t) {
   return convertToReal(str,t);
 }
 
-bool Tools::convert(const std::string & str,long double & t) {
+bool Tools::convertNoexcept(const std::string & str,long double & t) {
   return convertToReal(str,t);
 }
 
-bool Tools::convert(const std::string & str,std::string & t) {
+bool Tools::convertNoexcept(const std::string & str,std::string & t) {
   t=str;
   return true;
 }

src/tools/Tools.h

@@ -79,26 +79,27 @@ class Tools {
 /// compare two string in a case insensitive manner
   static bool caseInSensStringCompare(const std::string & str1, const std::string &str2);
 /// Convert a string to a double, reading it
-  static bool convert(const std::string & str,double & t);
+  static bool convertNoexcept(const std::string & str,double & t);
 /// Convert a string to a long double, reading it
-  static bool convert(const std::string & str,long double & t);
+  static bool convertNoexcept(const std::string & str,long double & t);
 /// Convert a string to a float, reading it
-  static bool convert(const std::string & str,float & t);
+  static bool convertNoexcept(const std::string & str,float & t);
 /// Convert a string to a int, reading it
-  static bool convert(const std::string & str,int & t);
+  static bool convertNoexcept(const std::string & str,int & t);
 /// Convert a string to a long int, reading it
-  static bool convert(const std::string & str,long int & t);
+  static bool convertNoexcept(const std::string & str,long int & t);
 /// Convert a string to an unsigned int, reading it
-  static bool convert(const std::string & str,unsigned & t);
+  static bool convertNoexcept(const std::string & str,unsigned & t);
 /// Convert a string to a long unsigned int, reading it
-  static bool convert(const std::string & str,long unsigned & t);
+  static bool convertNoexcept(const std::string & str,long unsigned & t);
 /// Convert a string to a atom number, reading it
-  static bool convert(const std::string & str,AtomNumber & t);
+  static bool convertNoexcept(const std::string & str,AtomNumber & t);
 /// Convert a string to a string (i.e. copy)
-  static bool convert(const std::string & str,std::string & t);
+  static bool convertNoexcept(const std::string & str,std::string & t);
 /// Convert anything into a string
   template<typename T>
-  static void convert(T i,std::string & str);
+  static bool convertNoexcept(T i,std::string & str);
+/// Convert anything into anything, throwing an exception in case there is an error
 /// Remove trailing blanks
   static void trim(std::string & s);
 /// Remove trailing comments
@@ -113,7 +114,11 @@ class Tools {
 /// will set s="xx"
   static bool getKey(std::vector<std::string>& line,const std::string & key,std::string & s,int rep=-1);
 /// Find a keyword on the input line, eventually deleting it, and saving its value to val
-  template <class T>
+  template <typename T,typename U>
+  static void convert(const T & t,U & u) {
+    plumed_assert(convertNoexcept(t,u)) <<"Error converting  "<<t;
+  }
+  template <typename T>
   static bool parse(std::vector<std::string>&line,const std::string&key,T&val,int rep=-1);
 /// Find a keyword on the input line, eventually deleting it, and saving its value to a vector
   template <class T>
@@ -221,7 +226,7 @@ template <class T>
 bool Tools::parse(std::vector<std::string>&line,const std::string&key,T&val,int rep) {
   std::string s;
   if(!getKey(line,key+"=",s,rep)) return false;
-  if(s.length()>0 && !convert(s,val))return false;
+  if(s.length()>0 && !convertNoexcept(s,val))return false;
   return true;
 }
 
@@ -241,7 +246,7 @@ bool Tools::parseVector(std::vector<std::string>&line,const std::string&key,std:
       plumed_assert(rep<static_cast<int>(words.size()));
       s=words[rep];
     }
-    if(!convert(s,v))return false;
+    if(!convertNoexcept(s,v))return false;
     val.push_back(v);
   }
   return true;
@@ -286,10 +291,11 @@ double Tools::pbc(double x) {
 }
 
 template<typename T>
-void Tools::convert(T i,std::string & str) {
+bool Tools::convertNoexcept(T i,std::string & str) {
   std::ostringstream ostr;
   ostr<<i;
   str=ostr.str();
+  return true;
 }
 
 inline

src/tools/Units.cpp

@@ -53,7 +53,7 @@ void Units::setEnergy(const std::string &s) {
   } else {
     energy=-1.0;
     energyString="";
-    if(!Tools::convert(s,energy)) {
+    if(!Tools::convertNoexcept(s,energy)) {
       plumed_merror("problem with setting the energy unit, either use give an numerical value or use one of the defined units: kj/mol, kcal/mol, j/mol, eV, Ha (case sensitive)");
     }
     plumed_massert(energy>0.0,"energy unit should be positive");
@@ -73,7 +73,7 @@ void Units::setLength(const std::string &s) {
   } else {
     length=-1.0;
     lengthString="";
-    if(!Tools::convert(s,length)) {
+    if(!Tools::convertNoexcept(s,length)) {
       plumed_merror("problem with setting the length unit, either use a numerical value or use one of the defined units: nm, A, um, Bohr (case sensitive)");
     }
     plumed_massert(length>0.0,"length unit should be positive");
@@ -93,7 +93,7 @@ void Units::setTime(const std::string &s) {
   } else {
     time=-1.0;
     timeString="";
-    if(!Tools::convert(s,time)) {
+    if(!Tools::convertNoexcept(s,time)) {
       plumed_merror("problem with setting the time unit, either use a numerical value or use one of the defined units: ps, fs, atomic (case sensitive)");
     }
     plumed_massert(time>0.0,"time unit should be positive");
@@ -107,7 +107,7 @@ void Units::setCharge(const std::string &s) {
   } else {
     charge=-1.0;
     chargeString="";
-    if(!Tools::convert(s,charge)) {
+    if(!Tools::convertNoexcept(s,charge)) {
       plumed_merror("problem with setting the charge unit, either use a numerical value or use one of the defined units: e (case sensitive)");
     }
     plumed_massert(charge>0.0,"charge unit should be positive");
@@ -121,7 +121,7 @@ void Units::setMass(const std::string &s) {
   } else {
     mass=-1.0;
     massString="";
-    if(!Tools::convert(s,mass)) {
+    if(!Tools::convertNoexcept(s,mass)) {
       plumed_merror("problem with setting the mass unit, either use a numerical value or use one of the defined units: amu (case sensitive)");
     }
     plumed_massert(mass>0.0,"mass unit should be positive");