Modernization/keywords

[Iximiel]

Description

I slightly compacted Keywords, and I think it makes easier maintain the keywords:

Before, for the keywords you had an hashmap for the type, one for the docstring, one for the allowmultiple option, one for the argument type, one for the atom tag specialization,one for the default value and one for the default values for the flags.

Now all of these have been compacted in a single hashmap that contain a struct with all the necessary info, that I think makes far more easier to work with this and maintain it.
Same thing has been done for the components.
This should also occupy less space, since we do not have to store all the N sets of keys but only one (plus an extra vector for keeping the order of declaration and ease the reserved/declared keyword logic).

Now adding a new functionality for the keywords or the components will mean not book-keep another hashmap (and update the signature of copyData) around the various function that add/remove components and keywords, but simply updating the existing struct and managing the presence of that functionality where it is needed

Some other things done here:

• I removed the get() method, since was a double of getKeyword() and the latter was more self-documenting.

• I removed the friend declarations, most of them where not needed since Keywords has alredy the needed getters method, and in other cases like isActionSuffixed() and isActionNeeded() that logic should be inside Keywords and not in other places.

  • About that, is it ok to use void setDisplayName( const std::string& name ); in the ActioRegister as I did? That function does exactly what the friend access did before.

• See how void copyData( std::vector<std::string>& kk, std::vector<std::string>& rk, std::map<std::string,KeyType>& tt, std::map<std::string,bool>& am, std::map<std::string,std::string>& docs, std::map<std::string,bool>& bools, std::map<std::string,std::string>& nums, std::map<std::string,std::string>& atags, std::vector<std::string>& cnam, std::map<std::string,std::string>& ck, std::map<std::string,std::string>& cd ) const ; has been compacted into void add( const Keywords& keys );

• Apart from the few now no more friend files, I also modified two files as an example: DumpForces.cpp and Distance.cpp with an example of the usage of the enums outside in the code, In my opinion this can avoid errors due to typos, so that the compiler will help.

---

removeOutputComponent and removeComponent do circa the same thing in the original, both remove the name of the component, but removeComponent removes also the component related keyword and checks for existence of the queried component, both did not remove the component type. Is this a undocumented wanted behavior?
(Also I do not understand why the all the searches here were embedded in while(true){} loops)

Using the diff interface the difference between the old version of the functions is:

--- removeComponent
+++ removeOutputComponent
-void Keywords::removeComponent( const std::string& name ) {
+void Keywords::removeOutputComponent( const std::string& name ) {
-  bool found=false;
+  unsigned j=0;
   while(true) {
-    unsigned j;
     for(j=0; j<cnames.size(); j++) if(cnames[j]==name)break;
     if(j<cnames.size()) {
       cnames.erase(cnames.begin()+j);
-      found=true;
     } else break;
   }
-  // Delete documentation, type and so on from the description
   cdocs.erase(name);
-  ckey.erase(name);
-  plumed_massert(found,"You are trying to remove " + name + " a component that isn't there");
 }

I maintained the throw/not throw behavior. But all the component-related data is removed in both cases.

---

On a secondary note I set up (taking a lot of "inspiration" here and there) also a way of using enums as named bitmask without getting too much crazy. I think this can also be useful elsewhere

I also tried to move something to string_view, but I found out that is not always a good idea.

---

Note that I did not changed the fix i proposed in #1168 is not applied here

Target release

I would like my code to appear in release 2.11, but maybe I can back port a version into v2.10 it needs some modifications

Type of contribution

• changes to code or doc authored by PLUMED developers, or additions of code in the core or within the default modules
• changes to a module not authored by you
• new module contribution or edit of a module authored by you

Copyright

• I agree to transfer the copyright of the code I have written to the PLUMED developers or to the author of the code I am modifying.

• the module I added or modified contains a COPYRIGHT file with the correct license information. Code should be released under an open source license. I also used the command cd src && ./header.sh mymodulename in order to make sure the headers of the module are correct.

Tests

• I added a new regtest or modified an existing regtest to validate my changes.
• I verified that all regtests are passed successfully on GitHub Actions.

[GiovanniBussi]

I noticed that this changes the way components are registered and I am wondering if there's an impact on LOADed source files.

Is the "old" way to specify the type of component still working after this change?

[Iximiel]

I noticed that this changes the way components are registered and I am wondering if there's an impact on LOADed source files.

Is the "old" way to specify the type of component still working after this change?

I left the old functions that accepts strings, on that point the API is not changed but grown with the function that get the enums. I only changed two files to give an idea on how it is possible to do that

I think I also maintained the same order of the old version when querying components and kws.

Is that what you were asking?

---

A thing that is changed behind the curtains is

reserve(...)
...
    if( t=="atoms" && isaction ) {
      fd = d + ".  For more information on how to specify lists of atoms see \\ref Group";
    }

that now tests for (.isAtomlist() && isaction)
because in the equivalent add() of the old version that passage is checked with .isAtomlist() to get "atoms" "atom" and "residues". But this only impacts the manual