mosdef-hub / mbuild

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

Add cap hydrogens to head and tail compounds in the polymer builder #1196

Closed chrisjonesBSU closed 2 months ago

chrisjonesBSU commented 2 months ago

PR Summary:

This is a small PR that changes how the capping hydrogen compounds are added to the polymer that is built up in the polymer builder. Right now, we add the hydrogen compounds to the polymer compound, so they are at the same hierarchical level as the monomer compounds. This PR now adds them to the head and tail monomers. This is cleaner, and creates a more consistent hierarchy that we would expect with a polymer compound, where its children are the monomers, instead of particles. You can see in these 2 examples below:

import mbuild as mb
from mbuild.lib.recipes import Polymer

chain = Polymer()
chain.add_monomer(compound=mb.load("CC", smiles=True), indices=[4,7], separation=0.145)
chain.build(n=6, sequence="A")

Current behavior:

>>> chain.children
[<Compound 6 particles, 5 bonds, non-periodic, id: 123233398406080>,
 <Compound 6 particles, 5 bonds, non-periodic, id: 123233398257904>,
 <Compound 6 particles, 5 bonds, non-periodic, id: 123233397959872>,
 <Compound 6 particles, 5 bonds, non-periodic, id: 123233391797712>,
 <Compound 6 particles, 5 bonds, non-periodic, id: 123233391798384>,
 <Compound 6 particles, 5 bonds, non-periodic, id: 123233409902032>,
 <H 1 particles, 0 bonds, non-periodic, id: 123233398398928>,
 <H 1 particles, 0 bonds, non-periodic, id: 123233398286400>]

Behavior in this PR:

>>> chain.children
[<Compound 7 particles, 6 bonds, non-periodic, id: 136093088604928>,
 <Compound 6 particles, 5 bonds, non-periodic, id: 136093093706400>,
 <Compound 6 particles, 5 bonds, non-periodic, id: 136093088586192>,
 <Compound 6 particles, 5 bonds, non-periodic, id: 136093088592720>,
 <Compound 6 particles, 5 bonds, non-periodic, id: 136093088583648>,
 <Compound 7 particles, 6 bonds, non-periodic, id: 136093088608576>]

Side note:

This also has a quick fix for a flaky unit test that seems to have some inconsistent behavior (test_get_smiles). This test seems to randomly fail, but not because the behavior is wrong, but because sometimes different (but structurally identical) SMILES strings are being returned.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 87.34%. Comparing base (cd53189) to head (8a04111). Report is 8 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1196 +/- ## ======================================= Coverage 87.34% 87.34% ======================================= Files 62 62 Lines 6584 6584 ======================================= Hits 5751 5751 Misses 833 833 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

chrisjonesBSU commented 2 months ago

Merged after discussing PR with Cal Craven on a mosdef dev call.