plumed2 icon indicating copy to clipboard operation
plumed2 copied to clipboard

More helpful exception text for Registerbase

Open Iximiel opened this issue 1 year ago • 3 comments

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
  • [X] 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
  • [x] 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.

Iximiel avatar May 21 '24 13:05 Iximiel

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.

GiovanniBussi avatar May 21 '24 21:05 GiovanniBussi

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 avatar May 22 '24 06:05 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

Iximiel avatar May 22 '24 14:05 Iximiel