search

mosdef-hub / mbuild

A hierarchical, component based molecule builder
https://mbuild.mosdef.org
Other
171 stars 80 forks source link

Use list to store compound.children #1121

Closed daico007 closed 1 year ago

daico007 commented 1 year ago

PR Summary:

After the discussion with @chrisiacovella, we have decided it would be faster to store Compound.children with list instead of the in house OrderedSet. The replacement will speed up both the Compound.add step by removing the comparing stage when adding new children to a set, and the indexing (getting element from a list/set by index). We realized the latter was the cause for https://github.com/mosdef-hub/mbuild/issues/1114, not directly the Compound.add_bond itself, but how the particle is retrieved from Compound.children to perform the addition.

PR Checklist

daico007 commented 1 year ago

@chrisiacovella, I kept the containment intact, since it's still being used by the polymer.build, and if it's false, the children is not being added anyway. The in-house OrderedSet is kept since we are still using it in the lammpsdata.py, and it's technically a modified OrderedSet with some extra property.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.02 :warning:

Comparison is base (87edc4a) 87.15% compared to head (acabea0) 87.13%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1121 +/- ## ========================================== - Coverage 87.15% 87.13% -0.02% ========================================== Files 62 62 Lines 6422 6420 -2 ========================================== - Hits 5597 5594 -3 - Misses 825 826 +1 ``` | [Impacted Files](https://app.codecov.io/gh/mosdef-hub/mbuild/pull/1121?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mosdef-hub) | Coverage Δ | | |---|---|---| | [mbuild/coarse\_graining.py](https://app.codecov.io/gh/mosdef-hub/mbuild/pull/1121?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mosdef-hub#diff-bWJ1aWxkL2NvYXJzZV9ncmFpbmluZy5weQ==) | `92.30% <100.00%> (-0.07%)` | :arrow_down: | | [mbuild/compound.py](https://app.codecov.io/gh/mosdef-hub/mbuild/pull/1121?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mosdef-hub#diff-bWJ1aWxkL2NvbXBvdW5kLnB5) | `97.11% <100.00%> (-0.01%)` | :arrow_down: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/mosdef-hub/mbuild/pull/1121/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mosdef-hub)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.