Modernization/keywords (#1183)

* Setting typo-safe types for keyword type and datatype for keywords self documenting addInputKeyword and mking the types public applied the example to dumpforces making the enum bitmask makes everithing compile :D reworking "add" reserve and use * removed some friend declaration to hide implementation details * made compontne in a struct, rempoved copyData * structured also the keyword informations * reorganization of some comments * Documenting better the bitmask enabler * adjusting the documentation * using algorithms for isActionSuffixed * adding constexpr to BitmaskEnum operations * added an example in Distance, modernized also Components declaration * restoring the flavour of removeOutputComponent with no existance checks * found some merge errors * Some small adjustments for reserveFlag * adding the erase/remove shortcut * grammar fix * compacted the argument_type into the key struct * compacted the add and reserve methods to simplify the life of future us * compacting also the atomtag into keyinfo * updating the documentation * Now plumed check should not hang anymore * checked that the json did not change and that cppcheck passes * removing removeComponet --------- Co-authored-by: Daniele Rapetti <[email protected]>
plumed · Feb 3, 2025 · 9d23b64 · 9d23b64 · PlumedBot · Feb 3, 2025
1 parent 9a14a0a
commit 9d23b64
Show file tree

Hide file tree

Showing 12 changed files with 1,077 additions and 613 deletions.
diff --git a/src/colvar/ColvarShortcut.h b/src/colvar/ColvarShortcut.h
@@ -38,10 +38,9 @@ template <class T>
 void ColvarShortcut<T>::registerKeywords(Keywords& keys ) {
   T::registerKeywords( keys );
   keys.remove("NO_ACTION_LOG");
-  unsigned nkeys = keys.size();
-  for(unsigned i=0; i<nkeys; ++i) {
-    if( keys.style( keys.get(i), "atoms" ) ) {
-      keys.reset_style( keys.get(i), "numbered" );
+  for (auto& key : keys.getKeys()) {
+    if( keys.style( key, "atoms" ) ) {
+      keys.reset_style( key, "numbered" );
     }
   }
   keys.addActionNameSuffix("_SCALAR");
@@ -53,7 +52,6 @@ ColvarShortcut<T>::ColvarShortcut(const ActionOptions&ao):
   Action(ao),
   ActionShortcut(ao) {
   bool scalar=true;
-  unsigned nkeys = keywords.size();
   if( getName()=="MASS" || getName()=="CHARGE" || getName()=="POSITION" ) {
     std::string inpt;
     parse("ATOMS",inpt);
@@ -62,12 +60,12 @@ ColvarShortcut<T>::ColvarShortcut(const ActionOptions&ao):
       scalar=false;
     }
   }
-  for(unsigned i=0; i<nkeys; ++i) {
-    if( keywords.style( keywords.get(i), "atoms" ) ) {
+  for (auto& key : keywords.getKeys()) {
+    if( keywords.style( key, "atoms" ) ) {
       std::string inpt;
-      parseNumbered( keywords.get(i), 1, inpt );
+      parseNumbered( key, 1, inpt );
       if( inpt.length()>0 ) {
-        readInputLine( getShortcutLabel() + ": " + getName() + "_VECTOR " + keywords.get(i) + "1=" + inpt + " " + convertInputLineToString() );
+        readInputLine( getShortcutLabel() + ": " + getName() + "_VECTOR " + key + "1=" + inpt + " " + convertInputLineToString() );
         scalar=false;
         break;
       }

diff --git a/src/colvar/Distance.cpp b/src/colvar/Distance.cpp
@@ -152,17 +152,18 @@ PLUMED_REGISTER_ACTION(DistanceMulti,"DISTANCE_VECTOR")
 void Distance::registerKeywords( Keywords& keys ) {
   Colvar::registerKeywords( keys );
   keys.setDisplayName("DISTANCE");
+  constexpr auto scalarOrVector = Keywords::componentType::scalar | Keywords::componentType::vector;
   keys.add("atoms","ATOMS","the pair of atom that we are calculating the distance between");
   keys.addFlag("COMPONENTS",false,"calculate the x, y and z components of the distance separately and store them as label.x, label.y and label.z");
   keys.addFlag("SCALED_COMPONENTS",false,"calculate the a, b and c scaled components of the distance separately and store them as label.a, label.b and label.c");
-  keys.addOutputComponent("x","COMPONENTS","scalar/vector","the x-component of the vector connecting the two atoms");
-  keys.addOutputComponent("y","COMPONENTS","scalar/vector","the y-component of the vector connecting the two atoms");
-  keys.addOutputComponent("z","COMPONENTS","scalar/vector","the z-component of the vector connecting the two atoms");
-  keys.addOutputComponent("a","SCALED_COMPONENTS","scalar/vector","the normalized projection on the first lattice vector of the vector connecting the two atoms");
-  keys.addOutputComponent("b","SCALED_COMPONENTS","scalar/vector","the normalized projection on the second lattice vector of the vector connecting the two atoms");
-  keys.addOutputComponent("c","SCALED_COMPONENTS","scalar/vector","the normalized projection on the third lattice vector of the vector connecting the two atoms");
+  keys.addOutputComponent("x","COMPONENTS",scalarOrVector,"the x-component of the vector connecting the two atoms");
+  keys.addOutputComponent("y","COMPONENTS",scalarOrVector,"the y-component of the vector connecting the two atoms");
+  keys.addOutputComponent("z","COMPONENTS",scalarOrVector,"the z-component of the vector connecting the two atoms");
+  keys.addOutputComponent("a","SCALED_COMPONENTS",scalarOrVector,"the normalized projection on the first lattice vector of the vector connecting the two atoms");
+  keys.addOutputComponent("b","SCALED_COMPONENTS",scalarOrVector,"the normalized projection on the second lattice vector of the vector connecting the two atoms");
+  keys.addOutputComponent("c","SCALED_COMPONENTS",scalarOrVector,"the normalized projection on the third lattice vector of the vector connecting the two atoms");
   keys.add("hidden","NO_ACTION_LOG","suppresses printing from action on the log");
-  keys.setValueDescription("scalar/vector","the DISTANCE between this pair of atoms");
+  keys.setValueDescription(scalarOrVector,"the DISTANCE between this pair of atoms");
 }
 
 Distance::Distance(const ActionOptions&ao):

diff --git a/src/colvar/MultiColvarTemplate.h b/src/colvar/MultiColvarTemplate.h
@@ -54,8 +54,8 @@ void MultiColvarTemplate<T>::registerKeywords(Keywords& keys ) {
   T::registerKeywords( keys );
   unsigned nkeys = keys.size();
   for(unsigned i=0; i<nkeys; ++i) {
-    if( keys.style( keys.get(i), "atoms" ) ) {
-      keys.reset_style( keys.get(i), "numbered" );
+    if( keys.style( keys.getKeyword(i), "atoms" ) ) {
+      keys.reset_style( keys.getKeyword(i), "numbered" );
     }
   }
   if( keys.outputComponentExists(".#!value") ) {

diff --git a/src/core/ActionRegister.cpp b/src/core/ActionRegister.cpp
@@ -43,7 +43,8 @@ std::unique_ptr<Action> ActionRegister::create(const std::vector<void*> & images
 
   auto content=get(images,ao.line[0]);
   Keywords keys;
-  keys.thisactname = ao.line[0];
+  //the name of the function is not clear but does `keys.thisactname = ao.line[0];`
+  keys.setDisplayName(ao.line[0]);
   content.keys(keys);
   ActionOptions nao( ao,keys );
   auto fullPath=getFullPath(images,ao.line[0]);
@@ -85,7 +86,8 @@ bool ActionRegister::printTemplate(const std::string& action, bool include_optio
   //no need to insert the try/catch block: check will ensure that action is known
   if( check(action) ) {
     Keywords keys;
-    keys.thisactname = action;
+    //the name of the function is not clear but does `keys.thisactname = action;`
+    keys.setDisplayName(action);
     get(action).keys(keys);
     keys.print_template(action, include_optional);
     return true;
@@ -110,7 +112,8 @@ ActionRegister::ID ActionRegister::add(std::string key,creator_pointer cp,keywor
 bool ActionRegister::getKeywords(const std::string& action, Keywords& keys) {
   //no need to insert the try/catch block: check will ensure that action is known
   if(check(action)) {
-    keys.thisactname = action;
+    //the name of the function is not clear but does `keys.thisactname = action;`
+    keys.setDisplayName(action);
     get(action).keys(keys);
     return true;
   }
@@ -119,7 +122,8 @@ bool ActionRegister::getKeywords(const std::string& action, Keywords& keys) {
 
 void ActionRegister::getKeywords(const std::vector<void*> & images, const std::string& action, Keywords& keys) {
   auto content=get(images,action);
-  keys.thisactname = action;
+  //the name of the function is not clear but does `keys.thisactname = action;`
+  keys.setDisplayName(action);
   content.keys(keys);
 }
 

diff --git a/src/core/ActionShortcut.cpp b/src/core/ActionShortcut.cpp
@@ -34,8 +34,8 @@ void ActionShortcut::registerKeywords( Keywords& keys ) {
 }
 
 void ActionShortcut::readShortcutKeywords( const Keywords& keys, std::map<std::string,std::string>& keymap ) {
-  for(unsigned i=0; i<keys.size(); ++i) {
-    std::string t, keyname = keys.get(i);
+  for (auto& keyname:keys.getKeys()) {
+    std::string t;
     if( keys.style( keyname, "optional") || keys.style( keyname, "compulsory") ) {
       parse(keyname,t);
       if( t.length()>0 ) {
@@ -80,15 +80,11 @@ void ActionShortcut::readInputLine( const std::string& input, bool saveline ) {
   std::vector<std::string> words=Tools::getWords(input);
   Tools::interpretLabel(words);
   // Check if this action name has been registered
-  bool founds=false, found = std::find(keywords.neededActions.begin(), keywords.neededActions.end(), words[0] )!=keywords.neededActions.end();
+  bool founds=false;
+  bool found = keywords.isActionNeeded(words[0]);
   // Check if we are just calling something like SUM_VECTOR using just SUM.
   if( !found && words[0].find(getName())!=std::string::npos ) {
-    for(unsigned j=0 ; j<keywords.actionNameSuffixes.size(); ++j) {
-      if( (getName() + keywords.actionNameSuffixes[j])==words[0] ) {
-        found=true;
-        break;
-      }
-    }
+    found =keywords.isActionSuffixed(words[0],getName());
     founds=true;
   }
   if( found ) {
@@ -121,17 +117,25 @@ void ActionShortcut::readInputLine( const std::string& input, bool saveline ) {
         std::string av_label = av->getLabel();
         if( av_label == getShortcutLabel() && av->getNumberOfComponents()==1 ) {
           savedOutputs.push_back( av_label );
-          plumed_massert( keywords.componentHasCorrectType(".#!value", (av->copyOutput(0))->getRank(), (av->copyOutput(0))->hasDerivatives() ), "documentation for type of value is incorrect");
+          plumed_massert( keywords.componentHasCorrectType(".#!value",
+                          (av->copyOutput(0))->getRank(), (av->copyOutput(0))->hasDerivatives() ),
+                          "documentation for type of value is incorrect");
         } else {
-          for(unsigned i=0; i<keywords.cnames.size(); ++i) {
-            if( av_label == getShortcutLabel() + "_" + keywords.cnames[i] ) {
+          for(const auto& cname: keywords.getOutputComponents()) {
+            if( av_label == getShortcutLabel() + "_" + cname ) {
               savedOutputs.push_back( av_label );
-              plumed_massert( keywords.componentHasCorrectType(keywords.cnames[i], (av->copyOutput(0))->getRank(), (av->copyOutput(0))->hasDerivatives() ), "documentation for type of component " + keywords.cnames[i] + " is incorrect");
-            } else if( keywords.getOutputComponentFlag(keywords.cnames[i])!="default" ) {
-              std::string thisflag = keywords.getOutputComponentFlag(keywords.cnames[i]);
-              if( keywords.numbered(thisflag) && av_label.find(getShortcutLabel() + "_" + keywords.cnames[i])!=std::string::npos ) {
+              plumed_massert( keywords.componentHasCorrectType(cname,
+                              (av->copyOutput(0))->getRank(),
+                              (av->copyOutput(0))->hasDerivatives() ),
+                              "documentation for type of component " + cname + " is incorrect");
+            } else if( keywords.getOutputComponentFlag(cname)!="default" ) {
+              std::string thisflag = keywords.getOutputComponentFlag(cname);
+              if( keywords.numbered(thisflag) && av_label.find(getShortcutLabel() + "_" + cname)!=std::string::npos ) {
                 savedOutputs.push_back( av_label );
-                plumed_massert( keywords.componentHasCorrectType(keywords.cnames[i], (av->copyOutput(0))->getRank(), (av->copyOutput(0))->hasDerivatives() ), "documentation for type of component " + keywords.cnames[i] + " is incorrect");
+                plumed_massert( keywords.componentHasCorrectType(cname,
+                                (av->copyOutput(0))->getRank(),
+                                (av->copyOutput(0))->hasDerivatives() ),
+                                "documentation for type of component " + cname + " is incorrect");
               }
             }
           }
@@ -186,9 +190,9 @@ void ActionShortcut::addToSavedInputLines( const std::string& line ) {
   Keywords thiskeys;
   actionRegister().getKeywords( actname, thiskeys );
   std::vector<std::string> numberedkeys;
-  for(unsigned i=0; i<thiskeys.size(); ++i ) {
-    if( thiskeys.numbered( thiskeys.getKeyword(i) ) ) {
-      numberedkeys.push_back( thiskeys.getKeyword(i) );
+  for (auto& keyname: thiskeys.getKeys()) {
+    if( thiskeys.numbered( keyname ) ) {
+      numberedkeys.push_back( keyname );
     }
   }
   if( numberedkeys.size()>0 && actname!="CONCATENATE" ) {

diff --git a/src/core/ActionWithArguments.cpp b/src/core/ActionWithArguments.cpp
@@ -49,7 +49,7 @@ void ActionWithArguments::registerKeywords(Keywords& keys) {
 
 void ActionWithArguments::parseArgumentList(const std::string&key,std::vector<Value*>&arg) {
   if( keywords.getArgumentType(key).length()==0 ) {
-    warning("keyword " + key + " for reading arguments should is registered using Keyword::add rather than Keyword::addInputKeyword.  The keyword will thus not appear in the correct place in the manual");
+    warning("keyword " + key + " for reading arguments is registered using Keyword::add rather than Keyword::addInputKeyword.  The keyword will thus not appear in the correct place in the manual");
   }
   std::string def;
   std::vector<std::string> c;
@@ -67,7 +67,7 @@ void ActionWithArguments::parseArgumentList(const std::string&key,std::vector<Va
 
 bool ActionWithArguments::parseArgumentList(const std::string&key,int i,std::vector<Value*>&arg) {
   if( keywords.getArgumentType(key).length()==0 ) {
-    warning("keyword " + key + " for reading argument should is registered using Keyword::add rather than Keyword::addInputKeyword.  The keyword will thus not appear in the correct place in the manual");
+    warning("keyword " + key + " for reading argument is registered using Keyword::add rather than Keyword::addInputKeyword.  The keyword will thus not appear in the correct place in the manual");
   }
   std::vector<std::string> c;
   arg.clear();

diff --git a/src/core/CLTool.cpp b/src/core/CLTool.cpp
@@ -78,7 +78,7 @@ bool CLTool::readCommandLineArgs( int argc, char**argv, FILE*out ) {
 
   // Set all flags to default false
   for(unsigned k=0; k<keywords.size(); ++k) {
-    thiskey=keywords.get(k);
+    thiskey=keywords.getKeyword(k);
     if( keywords.style(thiskey,"flag") ) {
       inputData.insert(std::pair<std::string,std::string>(thiskey,"false"));
     }
@@ -96,7 +96,7 @@ bool CLTool::readCommandLineArgs( int argc, char**argv, FILE*out ) {
     } else {
       bool found=false;
       for(unsigned k=0; k<keywords.size(); ++k) {
-        thiskey=keywords.get(k);
+        thiskey=keywords.getKeyword(k);
         if( keywords.style(thiskey,"flag") ) {
           if( a==thiskey ) {
             found=true;
@@ -147,7 +147,7 @@ bool CLTool::readCommandLineArgs( int argc, char**argv, FILE*out ) {
 void CLTool::setRemainingToDefault(FILE* out) {
   std::string def, thiskey;
   for(unsigned k=0; k<keywords.size(); ++k) {
-    thiskey=keywords.get(k);
+    thiskey=keywords.getKeyword(k);
     if( keywords.style(thiskey,"compulsory") ) {
       if( inputData.count(thiskey)==0 ) {
         if( keywords.getDefaultValue(thiskey,def) ) {
@@ -218,7 +218,7 @@ bool CLTool::readInputFile( int argc, char**argv, FILE* in, FILE*out ) {
     std::string keyword=buffer;
     bool found=false;
     for(unsigned i=0; i<keywords.size(); ++i) {
-      std::string thiskey=keywords.get(i);
+      std::string thiskey=keywords.getKeyword(i);
       if(thiskey==keyword) {
         found=true;
         std::size_t keypos=line.find_first_of(keyword)+keyword.length();

diff --git a/src/generic/DumpForces.cpp b/src/generic/DumpForces.cpp
@@ -71,7 +71,7 @@ void DumpForces::registerKeywords(Keywords& keys) {
   Action::registerKeywords(keys);
   ActionPilot::registerKeywords(keys);
   ActionWithArguments::registerKeywords(keys);
-  keys.addInputKeyword("compulsory","ARG","scalar","the labels of the values whose forces should be output");
+  keys.addInputKeyword("compulsory","ARG",Keywords::argType::scalar,"the labels of the values whose forces should be output");
   keys.add("compulsory","STRIDE","1","the frequency with which the forces should be output");
   keys.add("compulsory","FILE","the name of the file on which to output the forces");
   keys.add("compulsory","FMT","%15.10f","the format with which the derivatives should be output");

diff --git a/src/tools/BitmaskEnum.cpp b/src/tools/BitmaskEnum.cpp
@@ -0,0 +1,22 @@
+/* +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+   Copyright (c) 2024 The plumed team
+   (see the PEOPLE file at the root of the distribution for a list of names)
+
+   See http://www.plumed.org for more information.
+
+   This file is part of plumed, version 2.
+
+   plumed is free software: you can redistribute it and/or modify
+   it under the terms of the GNU Lesser General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   plumed is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public License
+along with plumed.  If not, see <http://www.gnu.org/licenses/>.
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ */
+#include "BitmaskEnum.h"