Skip to content

Conversation

@skfegan
Copy link
Member

@skfegan skfegan commented Oct 9, 2025

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:

  • For light atoms the selection string passed to MDAnalysis select_atoms is now "prop mass <= 1.1"
  • For heavy atoms the selection string passed to MDAnalysis select_atoms is now "prop mass > 1.1"

Change 2 Change bead definitions

  • When there is no heavy atom in a molecule, assign the whole molecule to one united atom bead

Impact

  • This should now be able to handle dummy atoms (mass usually 0, but not named as hydrogen)
  • It will work correctly as long as the masses are assigned correctly, regardless of how atom names or types are listed in the force field.
  • Hydrogen molecules can be included.
  • There is no risk of confusion between hydrogen and helium atom names.

@skfegan
Copy link
Member Author

skfegan commented Oct 9, 2025

@jkalayan or @ioanaapapa Could you please check if this fixes errors caused by dummy atoms in the 4-point water model?

@skfegan skfegan linked an issue Oct 9, 2025 that may be closed by this pull request
Copy link
Member

@harryswift01 harryswift01 left a 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.

@jkalayan
Copy link
Collaborator

jkalayan commented Oct 9, 2025

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.

Copy link
Collaborator

@ioanaapapa ioanaapapa 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 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

skfegan and others added 3 commits October 14, 2025 13:50
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>
@skfegan skfegan requested a review from harryswift01 October 14, 2025 13:42
Copy link
Member

@harryswift01 harryswift01 left a 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.

@harryswift01 harryswift01 added the bug Something isn't working label Oct 14, 2025
@harryswift01 harryswift01 added this to the v1.0.4 milestone Oct 14, 2025
@harryswift01 harryswift01 changed the title changing light atom selection from atom name hydrogen to mass < 1.1 Change Light Atom Selection Criteria From Name To Mass Oct 14, 2025
@skfegan skfegan merged commit a6d7047 into main Oct 14, 2025
13 checks passed
@skfegan skfegan deleted the 166-light-atoms branch October 14, 2025 13:48
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.

light atoms

5 participants