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 #234 #235

Merged
merged 5 commits into from
Oct 9, 2024
Merged

Fix #234 #235

merged 5 commits into from
Oct 9, 2024

Conversation

mb2055
Copy link
Contributor

@mb2055 mb2055 commented Sep 26, 2024

This PR closes #234. Adds the required N_A for volume correction in NPT ensembles.

  • I confirm that I have merged the latest version of devel into this branch before issuing this pull request (e.g. by running git pull origin devel): y
  • I confirm that I have added a changelog entry to the changelog (we will add a link to this PR as part of the review): y
  • I confirm that I have permission to release this code under the GPL3 license: y

Comment on lines 106 to 109
delta = -1.0 * (
beta1 * (nrgs1[0] - nrgs1[1] + (pressure1 * (volume1 - volume0) * N_A))
+ beta0 * (nrgs0[1] - nrgs0[0] + (pressure0 * (volume0 - volume1) * N_A))
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be written without the factor of -1 at the start, i.e. so it matches the formatting for the NVT case. Just makes things a bit clearer when glancing between them.

lohedges
lohedges previously approved these changes Oct 4, 2024
Copy link
Contributor

@lohedges lohedges left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the suggested change, everything is fine. Once ready, I'll merge this across into #237 so we can merge all of the fixes in one go.

Cheers.

@mb2055
Copy link
Contributor Author

mb2055 commented Oct 4, 2024

The sign is now multiplied out 👍

@lohedges lohedges merged commit 94e4d64 into devel Oct 9, 2024
@lohedges lohedges deleted the fix_234 branch October 9, 2024 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Replica exchange fails in NPT ensemble
2 participants