Merge pull request #2960 for optimization and cython tweaks

Fix cython declarations and some optimizations in molecule.py

Author: rwest

rmgpy/molecule/molecule.py

@@ -604,6 +604,7 @@ def apply_action(self, action):
         required parameters. The available actions can be found
         :ref:`here <reaction-recipe-actions>`.
         """
+        cython.declare(act=str, i=cython.Py_ssize_t, n=cython.Py_ssize_t)
         # Invalidate current atom type
         self.atomtype = None
         act = action[0].upper()
@@ -612,17 +613,23 @@ def apply_action(self, action):
             # Nothing else to do here
             pass
         elif act == 'GAIN_RADICAL':
-            for i in range(action[2]): self.increment_radical()
+            n = action[2]
+            for i in range(n): self.increment_radical()
         elif act == 'LOSE_RADICAL':
-            for i in range(abs(action[2])): self.decrement_radical()
+            n = action[2]
+            for i in range(n): self.decrement_radical()
         elif act == 'GAIN_CHARGE':
-            for i in range(action[2]): self.increment_charge()
+            n = action[2]
+            for i in range(n): self.increment_charge()
         elif act == 'LOSE_CHARGE':
-            for i in range(abs(action[2])): self.decrement_charge()
-        elif action[0].upper() == 'GAIN_PAIR':
-            for i in range(action[2]): self.increment_lone_pairs()
-        elif action[0].upper() == 'LOSE_PAIR':
-            for i in range(abs(action[2])): self.decrement_lone_pairs()
+            n = action[2]
+            for i in range(n): self.decrement_charge()
+        elif act == 'GAIN_PAIR':
+            n = action[2]
+            for i in range(n): self.increment_lone_pairs()
+        elif act == 'LOSE_PAIR':
+            n = action[2]
+            for i in range(n): self.decrement_lone_pairs()
         else:
             raise gr.ActionError('Unable to update Atom: Invalid action {0}".'.format(action))
 
@@ -1283,7 +1290,7 @@ def sort_atoms(self):
         Placing hydrogens last during sorting ensures that functions with hydrogen
         removal work properly.
         """
-        cython.declare(vertex=Vertex, a=Atom, index=int)
+        cython.declare(vertex=Vertex, index=int)
         for vertex in self.vertices:
             if vertex.sorting_label < 0:
                 self.update_connectivity_values()
@@ -1318,8 +1325,8 @@ def get_formula(self):
         """
         Return the molecular formula for the molecule.
         """
-        cython.declare(atom=Atom, symbol=str, elements=dict, keys=list, formula=str)
-        cython.declare(hasCarbon=cython.bint, hasHydrogen=cython.bint)
+        cython.declare(atom=Atom, symbol=str, element_dict=dict, keys=list, key=str,
+                       formula=str, count=int)
 
         # Count the number of each element in the molecule
         element_dict = {}
@@ -1391,7 +1398,7 @@ def get_num_atoms(self, element=None):
         Return the number of atoms in molecule.  If element is given, ie. "H" or "C",
         the number of atoms of that element is returned.
         """
-        cython.declare(numAtoms=cython.int, atom=Atom)
+        cython.declare(num_atoms=cython.int, atom=Atom)
         if element is None:
             return len(self.vertices)
         else:
@@ -1473,8 +1480,8 @@ def connect_the_dots(self, critical_distance_factor=0.45, raise_atomtype_excepti
         Delete all bonds, and set them again based on the Atoms' coords.
         Does not detect bond type.
         """
-        cython.declare(criticalDistance=float, i=int, atom1=Atom, atom2=Atom,
-                       bond=Bond, atoms=list, zBoundary=float)
+        cython.declare(critical_distance=float, i=cython.Py_ssize_t, atom=Atom, atom1=Atom, atom2=Atom,
+                       bond=Bond, atoms=list, sorted_atoms=list, z_boundary=float, distance_squared=float)
         # groupBond=GroupBond,
         self._fingerprint = None
 
@@ -1600,7 +1607,7 @@ def get_element_count(self):
         """
         Returns the element count for the molecule as a dictionary.
         """
-        cython.declare(atom=Atom, element_count=dict, symbol=str, key=str)
+        cython.declare(atom=Atom, element_count=dict, symbol=str)
         element_count = {}
         for atom in self.vertices:
             symbol = atom.element.symbol
@@ -1700,9 +1707,8 @@ def is_subgraph_isomorphic(self, other, initial_map=None, generate_initial_map=F
         while the atoms of `other` are the values). The `other` parameter must
         be a :class:`Group` object, or a :class:`TypeError` is raised.
         """
-        cython.declare(group=gr.Group, atom=Atom)
-        cython.declare(carbonCount=cython.short, nitrogenCount=cython.short, oxygenCount=cython.short,
-                       sulfurCount=cython.short, radicalCount=cython.short)
+        cython.declare(group=gr.Group, atom=Atom, element_count=dict, element=str, count=cython.short,
+                       result=cython.bint)
         cython.declare(L=list)
         # It only makes sense to compare a Molecule to a Group for subgraph
         # isomorphism, so raise an exception if this is not what was requested
@@ -1776,9 +1782,7 @@ def find_subgraph_isomorphisms(self, other, initial_map=None, save_order=False):
         The `other` parameter must be a :class:`Group` object, or a
         :class:`TypeError` is raised.
         """
-        cython.declare(group=gr.Group, atom=Atom)
-        cython.declare(carbonCount=cython.short, nitrogenCount=cython.short, oxygenCount=cython.short,
-                       sulfurCount=cython.short, radicalCount=cython.short)
+        cython.declare(group=gr.Group, element_count=dict, element=str, count=cython.short, result=list)
 
         # It only makes sense to compare a Molecule to a Group for subgraph
         # isomorphism, so raise an exception if this is not what was requested
@@ -1940,7 +1944,7 @@ def to_single_bonds(self, raise_atomtype_exception=True):
         This is useful for isomorphism comparison against something that was made
         via from_xyz, which does not attempt to perceive bond orders
         """
-        cython.declare(atom1=Atom, atom2=Atom, bond=Bond, newMol=Molecule, atoms=list, mapping=dict)
+        cython.declare(atom1=Atom, atom2=Atom, bond=Bond, new_mol=Molecule, atoms=list, mapping=dict)
 
         new_mol = Molecule()
         atoms = self.vertices
@@ -2208,7 +2212,7 @@ def is_aromatic(self):
         there will be at least one 6 membered aromatic ring so this algorithm
         will not fail for fused aromatic rings.
         """
-        cython.declare(rc=list, cycle=list, atom=Atom)
+        cython.declare(rings=list, cycle=list, atom=Atom)
         rings = self.get_smallest_set_of_smallest_rings()
         if rings:
             for cycle in rings:
@@ -2402,7 +2406,7 @@ def update_lone_pairs(self):
         Iterate through the atoms in the structure and calculate the
         number of lone electron pairs, assuming a neutral molecule.
         """
-        cython.declare(atom1=Atom, atom2=Atom, bond12=Bond, order=cython.double)
+        cython.declare(atom1=Atom, order=cython.double)
         for atom1 in self.vertices:
             if atom1.is_hydrogen() or atom1.is_surface_site() or atom1.is_electron() or atom1.is_lithium():
                 atom1.lone_pairs = 0
@@ -2419,8 +2423,19 @@ def get_net_charge(self):
         Iterate through the atoms in the structure and calculate the net charge
         on the overall molecule.
         """
-        cython.declare(atom=Atom)
-        return sum([atom.charge for atom in self.vertices])
+        # return sum(atom.charge for atom in self.vertices)
+        cython.declare(
+            i=cython.Py_ssize_t,
+            n=cython.Py_ssize_t,
+            total=cython.int,
+            atom=Atom,
+        )
+        total = 0
+        n = len(self.vertices)
+        for i in range(n):
+            atom = self.vertices[i]
+            total += atom.charge
+        return total
 
     def get_charge_span(self):
         """
@@ -2575,7 +2590,7 @@ def get_aromatic_rings(self, rings=None, save_order=False):
             return [], []
 
         cython.declare(rd_atom_indices=dict, ob_atom_ids=dict, aromatic_rings=list, aromatic_bonds=list)
-        cython.declare(ring0=list, i=cython.int, atom1=Atom, atom2=Atom)
+        cython.declare(ring0=list, i=cython.Py_ssize_t, atom1=Atom, atom2=Atom)
 
         from rdkit.Chem.rdchem import BondType
         AROMATIC = BondType.AROMATIC
@@ -2584,7 +2599,7 @@ def get_aromatic_rings(self, rings=None, save_order=False):
             rings = self.get_smallest_set_of_smallest_rings()
 
         # Remove rings that share more than 3 atoms, since they cannot be planar
-        cython.declare(toRemove=set, j=cython.int, toRemoveSorted=list)
+        cython.declare(to_remove=set, j=cython.Py_ssize_t, to_remove_sorted=list)
         if len(rings) < 2:
             pass
         else:
@@ -3157,7 +3172,7 @@ def get_ring_count_in_largest_fused_ring_system(self) -> int:
         Returns 0 if the molecule has no fused rings (only monocycles or no rings).
         """
         cython.declare(polycycles=list, sssr=list, sssr_sets=list, ring_counts=list)
-        cython.declare(polycycle=list, ring=list)
+        cython.declare(polycycle=list, poly_set=set, ring_set=set, ring_count=cython.int)
 
         polycycles = self.get_polycycles()
         if not polycycles: