-
Notifications
You must be signed in to change notification settings - Fork 33
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
Sort indices in Frame.bond.group
#769
Conversation
This seems reasonable especially because getting things into a format for freud is good. Yeah it looks like things go to parmed systems. Could be a good test to make sure the mbuild snapshot writer handles this the same way as the gmso one. |
Would it be easier/faster to sneak this into #765? Or would these clash/out-of-scope? |
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #769 +/- ##
==========================================
- Coverage 92.82% 92.81% -0.02%
==========================================
Files 66 66
Lines 6845 6845
==========================================
- Hits 6354 6353 -1
- Misses 491 492 +1
☔ View full report in Codecov by Sentry. |
We could definitely just include this in #765 |
I think the change is small enough, and that it has no impact on the other things (bond types and typeids), I think I gonna merge this and do a release of GMSO this morning. We could work on adding more tests later. |
This is a quick change that sorts the particle indices before appending to
Frame.bond.group
. This should have no effect on bond types or typeids. Thebond.group
is just a list of length 2 with the particle indices in the bond.The reason why I think we should do this is because the locality and neighbor list modules in freud require that the bond arrays are sorted. Also, I think its cleaner and easier to read if you're ever having to read these arrays manually for some reason.
I'm not sure why this was never a problem for me when using the hoomd writers in mBuild, maybe its some how related to how Parmed indexes particles in bonds.