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

Fix to ves_md_linearexpansion with periodic potential #649

Merged
merged 2 commits into from Nov 20, 2020

Conversation

bpampel
Copy link
Contributor

@bpampel bpampel commented Nov 20, 2020

Description

The periodic boundary conditions of the ves_md_linearexpansion tool previously shifted the potential range so it is symmetric with respect to the origin. (Example: [0,1] -> [-0.5,0.5])
This PR changes the way the pbc is calculated, so if the specified range is not symmetric, it is actually kept.

Also includes a new regtest to verify the correct behaviour.

This should not change any simulations where a symmetric range was specified, so it is fully backwards compatible.

The VES maintainer @valsson has seen and agreed to the changes.

Target release

I would like my code to appear in release v2.5 and all newer

Type of contribution
  • changes to code or doc authored by PLUMED developers
  • 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.

@codecov-io
Copy link

Codecov Report

Merging #649 (62ca1c8) into master (5becda9) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #649   +/-   ##
=======================================
  Coverage   85.62%   85.62%           
=======================================
  Files         582      582           
  Lines       45971    45971           
=======================================
  Hits        39361    39361           
  Misses       6610     6610           
Impacted Files Coverage Δ
src/ves/MD_LinearExpansionPES.cpp 97.01% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5becda9...62ca1c8. Read the comment docs.

@GiovanniBussi
Copy link
Member

Looks reasonable to me. @valsson do you confirm? Thanks! Giovanni

@valsson
Copy link
Contributor

valsson commented Nov 20, 2020

Yes, I approve of the PR. Omar

@GiovanniBussi GiovanniBussi merged commit e6749a0 into plumed:master Nov 20, 2020
GiovanniBussi added a commit that referenced this pull request Nov 20, 2020
@bpampel bpampel deleted the ves-md-linearexpansion-pbc branch February 3, 2021 09:10
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.

None yet

4 participants