More helpful exception text for Registerbase

[Iximiel]

Description

I was running some tests on some previous commit (where regtest/clusters/rt-dfg1/config had a wrong module prerequisites instead of plumed_modules="adjmat clusters") and the exception text +++ assertion failed: m.count(key)>0 felt not helpful.

I think that the text +++ assertion failed: m.count(key)>0, Missing key is "KEYNAME" feel a little more helpful.
I think these few lines may help at least in doing a git grep KEYNAME in src or a plumed manual KEYNAME to find at least which module has not been loaded.

---

An maybe we can use #1063 for giving directly that info, I do not know if directly in the register or also in an ad-hoc cltool or in cltools/Manual.cpp

---

Target release

I would like my code to appear in release 2.10

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 agree the error msg is terrible. I tried a stupid input file with AA on a single line.

With v2.9 the error is:

I cannot understand line: AA
Maybe a missing space or a typo?

which we deemed cryptic. Now it is

+++ assertion failed: m.count(key)>0

which I would argue is far worse.

Perhaps can be something like:

Action XXX is not known.

Plus, if XXX is in the ModuleMap:

An Action named XXX is available in module YYY. Please consider installing PLUMED with that module enabled.

[Iximiel]

ok, so the modifications could be

• set up a throw with the name of the key in RegisterBase
• and a try/catch block in the two specializations:
  • one with the action checks and the ModuleMap check
  • the other with an eventual cltool check (by also adding a CLmap?)
    I think that the flow control trough the try/catch block will not be a problem performance wise, since we are already hitting a wall before exiting

For the text I am going with your proposals

[Iximiel]

Now the output is:

PLUMED: ################################################################################
PLUMED: 
PLUMED: Action "CONTACT_MATRIX" is not known.
PLUMED: An Action named "CONTACT_MATRIX" is available in module "adjmat".
PLUMED: Please consider installing PLUMED with that module enabled.
PLUMED: ################################################################################

When the missing action is in the moduleMap

or

PLUMED: ################################################################################
PLUMED: 
PLUMED: Action "ASD" is not known.
PLUMED: ################################################################################

when the missing action is not in the moduleMap

---

I set up the exception also for the CLTool, for completion, since plumed notCL will exit already with a clear error message

[GiovanniBussi]

@Iximiel I think the standard way is to have a constructor with a std::string argument did you try and it didn't work?

[Iximiel]

I set up the additional member because the string ctor (Exception.h:222)

  explicit Exception(const std::string & msg):
    Exception()
  {
    *this << msg;
  }

calls operator << and that prepends a newline to the internal message if note is set to true, that is its default value, so .what() always starts with \n.
I think that manipulating the exception like this is clearer and more flexible like I did in ActionRegister.cpp:49:

try{
...
} catch (PLMD::ExceptionRegisterError &e ) {
  auto& actionName = e.getMissingKey();
  e <<"Action \"" << actionName << "\" is not known.";
  if (getModuleMap().count(actionName)>0) {
    e << "\nAn Action named \""
      <<actionName
      <<"\" is available in module \""
      << getModuleMap().at(actionName)
      << "\".\nPlease consider installing PLUMED with that module enabled.";
  }
}

In this way, I have this error about this "key", and I can react to the key in the catch block, separating the logic of getModuleMap() from the logic of the register.

I use throw ExceptionRegisterError().setMissingKey(key); three times, so moving the getModuleMapp() logic in the catch block prevents me from repeating the code.
This should make the code more flexible because the throws are in the RegisteBase and the exception could be thrown from an Action or CL register specialization.

Moreover, in this way, I do not need to remove the eventual stack trace that the ctor puts in msg to get the key

If you are ok with this, I think I need to add at least a docstring to explain how to use this for eventual new registers

[carlocamilloni]

@Iximiel @GiovanniBussi I think this is useful to merge, what is missing?

[Iximiel]

I need to add the docstring and then it is ready