Skip to content
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

Fix mismatched gmso residue name when molecule is loaded from mol2 #37

Merged
merged 5 commits into from
Aug 31, 2023
Merged

Fix mismatched gmso residue name when molecule is loaded from mol2 #37

merged 5 commits into from
Aug 31, 2023

Conversation

marjanalbooyeh
Copy link
Collaborator

This PR fixes the gmso topology parameterization issue caused by name mismatch in mbuild compound.
For a Molecule loaded from a mol2 file, if we change the mbuild compound name, the name of its children does not change accordingly. When the forcefield dictionary with keys similar to the compound name is being applied to the system, the gmso topology is not parameterized because of this naming mismach.

As a quick fix, this PR makes sure that the name is assigned to every child of the mbuild compound. However, more investigation needs to be done on gmso's behavior when converting an mbuild compound to a topology. (see the example in the comments).

Copy link
Member

@chrisjonesBSU chrisjonesBSU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks!

@marjanalbooyeh
Copy link
Collaborator Author

Here is the example that shows this inconsistency in gmso's from_mbuild behavior based on how that mbuild compound is being loaded:

import mbuild as mb
from mbuild.lib.molecules import Ethane
from gmso.parameterization import apply
from gmso.external.convert_mbuild import from_mbuild
import forcefield_utilities as ffutils

opls = ffutils.FoyerFFs().load(ffname="oplsaa").to_gmso_ff()

ethane = Ethane()
ethane.save("ethane.mol2", overwrite=True)
ethane_mol2 = mb.load("ethane.mol2")
ethane_mol2.name = "Ethane"

ethane_top = from_mbuild(ethane)
ethane_top.identify_connections()

ethane_mol2_top = from_mbuild(ethane_mol2)
ethane_mol2_top.identify_connections()

apply(
    ethane_top,
    {"Ethane": opls},
)

apply(
    ethane_mol2_top,
    {"Ethane": opls},
)

for site in ethane_top.sites:
    print(site.atom_type.parameters)

for site in ethane_mol2_top.sites:
    print(site.atom_type.parameters)

In this example, the ethane_mol2_top is not parameterized.

@marjanalbooyeh marjanalbooyeh marked this pull request as ready for review August 31, 2023 19:48
@codecov
Copy link

codecov bot commented Aug 31, 2023

Codecov Report

Merging #37 (d1a9e87) into main (165fc3f) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #37      +/-   ##
==========================================
+ Coverage   92.23%   92.24%   +0.01%     
==========================================
  Files          18       18              
  Lines        1326     1328       +2     
==========================================
+ Hits         1223     1225       +2     
  Misses        103      103              
Files Changed Coverage Δ
hoomd_organics/base/molecule.py 96.62% <100.00%> (+0.02%) ⬆️

@marjanalbooyeh marjanalbooyeh merged commit d91f1a9 into cmelab:main Aug 31, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants