-
Notifications
You must be signed in to change notification settings - Fork 1
Change Light Atom Selection Criteria From Name To Mass #167
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
Conversation
|
@jkalayan or @ioanaapapa Could you please check if this fixes errors caused by dummy atoms in the 4-point water model? |
harryswift01
left a comment
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.
These all look like good changes. It looks as though the test coverage has dropped ever so slightly though, I think this is in relation to line 345 in the levels.py file where there is an append for all now. I think this would be the case of updating the test_get_beads_united_atom_level test and we would be back up to 100% but apart from that I would be happy to approve this.
|
Changes look great @skfegan, this works for a simulation containing 4-point waters and ensures dummy atoms of zero mass are excluded as united atoms. |
ioanaapapa
left a comment
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.
Looks good to me! For a small organic molecule for 1 frame, compared to the result obtained with waterEntropy and the charge swapping python script, it seems to give a similar result.
SHF_CodeEntropy_light_atoms_1frame.txt
SHF_waterEntropy_swap.txt
Bumps [softprops/action-gh-release](https://github.com/softprops/action-gh-release) from 2.4.0 to 2.4.1. - [Release notes](https://github.com/softprops/action-gh-release/releases) - [Changelog](https://github.com/softprops/action-gh-release/blob/master/CHANGELOG.md) - [Commits](softprops/action-gh-release@v2.4.0...v2.4.1) --- updated-dependencies: - dependency-name: softprops/action-gh-release dependency-version: 2.4.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
harryswift01
left a comment
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.
These changes look great and it looks like we've now got a more robust way of ensuring we select the right atoms and tests have been added to ensure this works as expected, I'm very happy to approve this.
Summary
This pull request changes how we identify heavy/light atoms. We are now avoiding using the hydrogen atom name.
Changes
Change 1 Change atom selectors:
Change 2 Change bead definitions
Impact