Contributing new size-and-shape module

[hockyg]

Description

This pull request adds a module called "Size and Shape" based on our work operating directly on atomic positions which involves translation and rotating positions on the fly before computing CV values. This is work from papers with @mmccull @mccullaghlab and @SasmalSubarna

Target release

I would like my code to appear in release 2.9

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

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

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

Thanks @hockyg ! This is going to end up in plumed 2.10 (due this summer). A few comments, in addition to the astyle error that was reported by GitHub Actions:

• Maybe you can prefix the registered colvar names with SIZESHAPE? Something like SIZESHAPE_POSITION_LINEAR_PROJ?
• I noticed you used a std::vector<std::vector<double>>. Is there a reason why a Matrix does not work there? I am just saying because it might be more efficient (though I don't know if it's relevant in your case)
• If I saw it correctly, your regtests are only testing for COLVAR. I would suggest also testing for derivatives, either using DUMPDERIVATIVES or adding a force (e.g. with RESTRAINT or BIASVALUE) and printing forces from the driver command line option
• I see you variables are parallelized, you might want to duplicate your tests and also test with MPI. This requires specifying mpiprocs in the config file (there are many examples in the regtest directory).

Thanks again for this contribution!

[SasmalSubarna]

@GiovanniBussi

Hi! I have put another test for MPI, just like you mentioned last time. But I think it failed some tests, did I make some mistakes? Would you please take a look?

Thank you.

[GiovanniBussi]

Thanks @SasmalSubarna !

As fas as I can tell, the problem is that you save too many digits in derivatives, and the result is not reproducible across different machines for numerical reasons.

Usually it is sufficient to reduce the number of digits with the FMT keyword, something like

DUMPDERIVATIVES ARG=ld1 STRIDE=1 FILE=deriv FMT=%8.4f

and reset the regtest

[GiovanniBussi]

In addition, the codecheck job tells me that you didn't apply astyle formatting:

[sizeshape/mahadist.cpp:0] (error) :plmd_astyle: astyle not satisfied
[sizeshape/pos_proj.cpp:0] (error) :plmd_astyle: astyle not satisfied

It should be sufficient to run make astyle from plumed2/src/sizeshape, and then git commit

[hockyg]

Thanks Carlo and Giovanni!

On Mon, May 13, 2024 at 7:07 AM Carlo Camilloni @.***> wrote:

Merged #1069 https://github.com/plumed/plumed2/pull/1069 into master.

— Reply to this email directly, view it on GitHub https://github.com/plumed/plumed2/pull/1069#event-12785491249, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIONFH7HZ7HSLJGB34QDBDZCCNH3AVCNFSM6AAAAABHJKVBRWVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJSG44DKNBZGEZDIOI . You are receiving this because you were mentioned.Message ID: @.***>