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 bug when computing reduced potential in multiple states #385

Merged
merged 3 commits into from
Nov 9, 2018

Conversation

andrrizzi
Copy link
Contributor

This fixes a bug affecting reduced_potential_at_states() when computing the reduced potential of systems in different GlobalParameterStates when the same alchemical parameter appeared in force objects split in different force groups.

When the thermodynamic parameter appeared in multiple forces that were separated into different force groups, the current algorithm considered only the first force group and the reduced potential was computed incorrectly.
@jchodera
Copy link
Member

jchodera commented Nov 9, 2018

Thanks! Can you comment what impact this might have had on existing calculations?

@andrrizzi
Copy link
Contributor Author

This has no impact on our calculations because the current implementation of AbsoluteAlchemicalFactory keeps the forces associated with the same alchemical parameter into the same force group, but I thought it would be wise to fix this before it bites.

Copy link
Member

@jchodera jchodera left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@andrrizzi andrrizzi merged commit 10af5a6 into master Nov 9, 2018
@andrrizzi andrrizzi deleted the fix-multiple-groups branch November 9, 2018 19:38
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.

2 participants