From c4bfb8cd775be412ab4c528af5f5413e6f40e395 Mon Sep 17 00:00:00 2001
From: Parashara Shamaprasad <33552857+uppittu11@users.noreply.github.com>
Date: Thu, 29 Oct 2020 10:32:09 -0700
Subject: [PATCH] Ensure that top is updated only once in from_mbuild (#470)

Co-authored-by: Co Quach <43968221+daico007@users.noreply.github.com>
---
 gmso/core/subtopology.py        | 12 +++++++++---
 gmso/core/topology.py           |  5 ++++-
 gmso/external/convert_mbuild.py | 11 +++++------
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/gmso/core/subtopology.py b/gmso/core/subtopology.py
index a76f45403..0a1fa03a3 100644
--- a/gmso/core/subtopology.py
+++ b/gmso/core/subtopology.py
@@ -70,17 +70,23 @@ def parent(self, parent):
             )
         self._parent = _validate_parent(parent)
 
-    def add_site(self, site):
+    def add_site(self, site, update_types=True):
         """Add a site to this sub-topology
 
         This method adds a site to the sub-topology.
         If the sub-topology has a parent, the site will
-        also be added to the parent topology.
+        also be added to the parent topology. If the
+        update_types parameter is set to true (default
+        behavior), this method will also check if there
+        is an gmso.AtomType associated with the site and
+        it to the sub-topology's AtomTypes collection.
 
         Parameters
         ----------
         site : gmso.Atom
             The site to be added to this sub-topology
+        update_types : (bool), default=True
+            If true, add this site's atom type to the sub-topology's set of AtomTypes
 
         Raises
         ------
@@ -93,7 +99,7 @@ def add_site(self, site):
             warnings.warn("Redundantly adding Site {}".format(site))
         self._sites.add(site)
         if self.parent:
-            self.parent.add_site(site)
+            self.parent.add_site(site, update_types=update_types)
 
     def __repr__(self):
         descr = list('<')
diff --git a/gmso/core/topology.py b/gmso/core/topology.py
index c991ed49c..474a87ef6 100644
--- a/gmso/core/topology.py
+++ b/gmso/core/topology.py
@@ -491,7 +491,7 @@ def update_atom_types(self):
                 site.atom_type = self._atom_types[site.atom_type]
         self.is_typed(updated=True)
 
-    def add_subtopology(self, subtop):
+    def add_subtopology(self, subtop, update=True):
         """Add a sub-topology to this topology
 
         This methods adds a gmso.Core.SubTopology object to the topology
@@ -502,6 +502,7 @@ def add_subtopology(self, subtop):
         ----------
         subtop : gmso.SubTopology
             The sub-topology object to be added.
+        update : bool, default=True
 
         See Also
         --------
@@ -510,6 +511,8 @@ def add_subtopology(self, subtop):
         self._subtops.add(subtop)
         subtop.parent = self
         self._sites.union(subtop.sites)
+        if update:
+            self.update_topology()
 
     def is_typed(self, updated=False):
         if not updated:
diff --git a/gmso/external/convert_mbuild.py b/gmso/external/convert_mbuild.py
index c8d6b8aae..30223afdf 100644
--- a/gmso/external/convert_mbuild.py
+++ b/gmso/external/convert_mbuild.py
@@ -81,14 +81,13 @@ def from_mbuild(compound, box=None, search_method=element_by_symbol):
             continue
         else:
             subtop = SubTopology(name=child.name)
-            top.add_subtopology(subtop)
+            top.add_subtopology(subtop, update=False)
             for particle in child.particles():
                 pos = particle.xyz[0] * u.nanometer
                 ele = search_method(particle.name)
                 site = Atom(name=particle.name, position=pos, element=ele)
                 site_map[particle] = site
-                subtop.add_site(site)
-    top.update_topology()
+                subtop.add_site(site, update_types=False)
 
     for particle in compound.particles():
         already_added_site = site_map.get(particle, None)
@@ -106,14 +105,14 @@ def from_mbuild(compound, box=None, search_method=element_by_symbol):
         if len(top.subtops) > 0:
             subtop = SubTopology(name=particle.name)
             top.add_subtopology(subtop)
-            subtop.add_site(site)
+            subtop.add_site(site, update_types=False)
         else:
-            top.add_site(site)
+            top.add_site(site, update_types=False)
 
     for b1, b2 in compound.bonds():
         new_bond = Bond(connection_members=[site_map[b1], site_map[b2]],
                 bond_type=None)
-        top.add_connection(new_bond)
+        top.add_connection(new_bond, update_types=False)
     top.update_topology()
 
     if box: