Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes to documentation generators for PlumedToHTML #1081

Merged
merged 4 commits into from
May 22, 2024
Merged

Conversation

gtribello
Copy link
Member

Description

I have been doing some work on improving the PlumedToHTML code that generates the annotated input files for the tutorials and nest. The work is described in this PR. As part of this I have had to make some modifications to PLUMED. In particular, when you call PLUMED driver in parse only mode you can ask it to output two json dictionaries that contain:

  1. How any shortcut keywords are expanded into the longer input
  2. The definitions of the quantities that have labels in the input file

Getting the definitions of the labels directly from plumed when you run driver is superior to getting this information from the syntax file as:

  1. You are able to distinguish whether objects are scalars, vectors, matrices etc this way.
  2. You can find what the labels for actions that have custom labels (i.e. labels that depend on the arguments).
  3. This way you can get the meanings of components that are created by files that are created by LOAD actions.

I tested the new version of PlumedTo HTML on all the inputs in the plumed nest and found a small number places where the new code does not work. This PR fixes those problems.

@GiovanniBussi can you please check whether I have done the code for the first commit correctly? The problem here is that I need to call registerKeywords for all the actions defined in the input file. This will not work if you do:

actionRegister().getKeywords( name, keys )

The problem with that is that the code for the actions in LOAD is not accessible that way. I thus came up with a way of doing by looking at what you did for creating actions. I hope this is correct. The tests that were not working now run.

Also note that I found one input in the nest like this:

d1.mydist: DISTANCE ATOMS=1,2
PRINT ARG=* 

I managed to get it working but I also put a warning in about putting a . in the names of actions. I think this is a recipe for trouble. Not sure if we should make that an error. What do you think?

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.

Gareth Aneurin Tribello added 4 commits May 22, 2024 11:16
…keywords in a way that will work with LOAD
…ictionary in driver so that you can use action labels that have dots in them

Also added a warning to try to stop people from using dots in the labels for actions as it is not a great idea
@gtribello gtribello requested a review from GiovanniBussi May 22, 2024 12:28
@GiovanniBussi
Copy link
Member

GiovanniBussi commented May 22, 2024

I hope this is correct. The tests that were not working now run.

Great, it means that the code was not too bad :-). I should probably replace the std::vector<void*> with some custom class std::vector<DLimage> or so.

I managed to get it working but I also put a warning in about putting a . in the names of actions. I think this is a recipe for trouble. Not sure if we should make that an error. What do you think?

I would actually forbid this. Or we might start phasing it out and give a warning now (and a failure in plumed 2.11)

@gtribello gtribello merged commit f9b0bd9 into master May 22, 2024
40 checks passed
@gtribello gtribello deleted the small-doc-fixes branch May 22, 2024 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants