Skip to content

Commit

Permalink
Make sure incorrect conversions trigger an exception
Browse files Browse the repository at this point in the history 
See #717
  • Loading branch information
@GiovanniBussi
GiovanniBussi committed Jul 19, 2021
1 parent f4e420c commit 8f0d062
Show file tree
Hide file tree
Showing 18 changed files with 266 additions and 112 deletions.
22 changes: 15 additions & 7 deletions regtest/basic/rt-make-3/main.cpp
View file
Expand Up @@ -26,27 +26,35 @@ int main(){
in.close();

int i=0;
plumed_assert(Tools::convert("2*3",i));
Tools::convert("2*3",i);
plumed_assert(i==6);

i=0;
plumed_assert(Tools::convert("(10+3)/13",i));
Tools::convert("(10+3)/13",i);
plumed_assert(i==1);

i=0;
plumed_assert(Tools::convert("exp(log(7))",i));
Tools::convert("exp(log(7))",i);
plumed_assert(i==7);

i=0;
plumed_assert(!Tools::convert("10.1",i));
try {
Tools::convert("10.1",i);
plumed_error();
} catch(ExceptionConversionError& exc) {
}

long int l=0;
plumed_assert(Tools::convert("2397083434877565865",l));
Tools::convert("2397083434877565865",l);
plumed_assert(l==2397083434877565865L);

l=0;
// this version is using lepton conversions and should fail:
plumed_assert(!Tools::convert("1*2397083434877565865",l));

try {
Tools::convert("1*2397083434877565865",l);
plumed_error();
} catch(ExceptionConversionError& exc) {
}

return 0;
}
1 change: 1 addition & 0 deletions regtest/basic/rt-make-exceptions/main.cpp
View file
Expand Up @@ -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");
Expand Down
2 changes: 2 additions & 0 deletions regtest/basic/rt-make-exceptions/output.reference
View file
Expand Up @@ -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
Expand Down
21 changes: 16 additions & 5 deletions src/core/Action.h
View file
Expand Up @@ -321,7 +321,13 @@ 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) {
log.printf("ERROR in action %s with label %s : keyword %s has empty default value",name.c_str(),label.c_str(),key.c_str() );
this->exit(1);
}
try {
Tools::convert(def,t);
} catch(ExceptionConversionError& exc) {
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);
}
Expand Down Expand Up @@ -371,13 +377,18 @@ 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) {
log.printf("ERROR in action %s with label %s : keyword %s has empty default value",name.c_str(),label.c_str(),key.c_str() );
this->exit(1);
}
try {
Tools::convert(def,val);
} catch(ExceptionConversionError& exc) {
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 {
if(t.size()>0) for(unsigned i=0; i<t.size(); ++i) t[i]=val;
else t.push_back(val);
}
if(t.size()>0) for(unsigned i=0; i<t.size(); ++i) t[i]=val;
else t.push_back(val);
} else if( keywords.style(key,"compulsory") ) {
error("keyword " + key + " is compulsory for this action");
}
Expand Down
7 changes: 6 additions & 1 deletion src/core/ActionAtomistic.cpp
View file
Expand Up @@ -168,7 +168,12 @@ 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=false;
try {
Tools::convert(strings[i],atom); // this is converting strings to AtomNumbers
ok=true;
} catch(ExceptionConversionError& exc) {
}
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 ) {
Expand Down
17 changes: 13 additions & 4 deletions src/core/CLTool.h
View file
Expand Up @@ -104,7 +104,12 @@ 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=false;
try {
Tools::convert(inputData[key],t);
check=true;
} catch(ExceptionConversionError& exc) {
}
if(!check) error("data input for keyword " + key + " has wrong type");
return true;
}
Expand Down Expand Up @@ -143,11 +148,15 @@ 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) {
plumed_merror("ERROR in keyword "+key+ " has empty default value" );
}
try {
Tools::convert(def,val);
} catch(ExceptionConversionError& exc) {
plumed_merror("ERROR in keyword "+key+ " has weird default value" );
} else {
for(unsigned i=0; i<t.size(); ++i) t[i]=val;
}
for(unsigned i=0; i<t.size(); ++i) t[i]=val;
} else {
plumed_merror("keyword " + key + " is compulsory for this action");
}
Expand Down
12 changes: 7 additions & 5 deletions src/core/PlumedMain.cpp
View file
Expand Up @@ -591,12 +591,14 @@ void PlumedMain::cmd(const std::string & word,const TypesafePtr & val) {
break;
/* ADDED WITH API==7 */
case cmd_convert:
{
double v;
plumed_assert(words.size()==2);
if(Tools::convert(words[1],v)) atoms.double2MD(v,val);
}
break;
try {
double v;
Tools::convert(words[1],v);
atoms.double2MD(v,val);
} catch(ExceptionConversionError& exc) {
}
break;
default:
plumed_merror("cannot interpret cmd(\"" + word + "\"). check plumed developers manual to see the available commands.");
break;
Expand Down
12 changes: 10 additions & 2 deletions src/core/Value.cpp
View file
Expand Up @@ -107,9 +107,17 @@ 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");
try {
Tools::convert(str_min,min);
} catch(ExceptionConversionError& exc) {
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");
try {
Tools::convert(str_max,max);
} catch(ExceptionConversionError& exc) {
action->error("could not convert period string " + str_max + " to real");
}
setupPeriodicity();
}

Expand Down
16 changes: 14 additions & 2 deletions src/gridtools/GridVessel.cpp
View file
Expand Up @@ -92,8 +92,20 @@ 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] );
try {
Tools::convert( str_min[i], min[i] );
} catch(ExceptionConversionError& exc) {
// GB: I allow this failure 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.
}
try {
Tools::convert( str_max[i], max[i] );
} catch(ExceptionConversionError& exc) {
// GB: I allow this failure 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.
}
if( spacing.size()==dimension && binsin.size()==dimension ) {
if( spacing[i]==0 ) nbin[i] = binsin[i];
else {
Expand Down
20 changes: 17 additions & 3 deletions src/opes/OPESmetad.cpp
View file
Expand Up @@ -342,7 +342,11 @@ 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);
try {
Tools::convert(biasfactor_str,biasfactor_);
} catch(ExceptionConversionError& exc) {
plumed_error() << 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_;
}
Expand All @@ -359,7 +363,13 @@ 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))
bool ok=false;
if(sigma_str.size()==1) try {
Tools::convert(sigma_str[0],dummy);
} catch(ExceptionConversionError& exc) {
ok=true;
}
if(ok)
{
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");
Expand All @@ -377,7 +387,11 @@ 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);
try {
Tools::convert(sigma_str[i],sigma0_[i]);
} catch(ExceptionConversionError& exc) {
plumed_error()<<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)
}
Expand Down
12 changes: 12 additions & 0 deletions src/tools/Exception.h
View file
Expand Up @@ -292,6 +292,18 @@ class ExceptionTypeError :
}
};

/// Class representing a conversion error
class ExceptionConversionError :
public Exception {
public:
using Exception::Exception;
template<typename T>
ExceptionConversionError& operator<<(const T & x) {
*((Exception*) this) <<x;
return *this;
}
};

#ifdef __GNUG__
// With GNU compiler, we can use __PRETTY_FUNCTION__ to get the function name
#define __PLUMED_FUNCNAME __PRETTY_FUNCTION__
Expand Down
9 changes: 7 additions & 2 deletions src/tools/PDB.cpp
View file
Expand Up @@ -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;
{
Expand All @@ -354,7 +355,11 @@ bool PDB::readFromFilepointer(FILE *fp,bool naturalUnits,double scale) {
a.setSerial(result);
}

Tools::convert(resnum,resno);
{
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]);
Expand Down

1 comment on commit 8f0d062

@PlumedBot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found broken examples in automatic/performance-optimization.txt
Found broken examples in automatic/a-trieste-6.txt
Found broken examples in automatic/munster.txt
Found broken examples in automatic/ANN.tmp
Found broken examples in automatic/EDS.tmp
Found broken examples in automatic/EMMI.tmp
Found broken examples in automatic/FOURIER_TRANSFORM.tmp
Found broken examples in automatic/FUNCPATHGENERAL.tmp
Found broken examples in automatic/FUNCPATHMSD.tmp
Found broken examples in automatic/FUNNEL.tmp
Found broken examples in automatic/FUNNEL_PS.tmp
Found broken examples in automatic/GHBFIX.tmp
Found broken examples in automatic/INCLUDE.tmp
Found broken examples in automatic/MAZE_MEMETIC_SAMPLING.tmp
Found broken examples in automatic/MAZE_OPTIMIZER_BIAS.tmp
Found broken examples in automatic/MAZE_RANDOM_ACCELERATION_MD.tmp
Found broken examples in automatic/MAZE_RANDOM_WALK.tmp
Found broken examples in automatic/MAZE_SIMULATED_ANNEALING.tmp
Found broken examples in automatic/MAZE_STEERED_MD.tmp
Found broken examples in automatic/PIV.tmp
Found broken examples in automatic/PLUMED.tmp
Found broken examples in automatic/VES_DELTA_F.tmp
Found broken examples in MiscelaneousPP.md

Please sign in to comment.