Should we convert most parse_silent options to parse_echo ?

[jhenin]

Most often those are advanced options that were silent to make them less verbose, but now that parse_echo is available it seems appropriate in a lot of places.

Notable exception is parsing restarts.

Edit: More precisely, I propose changing all the somewhat specialized keywords for which the advice is "leave it as default unless you know what you are doing" to parse_echo. The hope is that more users will be able to make sense of the Colvars output.

To follow the completion of this in various classes:

• colvarmodule
• atom_group
• colvarcomp
• colvarcomp_*
• colvar
• colvarbias
• colvarbias_abf
• colvarbias_restraint
• colvarbias_abf
• colvarbias_metadynamics

[giacomofiorin]

That sounds okay to me. I just didn't convert all those cases after introducing the bitwise flags in Parse_Mode mostly to minimize risks to backward compatibility (read: inertia, actually).

This and other changes can be done incrementally, of course.

Longer term, get_keyval() should be replaced by a function that is very similar but has the additional features:

1. It handles colvardeps features without requiring a separate function.
2. It is called inside the init() function, and lets the constructor instead set the default value of the keyword (enabling the use of code analyzers that right now mostly nag endlessly about the calls to get_keyval() with an un-initialized argument). Because of this, I think that the fourth and fifth arguments (both optional) of get_keyval() could be inverted in the new version.
3. Thanks to (2), the init() function of a class can be fully idempotent.
4. logLevel is honored and allows truly quiet parsing if needed; this would be most useful for the previous feature, but can be implemented achieved at any time independently, even with the current get_keyval().
5. When applicable, register the keyword being parsed (or left at its default) as an entry in colvarparams, so that objects communicating with each other can do so without added code.
6. (Edited) Specify typical range checks via Parse_Mode bitwise flags, eliminating the need for a lot of duplicated code.

Key to these (but 3 and 5 especially) is a more versatile scripting interface, so I didn't want to put them in their own long-term issue so far only to ignore it for a long time, or revise it heavily (plenty of examples of each). But any part that's incremental towards these goals, I'm very happy with.

[jhenin]

To make this issue workable over a finite time, I am rephrasing it to something more focused.

[jhenin]

A different, less conservative perspective: we could make parse_echo the default behavior, and explicitly add the parse_default flag to parameters that we think should always be in the output. Thoughts @giacomofiorin ?

[giacomofiorin]

I believe a more pressing need is finishing up the support for logLevel, first introduced in 5e71715. All calls to get_keyval() handle that flag already. But there are plenty of other messages being printed via cvm::log(...) that are part of the parsing, and are not tied to an appropriate value of logLevel.

What you are suggesting is fine-tuning the behavior for some keywords based on their importance. I'm fine with it, but before putting in any work I'd need you to either agree with or revise/improve the following:

1. The list of values of logLevel, currently implemented as simple inline functions e.g. cvm::log_user_params(). Note that the keyword logLevel is not advertised to users precisely because we don't have yet an agreed-upon list of values for it. Once we do, let's tie the calls to cvm::log(...) to the appropriate value and then advertise the flag. The default value is the highest level, i.e. once you define logLevel in a script to a value low enough to be useful, even very important messages will be missed already: this needs to be fixed first. Of course, there is also the option of throwing logLevel away altogether, but you need to first put forward your argument.
2. The specifics of the improved version of get_keyval(), so that each time one of its instances is replaced by the improved one, it stays that way without further changes. I put all that I could think of in this earlier comment Should we convert most parse_silent options to parse_echo ? #331 (comment) but you have not commented on it yet. A possible way that this could get easier is to have the function accept an optional data structure as argument, so that we can have something as close as possible to named arguments (won't be a C++ feature that we can use anytime soon).