Skip to content

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.

3 participants