plumed / plumed2

Development version of plumed 2
https://www.plumed.org
GNU Lesser General Public License v3.0
357 stars 283 forks source link

Refactor manual to better include contributed modules #253

Closed GiovanniBussi closed 7 years ago

GiovanniBussi commented 7 years ago

I open an issue related to manual refactoring so that we can public comment on it.

The goal is to end up with a manual where there is a sub menu for each contributed module including:

The plan is to incorporate many ideas from how @valsson did the manual for his VES code (manual here).

In particular, we might define some rule such that:

carlocamilloni commented 7 years ago

I think i like the idea of the self containing manual for the modules at the same time I think the best would be to have everything accessible from both places e.g a Pippo_bias will also automatically appear inside the general biases list as well as in the Pippo module pages

GiovanniBussi commented 7 years ago

I think so. Just a question: in case Pippo_bias is of type bias, you put the \subpage in the module page and the \ref in the bias page, right? I think this would be the correct way.

Finally, maybe one could replace this:

keywords=`grep "^ *@" *.md | awk -F"@" '{print $2}' | uniq`

with this:

grep -o "@[A-Z_]*@" *.md | awk -F"@" '{print $2}'  | uniq

just to avoid troubles if someone puts the keyword @COLVAR@ not at the beginning of the line.

carlocamilloni commented 7 years ago

At the moment I am using subpage for both, with \ref it does not appear in the list otherwise (or more probably I don’t know how to do it) Anyway in the index of action it is related to the one in the module.

This is how it looks at the moment: https://plumed.github.io/doc-isdb/user-doc/html/index.html https://plumed.github.io/doc-isdb/user-doc/html/index.html

While I have replaced the grep with yours.

On 19 Jul 2017, at 18:06, Giovanni notifications@github.com wrote:

I think so. Just a question: in case Pippo_bias is of type bias, you put the \subpage in the module page and the \ref in the bias page, right? I think this would be the correct way.

Finally, maybe one could replace this:

keywords=grep "^ *@" *.md | awk -F"@" '{print $2}' | uniq with this:

grep -o "@[A-Z_]@" .md | awk -F"@" '{print $2}' | uniq just to avoid troubles if someone puts the keyword @COLVAR@ not at the beginning of the line.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/plumed/plumed2/issues/253#issuecomment-316435789, or mute the thread https://github.com/notifications/unsubscribe-auth/AEXl9h0Z7gjxRrMgxgYmi5HRhmUtsm1lks5sPilogaJpZM4OH7G4.

GiovanniBussi commented 7 years ago

I see, maybe Doxygen is clever enough to create the page only once...

Looking at the manual I am not completely sure that it makes sense to have actions repeated (in the left menu). To me the confusing thing is indeed that when you expand the tree on the lext you see the same action twice. Notice that for instance in Index of actions they do not expand (this is just a table with links).

Maybe an alternative could be to add e.g. at the end of the bias page a sentence such as

Notice that other biases are available in the following modules:
- PLUMED-ISDB

Here PLUMED-ISDB could have a link directly to isdb-bias

Could this make sense?

carlocamilloni commented 7 years ago

I would prefer as it now because a bias is a bias irrespectively of the module where it is implemented. Furthermore as it is now it requires less work, essentially the only step that is not automatic is to add ISDBPP.md in the go-doxygen.

On 19 Jul 2017, at 18:29, Giovanni notifications@github.com wrote:

I see, maybe Doxygen is clever enough to create the page only once...

Looking at the manual I am not completely sure that it makes sense to have actions repeated (in the left menu). To me the confusing thing is indeed that when you expand the tree on the lext you see the same action twice. Notice that for instance in Index of actions they do not expand (this is just a table with links).

Maybe an alternative could be to add e.g. at the end of the bias https://plumed.github.io/doc-isdb/user-doc/html/_bias.html page a sentence such as

Notice that other biases are available in the following modules:

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/plumed/plumed2/issues/253#issuecomment-316442588, or mute the thread https://github.com/notifications/unsubscribe-auth/AEXl9qeCd7oeU75ThlO0w8JT7LMKWvRdks5sPi8EgaJpZM4OH7G4.

GiovanniBussi commented 7 years ago

Yes this I understand, still to me it is confusing that they appear twice...

I wanted to check the pdf manual (to see if variables appear twice) but I now realize that in this manual the section @PDFMANUAL@ is missing. Is this on purpose (i.e. you disabled it) or maybe there is some problem in the script?

carlocamilloni commented 7 years ago

I should have fixed it now

On 19 Jul 2017, at 18:45, Giovanni notifications@github.com wrote:

Yes this I understand, still to me it is confusing that they appear twice...

I wanted to check the pdf manual (to see if variables appear twice) but I now realize that in this manual https://plumed.github.io/doc-isdb/user-doc/html/index.html the section @PDFMANUAL@ is missing. Is this on purpose (i.e. you disabled it) or maybe there is some problem in the script?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/plumed/plumed2/issues/253#issuecomment-316446866, or mute the thread https://github.com/notifications/unsubscribe-auth/AEXl9lBi4HZjzNSiw_etMw5MQb3YslErks5sPjK5gaJpZM4OH7G4.

carlocamilloni commented 7 years ago

I am also fixing for VERSION, this should be the last one

On 19 Jul 2017, at 18:52, Carlo Camilloni carlo.camilloni@gmail.com wrote:

I should have fixed it now

On 19 Jul 2017, at 18:45, Giovanni <notifications@github.com mailto:notifications@github.com> wrote:

Yes this I understand, still to me it is confusing that they appear twice...

I wanted to check the pdf manual (to see if variables appear twice) but I now realize that in this manual https://plumed.github.io/doc-isdb/user-doc/html/index.html the section @PDFMANUAL@ is missing. Is this on purpose (i.e. you disabled it) or maybe there is some problem in the script?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/plumed/plumed2/issues/253#issuecomment-316446866, or mute the thread https://github.com/notifications/unsubscribe-auth/AEXl9lBi4HZjzNSiw_etMw5MQb3YslErks5sPjK5gaJpZM4OH7G4.

GiovanniBussi commented 7 years ago

I see, actually the extract script is not what I would call a user-friendly script... Perhaps after we have something that works and does what we want it could be simplified a bit.

carlocamilloni commented 7 years ago

I agree

valsson commented 7 years ago

I tried the modified extract script by Carlo on the manual of my code and it seems to work exactly as it should.

I would agree that the extract script is not very user-friendly, nor is it easy to understand how it work.

I noticed one issue, if I have a command line tools that is in the category VES_TOOLS it goes correctly into both sections of the manual (VES_TOOLS and TOOLS). However, there is a plumedcheck failure:

[global_check] (error) :plmd_unregistered_action: action ves_md_linearexpansion at ves/MD_LinearExpansionPES.cpp:53 is not registered [global_check] (error) :plmd_undocumented_cltool: cltool ves_md_linearexpansion at ves/MD_LinearExpansionPES.cpp:80 is not documented

So I guess the plumedcheck script has to be modified to reflect that changes in the manual construction.

GiovanniBussi commented 7 years ago

@valsson ok that should be easy to fix.

@carlocamilloni can you transplant the script on the v2.4 branch? I don't know if you plan to merge the whole isdb branch now. In case not, you can just copy the new script. (notice though that there is still some issue with the description of the PDF manual).

I reiterate that I am not fully satisfied with the duplication of the keywords and I would like to find a solution where in one of the two places there are just links, but I agree that this is a good starting point. I can try to refine the script later.

As a reminder, if anyone (@plumed/all) wants to play with manual generating it on Travis-CI, you can just force push on testdoc branch so that it ends up here.

carlocamilloni commented 7 years ago

PDFmanual is now fixed, yes I can copy the extract script to v2.4. Max and I want to merge the isdb branch before the beta version but it will probably take a few more days (week) before being ready

carlocamilloni commented 7 years ago

about the double listing now it is actually not too bad, if you select METAINFERENCE in the bias section it automatically moves you to the isdb section

maxbonomi commented 7 years ago

I actually like the repetition and I think it can be useful to have in a single page colvars, biases, and functions of all modules, so people can quickly look across different modules and have ideas about how to combine CVs from different areas. On the other side, I think we need for each module a separate page that shows its functionalities. This is what we have been asked when submitting the paper of the isdb module and, more important, it gives a better visibility to other people contribution.

On Jul 21, 2017, at 12:18, Carlo Camilloni notifications@github.com wrote:

about the double listing now it is actually not too bad, if you select METAINFERENCE in the bias section it automatically moves you to the isdb section

— You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

GiovanniBussi commented 7 years ago

I just pushed on branch v2.4 a version where actions from separate modules appear at the end of the list in a separate table.

@valsson can you check if this works well also for the VES module?

It should be sufficient to merge v2.4 in.

GiovanniBussi commented 7 years ago

I think @hockyg and @whitead should also be notified about this thread. At some point they will also have to add the description of their EDS module.

carlocamilloni commented 7 years ago

I merged it and it works but personally while I like the table I would prefer to have the keywords also in the left menu, just because that is the only thing I look at. @maxbonomi ?

GiovanniBussi commented 7 years ago

As an alternative, would it be ok to remove them from the left menu of PLUMED-ISDB so that they appear only in the left menu of the bias/colvar/function part? That's super easy (just swapping ref and subpage in the generation of the manual)

carlocamilloni commented 7 years ago

The point is that I don't share your dislike for the duplication of the keywords in the left menu ;-) I mean I don't see why there should be a table but then only some actions are on the left and some are not. It is clear that if something appear twice is because it can belong to more than one category

valsson commented 7 years ago

I tested the new script on the manual for the VES module and it seems to work fine. I also agree with @carlocamilloni regarding the duplication of the keywords in the left menu, to me it looks better.

What about tutorial, would it be possible to extend this scheme to tutorials? I have tutorials inside the tutorials folder that are called ves_aaa that I would like to have inside the module subpage. What about if there are tutorials called pippo_aaa they would go to a list called PIPPO_TUTORIALS.list which could be used inside module page? Would this be possible?

maxbonomi commented 7 years ago

I will add one more thought. Shall we group all the modules in a page 'Contributed Modules' ?

As people will start contributing modules, the list on the left menu will get longer and longer (and kind of ugly). I think it would be better just to have one line on the left that expands into all the contributed modules. The main page for 'Contributed Modules' would report a brief description of each module and the authors.

Max

Sent from my Nexus. Please excuse typos and brevity.

On Jul 24, 2017 10:56, "Omar Valsson" notifications@github.com wrote:

I tested the new script on the manual for the VES module and it seems to work fine. I also agree with @carlocamilloni https://github.com/carlocamilloni regarding the duplication of the keywords in the left menu, to me it looks better.

What about tutorial, would it be possible to extend this scheme to tutorials? I have tutorials inside the tutorials folder that are called ves_aaa that I would like to have inside the module subpage. What about if there are tutorials called pippo_aaa they would go to a list called PIPPO_TUTORIALS.list which could be used inside module page? Would this be possible?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/plumed/plumed2/issues/253#issuecomment-317360675, or mute the thread https://github.com/notifications/unsubscribe-auth/ACj-wTuRucXd0d2UOzNgl9uxLWbzRpYbks5sRFw6gaJpZM4OH7G4 .

carlocamilloni commented 7 years ago

Yes I agree with Max on collecting modules together

Sent from my iPhone

Il giorno 24 lug 2017, alle ore 14:12, Massimiliano Bonomi notifications@github.com ha scritto:

I will add one more thought. Shall we group all the modules in a page 'Contributed Modules' ?

As people will start contributing modules, the list on the left menu will get longer and longer (and kind of ugly). I think it would be better just to have one line on the left that expands into all the contributed modules. The main page for 'Contributed Modules' would report a brief description of each module and the authors.

Max

Sent from my Nexus. Please excuse typos and brevity.

On Jul 24, 2017 10:56, "Omar Valsson" notifications@github.com wrote:

I tested the new script on the manual for the VES module and it seems to work fine. I also agree with @carlocamilloni https://github.com/carlocamilloni regarding the duplication of the keywords in the left menu, to me it looks better.

What about tutorial, would it be possible to extend this scheme to tutorials? I have tutorials inside the tutorials folder that are called ves_aaa that I would like to have inside the module subpage. What about if there are tutorials called pippo_aaa they would go to a list called PIPPO_TUTORIALS.list which could be used inside module page? Would this be possible?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/plumed/plumed2/issues/253#issuecomment-317360675, or mute the thread https://github.com/notifications/unsubscribe-auth/ACj-wTuRucXd0d2UOzNgl9uxLWbzRpYbks5sRFw6gaJpZM4OH7G4 .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

carlocamilloni commented 7 years ago

about the tutorials @valsson yes I think we should also include the tutorials in the module, it should be easy

carlocamilloni commented 7 years ago

@valsson could you check the new changes related to tutorials? commit 6a703bed8ba8251f9d664cc0d5ec3b489d8f10d4 Tutorials related to modules should be places in tutorials/others and named xxx-yyy.txt where xxx is the module name i.e. isdb-1.txt

valsson commented 7 years ago

I checked the updated scripts and it seems to work fine. However, instead of xxx-yyy.txt why not use the convention xxx_yyy.txt? This would be consistent with how the actions are labelled (e.g. XXX_COLVAR).

On 25 July 2017 at 12:23:42, Carlo Camilloni (notifications@github.com) wrote:

@valsson https://github.com/valsson could you check the new changes related to tutorials? commit 6a703be https://github.com/plumed/plumed2/commit/6a703bed8ba8251f9d664cc0d5ec3b489d8f10d4 Tutorials related to modules should be places in tutorials/others and named xxx-yyy.txt where xxx is the module name i.e. isdb-1.txt

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/plumed/plumed2/issues/253#issuecomment-317695731, or mute the thread https://github.com/notifications/unsubscribe-auth/AFopLsEg4UyZB1L7cD00wBrWG6RpFIIgks5sRcItgaJpZM4OH7G4 .

carlocamilloni commented 7 years ago

You are right, the only reason is that all other tutorials are named in this way.

On 25 Jul 2017, at 13:41, Omar Valsson notifications@github.com wrote:

I checked the updated scripts and it seems to work fine. However, instead of xxx-yyy.txt why not use the convention xxx_yyy.txt? This would be consistent with how the actions are labelled (e.g. XXX_COLVAR).

On 25 July 2017 at 12:23:42, Carlo Camilloni (notifications@github.com) wrote:

@valsson https://github.com/valsson could you check the new changes related to tutorials? commit 6a703be https://github.com/plumed/plumed2/commit/6a703bed8ba8251f9d664cc0d5ec3b489d8f10d4 Tutorials related to modules should be places in tutorials/others and named xxx-yyy.txt where xxx is the module name i.e. isdb-1.txt

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/plumed/plumed2/issues/253#issuecomment-317695731, or mute the thread https://github.com/notifications/unsubscribe-auth/AFopLsEg4UyZB1L7cD00wBrWG6RpFIIgks5sRcItgaJpZM4OH7G4 . — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/plumed/plumed2/issues/253#issuecomment-317711961, or mute the thread https://github.com/notifications/unsubscribe-auth/AEXl9mMnvyPEj6Xo401J9eEKtqed1-y4ks5sRdSBgaJpZM4OH7G4.

carlocamilloni commented 7 years ago

@valsson could you check if the new addition of a single page where all module are collected works for your version? You can look at the ISDB branch for reference.

maxbonomi commented 7 years ago

Carlo, can we just add to the page "Additional Modules” a brief, one line description of the ISDB module?

On Aug 2, 2017, at 15:19, Carlo Camilloni notifications@github.com wrote:

@valsson could you check if the new addition of a single page where all module are collected works for your version? You can look at the ISDB branch for reference.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

maxbonomi commented 7 years ago

Maybe with authors and a reference (where available)

On Aug 2, 2017, at 15:25, Massimiliano Bonomi massimiliano.bonomi@gmail.com wrote:

Carlo, can we just add to the page "Additional Modules” a brief, one line description of the ISDB module?

On Aug 2, 2017, at 15:19, Carlo Camilloni notifications@github.com wrote:

@valsson could you check if the new addition of a single page where all module are collected works for your version? You can look at the ISDB branch for reference.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

maxbonomi commented 7 years ago

I am also wondering if in the global bias / cv page we can substitute:

(from ISDB module)

with (from PLUMED-ISDB) where PLUMED-ISDB matches the name listed in the Additional Modules and it is a link to the main module page

Max

On Aug 2, 2017, at 15:29, Massimiliano Bonomi massimiliano.bonomi@gmail.com wrote:

Maybe with authors and a reference (where available)

On Aug 2, 2017, at 15:25, Massimiliano Bonomi massimiliano.bonomi@gmail.com wrote:

Carlo, can we just add to the page "Additional Modules” a brief, one line description of the ISDB module?

On Aug 2, 2017, at 15:19, Carlo Camilloni notifications@github.com wrote:

@valsson could you check if the new addition of a single page where all module are collected works for your version? You can look at the ISDB branch for reference.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

gtribello commented 7 years ago

I think that the substitution with from PLUMED-ISDB should be made.

carlocamilloni commented 7 years ago

Yes I was also thinking about it but I haven’t yet found a not too dirty way of doing it

On 2 Aug 2017, at 15:25, Massimiliano Bonomi notifications@github.com wrote:

Carlo, can we just add to the page "Additional Modules” a brief, one line description of the ISDB module?

On Aug 2, 2017, at 15:19, Carlo Camilloni notifications@github.com wrote:

@valsson could you check if the new addition of a single page where all module are collected works for your version? You can look at the ISDB branch for reference.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/plumed/plumed2/issues/253#issuecomment-319671201, or mute the thread https://github.com/notifications/unsubscribe-auth/AEXl9oXfCFKAHQpasKfbbb-53KkX8BS2ks5sUHjhgaJpZM4OH7G4.

carlocamilloni commented 7 years ago

Done, this can be added using these lines in the XXX.md file (e.g. ISDB.md) \

maxbonomi commented 7 years ago

very nice!

On Aug 2, 2017, at 16:50, Carlo Camilloni notifications@github.com wrote:

Done, this can be added using these lines in the XXX.md file (e.g. ISDB.md)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

carlocamilloni commented 7 years ago

now instead of (from ISDB) we have the link to the module page

I am also wondering if in the global bias / cv page we can substitute:

(from ISDB module)

with (from PLUMED-ISDB) where PLUMED-ISDB matches the name listed in the Additional Modules and it is a link to the main module page

maxbonomi commented 7 years ago

I think it is better this way!

On Aug 2, 2017, at 17:10, Carlo Camilloni notifications@github.com wrote:

now instead of (from ISDB) we have the link to the module page

I am also wondering if in the global bias / cv page we can substitute:

(from ISDB module)

with (from PLUMED-ISDB) where PLUMED-ISDB matches the name listed in the Additional Modules and it is a link to the main module page

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

valsson commented 7 years ago

@carlocamilloni I tried the new script and it works pretty well.

One thing is that now one has to add by hand the XXXXPP.md file to the list in the go-doxygen script, and this is something that is easy to forget. Is there a way to do this in an automatic way, for example by copying the XXXXPP.md file to the automatic folder in the extract script, then there is no need to add it to the list in go-doxygen. Then everything is done in an automatic fashion.

Another thing is that the plumedcheck script has to be modified so there is not a failure for command line tools that are documented with XXXX_TOOLS, see my comment above.

carlocamilloni commented 7 years ago

All issues should be addressed now