Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add NonbondedInteractionGroup potential, rename existing nonbonded potentials #578
Add NonbondedInteractionGroup potential, rename existing nonbonded potentials #578
Changes from 44 commits
fe0fe9b
5848e88
5bf391e
b915a4e
3b93f01
0c06115
6774f28
71c96e3
980185e
945e62a
b532e89
0f24785
ecf76d8
fcd4199
4f4bc51
6185c4b
efa4828
3b3ea55
d0f154a
3eb51ce
0cd9168
05b1b35
43fd522
653fe4d
a70a330
690a57c
1097ca4
d728988
eb07db3
3e36b51
2dfb18d
39907d6
561ce2f
dd543ea
25fd4f6
860dc9d
b7ebc26
89ee22f
a6486c1
6163d3c
824342d
a986a34
91d1722
efb100d
90d0cd5
695f2c1
6c5598b
dbf4fbe
6d199ca
22f9dc9
cdd37ff
6d3ce6c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests here look great to me now!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that most of the alchemical changes (and their contributions to the delta Us) will be coming from the interaction group, we should expand the coverage to include typical ways that the interaction group may depend on lambda.
a) Currently the offset_idxs are all zero for the tests (only plane_idxs are modified in _lambda_planes()). The dependence on lambda isn't fully tested here. Because the w coordinates are:
b) Need more test coverage for the InterpolatedNonbondedInteractionGroup (esp in conjunction with varying
lambda_plane_idxs
andlambda_offset_idxs
etc.). I don't think I see any right now.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, makes sense. Updated to test with nonzero plane and offset idxs in 6c5598b and dbf4fbe.
Good call, this was untested. Added a Python wrapper and test in 6d199ca