From 8c0d25817f8cd5031fab4eb44b0e81195cc2a834 Mon Sep 17 00:00:00 2001 From: Bjarne Kreitz Date: Fri, 12 Jul 2024 16:13:08 -0400 Subject: [PATCH 01/27] draw vdW bond, the same way we do H bonds combine drawing of vdW and H bonds Two commits by Bjarne Kreitz Merged by Richard West --- rmgpy/molecule/converter.py | 4 ++-- rmgpy/molecule/draw.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rmgpy/molecule/converter.py b/rmgpy/molecule/converter.py index ae005cb25b6..5142f916527 100644 --- a/rmgpy/molecule/converter.py +++ b/rmgpy/molecule/converter.py @@ -99,10 +99,10 @@ def to_rdkit_mol(mol, remove_h=True, return_mapping=False, sanitize=True, label_dict[index] = atom.label rd_bonds = Chem.rdchem.BondType - # no vdW bond in RDKit, so "ZERO" or "OTHER" might be OK + # no vdW bond in RDKit, so use UNSPECIFIED orders = {'S': rd_bonds.SINGLE, 'D': rd_bonds.DOUBLE, 'T': rd_bonds.TRIPLE, 'B': rd_bonds.AROMATIC, - 'Q': rd_bonds.QUADRUPLE, 'vdW': rd_bonds.ZERO, + 'Q': rd_bonds.QUADRUPLE, 'vdW': rd_bonds.UNSPECIFIED, 'H': rd_bonds.HYDROGEN, 'R': rd_bonds.UNSPECIFIED, None: rd_bonds.UNSPECIFIED} # Add the bonds diff --git a/rmgpy/molecule/draw.py b/rmgpy/molecule/draw.py index c6931e46ea6..377ca12a39c 100644 --- a/rmgpy/molecule/draw.py +++ b/rmgpy/molecule/draw.py @@ -1215,7 +1215,7 @@ def _render_bond(self, atom1, atom2, bond, cr): dv *= 1.6 self._draw_line(cr, x1 - du, y1 - dv, x2 - du, y2 - dv) self._draw_line(cr, x1 + du, y1 + dv, x2 + du, y2 + dv, dashed=True) - elif bond.is_hydrogen_bond(): + elif bond.is_hydrogen_bond() or bond.is_van_der_waals(): # Draw a dashed line self._draw_line(cr, x1, y1, x2, y2, dashed=True, dash_sizes=[0.5, 3.5]) else: From e1556f558262edf62d1cdb2415190861e3521cda Mon Sep 17 00:00:00 2001 From: Bjarne Kreitz Date: Mon, 15 Jul 2024 11:09:46 -0400 Subject: [PATCH 02/27] Adjust remove vdW bond, using new has_covalent_surface_bond function Combination of two commits by Bjarne Kreitz Merged by Richard West. --- rmgpy/molecule/molecule.py | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/rmgpy/molecule/molecule.py b/rmgpy/molecule/molecule.py index 8cf2ddca7d4..acc1531863c 100644 --- a/rmgpy/molecule/molecule.py +++ b/rmgpy/molecule/molecule.py @@ -1219,6 +1219,19 @@ def has_bond(self, atom1, atom2): """ return self.has_edge(atom1, atom2) + def has_covalent_surface_bond(self): + """ + Return True if any bond in this molecule connects a surface site (X) via a covalent bond. + """ + cython.declare(bond=Bond) + for bond in self.get_all_edges(): + if bond.is_van_der_waals(): + continue + atom1, atom2 = bond.atom1, bond.atom2 + if atom1.is_surface_site() or atom2.is_surface_site(): + return True + return False + def contains_surface_site(self): """ Returns ``True`` iff the molecule contains an 'X' surface site. @@ -1276,9 +1289,17 @@ def remove_van_der_waals_bonds(self): Remove all van der Waals bonds. """ cython.declare(bond=Bond) - for bond in self.get_all_edges(): - if bond.is_van_der_waals(): - self.remove_bond(bond) + if self.is_multidentate(): + if self.has_covalent_surface_bond(): + return #preserve the remaining vdW bonds for this structure + else: + for bond in self.get_all_edges(): + if bond.is_van_der_waals(): + self.remove_bond(bond) + else: + for bond in self.get_all_edges(): + if bond.is_van_der_waals(): + self.remove_bond(bond) def sort_atoms(self): """ From 9bf28a58e08953b72de1bf737974f7eed1abc0a4 Mon Sep 17 00:00:00 2001 From: Richard West Date: Mon, 18 May 2026 23:20:43 -0400 Subject: [PATCH 03/27] Make has_covalent_surface_bond more efficient. Rather than generate all the bonds, each with two ends (which happens by iterating over the atoms) and then iterating over the bonds, and seeing if either end is an X. We instead iterate over the atoms (there are fewer) and only if one is an X do we check its bonds. As soon as one is found that's not vdW, we return True. Also added a Cython declaration. --- rmgpy/molecule/molecule.pxd | 2 ++ rmgpy/molecule/molecule.py | 13 ++++++------- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/rmgpy/molecule/molecule.pxd b/rmgpy/molecule/molecule.pxd index 1b62af07d76..219bb6acb3f 100644 --- a/rmgpy/molecule/molecule.pxd +++ b/rmgpy/molecule/molecule.pxd @@ -206,6 +206,8 @@ cdef class Molecule(Graph): cpdef int number_of_surface_sites(self) except -1 + cpdef bint has_covalent_surface_bond(self) + cpdef bint is_surface_site(self) cpdef remove_atom(self, Atom atom) diff --git a/rmgpy/molecule/molecule.py b/rmgpy/molecule/molecule.py index acc1531863c..b1512765034 100644 --- a/rmgpy/molecule/molecule.py +++ b/rmgpy/molecule/molecule.py @@ -1223,13 +1223,12 @@ def has_covalent_surface_bond(self): """ Return True if any bond in this molecule connects a surface site (X) via a covalent bond. """ - cython.declare(bond=Bond) - for bond in self.get_all_edges(): - if bond.is_van_der_waals(): - continue - atom1, atom2 = bond.atom1, bond.atom2 - if atom1.is_surface_site() or atom2.is_surface_site(): - return True + cython.declare(atom=Atom, bond=Bond) + for atom in self.atoms: + if atom.is_surface_site(): + for bond in atom.bonds.values(): + if not bond.is_van_der_waals(): + return True return False def contains_surface_site(self): From 3c7369849243eaaa50adaa5bd1cdfe23b3293ebd Mon Sep 17 00:00:00 2001 From: Richard West Date: Mon, 18 May 2026 23:27:01 -0400 Subject: [PATCH 04/27] Add test_has_covalent_surface_bond to test has_covalent_surface_bond. --- test/rmgpy/molecule/moleculeTest.py | 38 +++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/test/rmgpy/molecule/moleculeTest.py b/test/rmgpy/molecule/moleculeTest.py index 5ece70da6a8..68f327e723c 100644 --- a/test/rmgpy/molecule/moleculeTest.py +++ b/test/rmgpy/molecule/moleculeTest.py @@ -3110,6 +3110,44 @@ def test_remove_van_der_waals_bonds(self): mol.remove_van_der_waals_bonds() assert len(mol.get_all_edges()) == 1 + def test_has_covalent_surface_bond(self): + """Test Molecule.has_covalent_surface_bond() distinguishes vdW from covalent X bonds.""" + # X present but only physisorbed via a vdW bond + vdw_only = Molecule().from_adjacency_list( + """ +1 X u0 p0 c0 {2,vdW} +2 H u0 p0 c0 {1,vdW} {3,S} +3 H u0 p0 c0 {2,S} +""" + ) + assert not vdw_only.has_covalent_surface_bond() + + # X covalently bonded (chemisorbed) + chemisorbed = Molecule().from_adjacency_list( + """ +1 H u0 p0 c0 {2,S} +2 X u0 p0 c0 {1,S} +""" + ) + assert chemisorbed.has_covalent_surface_bond() + + # Two X atoms: one vdW, one covalent + mixed = Molecule().from_adjacency_list( + """ +1 X u0 p0 c0 {3,S} +2 X u0 p0 c0 {4,vdW} +3 C u0 p0 c0 {1,S} {4,S} {5,S} {6,S} +4 H u0 p0 c0 {2,vdW} {3,S} +5 H u0 p0 c0 {3,S} +6 H u0 p0 c0 {3,S} +""" + ) + assert mixed.has_covalent_surface_bond() + + # No surface sites at all + gas = Molecule().from_smiles("CCO") + assert not gas.has_covalent_surface_bond() + def test_get_relevant_cycles(self): """ Test the Molecule.get_relevant_cycles() raises correct error after deprecation. From 37b2b28a2a031ad9bbd51ef5ce12f567b73e1a9a Mon Sep 17 00:00:00 2001 From: Richard West Date: Mon, 18 May 2026 23:32:08 -0400 Subject: [PATCH 05/27] Simplify remove_van_der_waals_bonds We don't need to check if it's multidentate and THEN check if there's a covalent surface bond. Knowing that there's a covalent surface bond will suffice, and still only involves looping through the atoms once each. (same as counting the X atoms). And if we haven't returned early, we should remove the bonds. --- rmgpy/molecule/molecule.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/rmgpy/molecule/molecule.py b/rmgpy/molecule/molecule.py index b1512765034..3b430a46bc9 100644 --- a/rmgpy/molecule/molecule.py +++ b/rmgpy/molecule/molecule.py @@ -1288,17 +1288,11 @@ def remove_van_der_waals_bonds(self): Remove all van der Waals bonds. """ cython.declare(bond=Bond) - if self.is_multidentate(): - if self.has_covalent_surface_bond(): - return #preserve the remaining vdW bonds for this structure - else: - for bond in self.get_all_edges(): - if bond.is_van_der_waals(): - self.remove_bond(bond) - else: - for bond in self.get_all_edges(): - if bond.is_van_der_waals(): - self.remove_bond(bond) + if self.has_covalent_surface_bond(): + return # preserve any vdW bonds if there's also a covalent X + for bond in self.get_all_edges(): + if bond.is_van_der_waals(): + self.remove_bond(bond) def sort_atoms(self): """ From da4c7d301772fdb621b21a776c17a643afee26ee Mon Sep 17 00:00:00 2001 From: Richard West Date: Mon, 18 May 2026 23:39:20 -0400 Subject: [PATCH 06/27] Accelerate is_multidentate. The optimized is_multidentate now short-circuits on the second surface site, and avoids building an intermediate list. --- rmgpy/molecule/molecule.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/rmgpy/molecule/molecule.py b/rmgpy/molecule/molecule.py index 3b430a46bc9..f2991a55984 100644 --- a/rmgpy/molecule/molecule.py +++ b/rmgpy/molecule/molecule.py @@ -3062,9 +3062,13 @@ def is_multidentate(self): Return ``True`` if the adsorbate contains at least two binding sites, or ``False`` otherwise. """ - cython.declare(atom=Atom) - if len([atom for atom in self.vertices if atom.is_surface_site()])>=2: - return True + cython.declare(atom=Atom, found_one=cython.bint) + found_one = False + for atom in self.atoms: + if atom.is_surface_site(): + if found_one: + return True + found_one = True return False def get_adatoms(self): From 5528ac48371cd9232dbee102f91bdca648f40c45 Mon Sep 17 00:00:00 2001 From: Bjarne Kreitz Date: Wed, 17 Jul 2024 09:03:32 -0400 Subject: [PATCH 07/27] change is_molecule_forbidden --- rmgpy/data/kinetics/family.py | 14 ++++++++++---- rmgpy/molecule/molecule.py | 4 +++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/rmgpy/data/kinetics/family.py b/rmgpy/data/kinetics/family.py index e181ae99e44..c9c499f6f1b 100644 --- a/rmgpy/data/kinetics/family.py +++ b/rmgpy/data/kinetics/family.py @@ -1676,11 +1676,17 @@ def is_molecule_forbidden(self, molecule): return True # forbid vdw multi-dentate molecules for surface families + surface_sites = [] if "surface" in self.label.lower(): - if molecule.get_num_atoms('X') > 1: - for atom in molecule.atoms: - if atom.atomtype.label == 'Xv': - return True + if "surface_monodentate_to_vdw_bidentate" in self.label.lower() and molecule.get_num_atoms('X') > 1: + surface_sites = [atom.atomtype.label for atom in molecule.atoms if 'X' in atom.atomtype.label] + if all(site == 'Xv' for site in surface_sites): + return True + else: + if molecule.get_num_atoms('X') > 1: + for atom in molecule.atoms: + if atom.atomtype.label == 'Xv': + return True return False diff --git a/rmgpy/molecule/molecule.py b/rmgpy/molecule/molecule.py index f2991a55984..dc263872b40 100644 --- a/rmgpy/molecule/molecule.py +++ b/rmgpy/molecule/molecule.py @@ -67,7 +67,7 @@ def _skip_first(in_tuple): return in_tuple[1:] -bond_orders = {'S': 1, 'D': 2, 'T': 3, 'B': 1.5} +bond_orders = {'S': 1, 'D': 2, 'T': 3, 'B': 1.5, 'vdW': 0} globals().update({ 'bond_orders': bond_orders, @@ -3132,6 +3132,8 @@ def get_desorbed_molecules(self): bonded_atom.increment_radical() bonded_atom.increment_lone_pairs() bonded_atom.label = '*4' + elif bond.is_van_der_waals(): + bonded_atom.label = '*5' else: raise NotImplementedError("Can't remove surface bond of type {}".format(bond.order)) desorbed_molecule.remove_atom(site) From 3d29d618cbf78b3f965a4e3adc27b1a2cd712ea0 Mon Sep 17 00:00:00 2001 From: Bjarne Kreitz Date: Tue, 12 May 2026 21:05:54 -0400 Subject: [PATCH 08/27] adding find_formate_delocalization_paths to pathfinder --- rmgpy/molecule/pathfinder.pxd | 4 +++- rmgpy/molecule/pathfinder.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/rmgpy/molecule/pathfinder.pxd b/rmgpy/molecule/pathfinder.pxd index 469417983fb..d34ae224e2f 100644 --- a/rmgpy/molecule/pathfinder.pxd +++ b/rmgpy/molecule/pathfinder.pxd @@ -61,4 +61,6 @@ cpdef bint is_atom_able_to_lose_lone_pair(Vertex atom) cpdef list find_adsorbate_delocalization_paths(Vertex atom1) -cpdef list find_adsorbate_conjugate_delocalization_paths(Vertex atom1) \ No newline at end of file +cpdef list find_adsorbate_conjugate_delocalization_paths(Vertex atom1) + +cpdef list find_formate_delocalization_paths(Vertex atom1) \ No newline at end of file diff --git a/rmgpy/molecule/pathfinder.py b/rmgpy/molecule/pathfinder.py index 8bce591a4dd..9e88cebf52b 100644 --- a/rmgpy/molecule/pathfinder.py +++ b/rmgpy/molecule/pathfinder.py @@ -535,3 +535,31 @@ def find_adsorbate_conjugate_delocalization_paths(atom1): if atom5.is_surface_site(): paths.append([atom1, atom2, atom3, atom4, atom5, bond12, bond23, bond34, bond45]) return paths + +def find_formate_delocalization_paths(atom1): + """ + Find all resonance structures which have a bonding configuration X...O-C-O-X. + Examples: + + - XOC(H)XO/XOC(H)XO, where X is the surface site. The adsorption site X + is always placed on the left-hand side of the adatom and every adatom + is bonded to only one surface site X. + + In this transition atom1 and atom5 are surface sites while atom2 + and atom4 are oxygen and atom3 is a carbon atom. + """ + + cython.declare(paths=list, atom2=Vertex, atom3=Vertex, atom4=Vertex, atom5=Vertex, bond12=Edge, bond23=Edge, bond34=Edge, bond45=Edge) + + paths = [] + if atom1.is_surface_site(): + for atom2, bond12 in atom1.edges.items(): + if atom2.is_oxygen() and bond12.is_van_der_waals(): + for atom3, bond23 in atom2.edges.items(): + if atom3.is_carbon(): + for atom4, bond34 in atom3.edges.items(): + if atom2 is not atom4 and atom4.is_oxygen(): + for atom5, bond45 in atom4.edges.items(): + if atom5.is_surface_site(): + paths.append([atom1, atom2, atom3, atom4, atom5, bond12, bond23, bond34, bond45]) + return paths \ No newline at end of file From df826a639900f5df6670caae8734366192a00ec9 Mon Sep 17 00:00:00 2001 From: Richard West Date: Tue, 19 May 2026 11:31:33 -0400 Subject: [PATCH 09/27] Fix some Cython declarations in pathfinder.py If we declare these things as Atoms and Bonds then Cython can directly access the methods like .is_surface_site() and .is_van_der_waals() I noticed the pxd declares a whole ton of things as Vertex instead of Atom. This should probably also be fixed, if I'm right. --- rmgpy/molecule/pathfinder.pxd | 3 ++- rmgpy/molecule/pathfinder.py | 37 ++++++++++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/rmgpy/molecule/pathfinder.pxd b/rmgpy/molecule/pathfinder.pxd index d34ae224e2f..1403d046bcd 100644 --- a/rmgpy/molecule/pathfinder.pxd +++ b/rmgpy/molecule/pathfinder.pxd @@ -26,6 +26,7 @@ ############################################################################### from .graph cimport Vertex, Edge, Graph +from .molecule cimport Atom, Bond, Molecule cpdef list find_butadiene(Vertex start, Vertex end) @@ -63,4 +64,4 @@ cpdef list find_adsorbate_delocalization_paths(Vertex atom1) cpdef list find_adsorbate_conjugate_delocalization_paths(Vertex atom1) -cpdef list find_formate_delocalization_paths(Vertex atom1) \ No newline at end of file +cpdef list find_formate_delocalization_paths(Vertex atom1) diff --git a/rmgpy/molecule/pathfinder.py b/rmgpy/molecule/pathfinder.py index 9e88cebf52b..33756be3ebd 100644 --- a/rmgpy/molecule/pathfinder.py +++ b/rmgpy/molecule/pathfinder.py @@ -36,7 +36,7 @@ import cython -from rmgpy.molecule.molecule import Atom +from rmgpy.molecule.molecule import Atom, Bond from rmgpy.molecule.graph import Vertex, Edge def find_butadiene(start, end): @@ -494,7 +494,15 @@ def find_adsorbate_delocalization_paths(atom1): In this transition atom1 and atom4 are surface sites while atom2 and atom3 are carbon or nitrogen atoms. """ - cython.declare(paths=list, atom2=Vertex, atom3=Vertex, atom4=Vertex, bond12=Edge, bond23=Edge, bond34=Edge) + cython.declare( + paths=list, + atom2=Atom, + atom3=Atom, + atom4=Atom, + bond12=Bond, + bond23=Bond, + bond34=Bond, + ) paths = [] if atom1.is_surface_site(): @@ -521,7 +529,17 @@ def find_adsorbate_conjugate_delocalization_paths(atom1): and atom4 are carbon or nitrogen atoms. """ - cython.declare(paths=list, atom2=Vertex, atom3=Vertex, atom4=Vertex, atom5=Vertex, bond12=Edge, bond23=Edge, bond34=Edge, bond45=Edge) + cython.declare( + paths=list, + atom2=Atom, + atom3=Atom, + atom4=Atom, + atom5=Atom, + bond12=Bond, + bond23=Bond, + bond34=Bond, + bond45=Bond, + ) paths = [] if atom1.is_surface_site(): @@ -549,8 +567,17 @@ def find_formate_delocalization_paths(atom1): and atom4 are oxygen and atom3 is a carbon atom. """ - cython.declare(paths=list, atom2=Vertex, atom3=Vertex, atom4=Vertex, atom5=Vertex, bond12=Edge, bond23=Edge, bond34=Edge, bond45=Edge) - + cython.declare( + paths=list, + atom2=Atom, + atom3=Atom, + atom4=Atom, + atom5=Atom, + bond12=Bond, + bond23=Bond, + bond34=Bond, + bond45=Bond, + ) paths = [] if atom1.is_surface_site(): for atom2, bond12 in atom1.edges.items(): From d0a0cce0a4078ce0eb6ee58bea223d781de82d2f Mon Sep 17 00:00:00 2001 From: Bjarne Kreitz Date: Tue, 12 May 2026 21:13:11 -0400 Subject: [PATCH 10/27] add resonance structure generation for formate to resonance.py --- rmgpy/molecule/resonance.pxd | 2 ++ rmgpy/molecule/resonance.py | 40 ++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/rmgpy/molecule/resonance.pxd b/rmgpy/molecule/resonance.pxd index b95c0b8ca0b..e0b0381d646 100644 --- a/rmgpy/molecule/resonance.pxd +++ b/rmgpy/molecule/resonance.pxd @@ -73,3 +73,5 @@ cpdef list generate_adsorbate_shift_down_resonance_structures(Graph mol) cpdef list generate_adsorbate_shift_up_resonance_structures(Graph mol) cpdef list generate_adsorbate_conjugate_resonance_structures(Graph mol) + +cpdef list generate_formate_resonance_structures(Graph mol) diff --git a/rmgpy/molecule/resonance.py b/rmgpy/molecule/resonance.py index 577d7088931..622e419921c 100644 --- a/rmgpy/molecule/resonance.py +++ b/rmgpy/molecule/resonance.py @@ -124,6 +124,7 @@ def populate_resonance_algorithms(features=None): method_list.append(generate_adsorbate_shift_down_resonance_structures) method_list.append(generate_adsorbate_shift_up_resonance_structures) method_list.append(generate_adsorbate_conjugate_resonance_structures) + method_list.append(generate_formate_resonance_structures) return method_list @@ -1257,3 +1258,42 @@ def generate_adsorbate_conjugate_resonance_structures(mol): else: structures.append(structure) return structures + + +def generate_formate_resonance_structures(mol): + """ + Generate all of the resonance structures formed by the shift of two + electrons in a conjugated pi bond system of a bidentate adsorbate + with a bridging atom in between. + + Example [X]OC(H)O[X]: [X]~OC(H)O[X] <=> [X]OC(H)O~[X] + (where '~' denotes a vdW bond) + """ + cython.declare(structures=list, paths=list, index=cython.int, structure=Graph) + cython.declare(atom=Vertex, atom1=Vertex, atom2=Vertex, atom3=Vertex, atom4=Vertex, atom5=Vertex, bond12=Edge, bond23=Edge, bond34=Edge, bond45=Edge) + cython.declare(v1=Vertex, v2=Vertex) + + structures = [] + if mol.is_multidentate(): + for atom in mol.vertices: + paths = pathfinder.find_formate_delocalization_paths(atom) + for atom1, atom2, atom3, atom4, atom5, bond12, bond23, bond34, bond45 in paths: + if ((atom2.is_oxygen() and bond12.is_van_der_waals()) and + (atom4.is_oxygen() and atom5.is_surface_site() and bond45.is_single())): + bond12.increment_order() + bond23.decrement_order() + bond34.increment_order() + bond45.decrement_order() + structure = mol.copy(deep=True) + bond12.decrement_order() + bond23.increment_order() + bond34.decrement_order() + bond45.increment_order() + try: + structure.update_atomtypes(log_species=False) + except AtomTypeError: + pass + else: + structures.append(structure) + + return structures From 666a1406b6ee867a4785cd03b80b259a27569f9e Mon Sep 17 00:00:00 2001 From: Bjarne Kreitz Date: Tue, 12 May 2026 21:48:33 -0400 Subject: [PATCH 11/27] add label for vdW bond in get_desorbed_molecules --- rmgpy/molecule/molecule.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/rmgpy/molecule/molecule.py b/rmgpy/molecule/molecule.py index dc263872b40..048f26f8734 100644 --- a/rmgpy/molecule/molecule.py +++ b/rmgpy/molecule/molecule.py @@ -1285,7 +1285,10 @@ def remove_bond(self, bond): def remove_van_der_waals_bonds(self): """ - Remove all van der Waals bonds. + Remove all van der Waals bonds. For multidentate species, + vdW bonds are preserved when there are still other + covalent bonds with the surface present. If no covalent surface bonds are present, + all vdW bonds are removed. """ cython.declare(bond=Bond) if self.has_covalent_surface_bond(): @@ -3094,6 +3097,7 @@ def get_desorbed_molecules(self): ``*2`` - double bond ``*3`` - triple bond ``*4`` - quadruple bond + ``*0`` - vdW bond """ cython.declare(desorbed_molecules=list, desorbed_molecule=Molecule, sites_to_remove=list, adsorbed_atoms=list, site=Atom, numbonds=cython.int, bonded_atom=Atom, bond=Bond, i=cython.int, j=cython.int, atom0=Atom, @@ -3133,7 +3137,7 @@ def get_desorbed_molecules(self): bonded_atom.increment_lone_pairs() bonded_atom.label = '*4' elif bond.is_van_der_waals(): - bonded_atom.label = '*5' + bonded_atom.label = '*0' else: raise NotImplementedError("Can't remove surface bond of type {}".format(bond.order)) desorbed_molecule.remove_atom(site) From 707a2e494422122afa4361025fe9dd2dac46dd63 Mon Sep 17 00:00:00 2001 From: Bjarne Kreitz Date: Mon, 18 May 2026 17:39:08 -0400 Subject: [PATCH 12/27] add more constraints to formate path in resonance.py --- rmgpy/molecule/resonance.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/rmgpy/molecule/resonance.py b/rmgpy/molecule/resonance.py index 622e419921c..756c5e0a855 100644 --- a/rmgpy/molecule/resonance.py +++ b/rmgpy/molecule/resonance.py @@ -1262,9 +1262,9 @@ def generate_adsorbate_conjugate_resonance_structures(mol): def generate_formate_resonance_structures(mol): """ - Generate all of the resonance structures formed by the shift of two - electrons in a conjugated pi bond system of a bidentate adsorbate - with a bridging atom in between. + Generate all resonance structures formed by the shift of two + electrons in a conjugated bonding system of a bidentate adsorbate + with a bridging atom in between, where one bond to the surface is vdW. Example [X]OC(H)O[X]: [X]~OC(H)O[X] <=> [X]OC(H)O~[X] (where '~' denotes a vdW bond) @@ -1279,7 +1279,8 @@ def generate_formate_resonance_structures(mol): paths = pathfinder.find_formate_delocalization_paths(atom) for atom1, atom2, atom3, atom4, atom5, bond12, bond23, bond34, bond45 in paths: if ((atom2.is_oxygen() and bond12.is_van_der_waals()) and - (atom4.is_oxygen() and atom5.is_surface_site() and bond45.is_single())): + (atom4.is_oxygen() and atom5.is_surface_site() and + bond45.is_single() and bond23.is_double() and bond34.is_single())): bond12.increment_order() bond23.decrement_order() bond34.increment_order() From 9a41f934d7992c3c96b1f6a839a62f7d6537bc17 Mon Sep 17 00:00:00 2001 From: Bjarne Kreitz Date: Mon, 18 May 2026 17:40:55 -0400 Subject: [PATCH 13/27] clean up pathfinder and add constraints --- rmgpy/molecule/pathfinder.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rmgpy/molecule/pathfinder.py b/rmgpy/molecule/pathfinder.py index 33756be3ebd..11a65a5c2d2 100644 --- a/rmgpy/molecule/pathfinder.py +++ b/rmgpy/molecule/pathfinder.py @@ -556,10 +556,10 @@ def find_adsorbate_conjugate_delocalization_paths(atom1): def find_formate_delocalization_paths(atom1): """ - Find all resonance structures which have a bonding configuration X...O-C-O-X. + Find all resonance structures which have a bonding configuration X~O=C-O-X. Examples: - - XOC(H)XO/XOC(H)XO, where X is the surface site. The adsorption site X + - [X]~OC(H)O[X]/[X]OC(H)O~[X], where '~' denotes a vdW bond and X is the surface site. The adsorption site X is always placed on the left-hand side of the adatom and every adatom is bonded to only one surface site X. @@ -583,10 +583,10 @@ def find_formate_delocalization_paths(atom1): for atom2, bond12 in atom1.edges.items(): if atom2.is_oxygen() and bond12.is_van_der_waals(): for atom3, bond23 in atom2.edges.items(): - if atom3.is_carbon(): + if atom3.is_carbon() and bond23.is_double(): for atom4, bond34 in atom3.edges.items(): - if atom2 is not atom4 and atom4.is_oxygen(): + if atom2 is not atom4 and atom4.is_oxygen() and bond34.is_single(): for atom5, bond45 in atom4.edges.items(): - if atom5.is_surface_site(): + if atom5.is_surface_site() and bond45.is_single(): paths.append([atom1, atom2, atom3, atom4, atom5, bond12, bond23, bond34, bond45]) return paths \ No newline at end of file From 0183e97da0d3ec989b3a31b1c198072ac4da9e38 Mon Sep 17 00:00:00 2001 From: Bjarne Kreitz Date: Mon, 18 May 2026 17:56:02 -0400 Subject: [PATCH 14/27] add features for N species --- rmgpy/molecule/pathfinder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rmgpy/molecule/pathfinder.py b/rmgpy/molecule/pathfinder.py index 11a65a5c2d2..dc4d9eeeb59 100644 --- a/rmgpy/molecule/pathfinder.py +++ b/rmgpy/molecule/pathfinder.py @@ -583,7 +583,7 @@ def find_formate_delocalization_paths(atom1): for atom2, bond12 in atom1.edges.items(): if atom2.is_oxygen() and bond12.is_van_der_waals(): for atom3, bond23 in atom2.edges.items(): - if atom3.is_carbon() and bond23.is_double(): + if (atom3.is_carbon() or atom3.is_nitrogen()) and bond23.is_double(): for atom4, bond34 in atom3.edges.items(): if atom2 is not atom4 and atom4.is_oxygen() and bond34.is_single(): for atom5, bond45 in atom4.edges.items(): From 31c9f956d1258508c3ed32e68552389e40999fb8 Mon Sep 17 00:00:00 2001 From: Richard West Date: Tue, 19 May 2026 08:55:30 -0400 Subject: [PATCH 15/27] Consider generate_formate_resonance_structures when no features specified. Suggested by copilot: Previously, populate_resonance_algorithms() adds generate_formate_resonance_structures only in the features['is_multidentate'] branch, but it is not included in the default (features=None) method list. This means call sites that iterate populate_resonance_algorithms() without passing features (e.g., the isomorphic-resonance enumeration later in this module) will never generate the new formate resonance structures. Consider adding the new generator to the features is None list as well to keep behavior consistent. --- rmgpy/molecule/resonance.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rmgpy/molecule/resonance.py b/rmgpy/molecule/resonance.py index 756c5e0a855..d9d17b33341 100644 --- a/rmgpy/molecule/resonance.py +++ b/rmgpy/molecule/resonance.py @@ -96,7 +96,8 @@ def populate_resonance_algorithms(features=None): generate_clar_structures, generate_adsorbate_shift_down_resonance_structures, generate_adsorbate_shift_up_resonance_structures, - generate_adsorbate_conjugate_resonance_structures + generate_adsorbate_conjugate_resonance_structures, + generate_formate_resonance_structures, ] else: # If the molecule is aromatic, then radical resonance has already been considered From bf70032ca7d40051160a57d97cc4f7bef8a96a96 Mon Sep 17 00:00:00 2001 From: Richard West Date: Tue, 19 May 2026 08:56:53 -0400 Subject: [PATCH 16/27] Rename generate_formate_resonance_structures to generate_adsorbate_formate_resonance_structures Just so all the adsorbate things start with similar names. --- rmgpy/molecule/resonance.pxd | 2 +- rmgpy/molecule/resonance.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/rmgpy/molecule/resonance.pxd b/rmgpy/molecule/resonance.pxd index e0b0381d646..47aa3c01c18 100644 --- a/rmgpy/molecule/resonance.pxd +++ b/rmgpy/molecule/resonance.pxd @@ -74,4 +74,4 @@ cpdef list generate_adsorbate_shift_up_resonance_structures(Graph mol) cpdef list generate_adsorbate_conjugate_resonance_structures(Graph mol) -cpdef list generate_formate_resonance_structures(Graph mol) +cpdef list generate_adsorbate_formate_resonance_structures(Graph mol) diff --git a/rmgpy/molecule/resonance.py b/rmgpy/molecule/resonance.py index d9d17b33341..23fc598684f 100644 --- a/rmgpy/molecule/resonance.py +++ b/rmgpy/molecule/resonance.py @@ -97,7 +97,7 @@ def populate_resonance_algorithms(features=None): generate_adsorbate_shift_down_resonance_structures, generate_adsorbate_shift_up_resonance_structures, generate_adsorbate_conjugate_resonance_structures, - generate_formate_resonance_structures, + generate_adsorbate_formate_resonance_structures, ] else: # If the molecule is aromatic, then radical resonance has already been considered @@ -125,7 +125,7 @@ def populate_resonance_algorithms(features=None): method_list.append(generate_adsorbate_shift_down_resonance_structures) method_list.append(generate_adsorbate_shift_up_resonance_structures) method_list.append(generate_adsorbate_conjugate_resonance_structures) - method_list.append(generate_formate_resonance_structures) + method_list.append(generate_adsorbate_formate_resonance_structures) return method_list @@ -1261,7 +1261,7 @@ def generate_adsorbate_conjugate_resonance_structures(mol): return structures -def generate_formate_resonance_structures(mol): +def generate_adsorbate_formate_resonance_structures(mol): """ Generate all resonance structures formed by the shift of two electrons in a conjugated bonding system of a bidentate adsorbate From d0c5940367428ff5c39950cde84551630350d46f Mon Sep 17 00:00:00 2001 From: Richard West Date: Tue, 19 May 2026 09:11:55 -0400 Subject: [PATCH 17/27] Test the new remove_van_der_waals_bonds behaviour. Should NOT remove vdW if there's also a covalent bond to the surface. --- test/rmgpy/molecule/moleculeTest.py | 45 ++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/test/rmgpy/molecule/moleculeTest.py b/test/rmgpy/molecule/moleculeTest.py index 68f327e723c..fe696255fa2 100644 --- a/test/rmgpy/molecule/moleculeTest.py +++ b/test/rmgpy/molecule/moleculeTest.py @@ -3098,7 +3098,7 @@ def test_count_aromatic_rings(self): assert result == [2, 1, 0] - def test_remove_van_der_waals_bonds(self): + def test_remove_van_der_waals_bond(self): """Test we can remove a van-der-Waals bond""" adjlist = """ 1 X u0 p0 c0 {2,vdW} @@ -3110,6 +3110,49 @@ def test_remove_van_der_waals_bonds(self): mol.remove_van_der_waals_bonds() assert len(mol.get_all_edges()) == 1 + def test_remove_van_der_waals_bonds_bidentate(self): + """vdW bonds are preserved on bidentates that also have a covalent X bond, removed otherwise.""" + # Bidentate ethyl-like fragment: one C–X covalent, one C~X vdW. The vdW should be preserved. + mixed = Molecule().from_adjacency_list( + """ +1 X u0 p0 c0 {3,S} +2 X u0 p0 c0 {4,vdW} +3 C u0 p0 c0 {1,S} {4,S} {5,S} {6,S} +4 C u0 p0 c0 {2,vdW} {3,S} {7,S} {8,S} {9,S} +5 H u0 p0 c0 {3,S} +6 H u0 p0 c0 {3,S} +7 H u0 p0 c0 {4,S} +8 H u0 p0 c0 {4,S} +9 H u0 p0 c0 {4,S} +""" + ) + assert mixed.has_covalent_surface_bond() + n_edges_before = len(mixed.get_all_edges()) + mixed.remove_van_der_waals_bonds() + assert len(mixed.get_all_edges()) == n_edges_before # nothing removed + assert any(bond.is_van_der_waals() for bond in mixed.get_all_edges()) + + # Bidentate physisorbed: both C~X bonds are vdW. All vdW bonds should be removed. + vdw_only = Molecule().from_adjacency_list( + """ +1 X u0 p0 c0 {3,vdW} +2 X u0 p0 c0 {4,vdW} +3 C u0 p0 c0 {1,vdW} {4,S} {5,S} {6,S} {7,S} +4 C u0 p0 c0 {2,vdW} {3,S} {8,S} {9,S} {10,S} +5 H u0 p0 c0 {3,S} +6 H u0 p0 c0 {3,S} +7 H u0 p0 c0 {3,S} +8 H u0 p0 c0 {4,S} +9 H u0 p0 c0 {4,S} +10 H u0 p0 c0 {4,S} +""" + ) + assert not vdw_only.has_covalent_surface_bond() + n_edges_before = len(vdw_only.get_all_edges()) + vdw_only.remove_van_der_waals_bonds() + assert len(vdw_only.get_all_edges()) == n_edges_before - 2 + assert not any(bond.is_van_der_waals() for bond in vdw_only.get_all_edges()) + def test_has_covalent_surface_bond(self): """Test Molecule.has_covalent_surface_bond() distinguishes vdW from covalent X bonds.""" # X present but only physisorbed via a vdW bond From 1a8cc772541cd033db3b945a818e282798e0846a Mon Sep 17 00:00:00 2001 From: Richard West Date: Tue, 19 May 2026 09:54:05 -0400 Subject: [PATCH 18/27] Add comments describing current implementation of is_molecule_forbidden For checking multi-dentate vdW things. I'm about to refactor, and want to first document what the current code does, then change how we do it in the next commit. --- rmgpy/data/kinetics/family.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rmgpy/data/kinetics/family.py b/rmgpy/data/kinetics/family.py index c9c499f6f1b..1d27070b5a2 100644 --- a/rmgpy/data/kinetics/family.py +++ b/rmgpy/data/kinetics/family.py @@ -1678,11 +1678,15 @@ def is_molecule_forbidden(self, molecule): # forbid vdw multi-dentate molecules for surface families surface_sites = [] if "surface" in self.label.lower(): + # Within the surface_monodentate_to_vdw_bidentate, allow (don't forbid) + # vdW in multi-dentate molecules if at least one bond to the surface + # is covalent (not vdW). if "surface_monodentate_to_vdw_bidentate" in self.label.lower() and molecule.get_num_atoms('X') > 1: surface_sites = [atom.atomtype.label for atom in molecule.atoms if 'X' in atom.atomtype.label] if all(site == 'Xv' for site in surface_sites): return True else: + # for all other families, forbid multi-dentate molecules with any vdW bonds if molecule.get_num_atoms('X') > 1: for atom in molecule.atoms: if atom.atomtype.label == 'Xv': From eb78197df17d2c11d36cd245f42e4a3d150e1d48 Mon Sep 17 00:00:00 2001 From: Richard West Date: Tue, 19 May 2026 10:33:01 -0400 Subject: [PATCH 19/27] Refactor is_molecule_forbidden checks in reaction family. Lifts is_multidentate() into the outer guard (short-circuits on the second surface atom, so it's cheaper than get_num_atoms('X') > 1 which counts all atoms). Replaces the surface_sites list + all(... == 'Xv') check with `not has_covalent_surface_bond()`. Drops the now-unused surface_sites = [] initializer. --- rmgpy/data/kinetics/family.py | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/rmgpy/data/kinetics/family.py b/rmgpy/data/kinetics/family.py index 1d27070b5a2..03e00bd7480 100644 --- a/rmgpy/data/kinetics/family.py +++ b/rmgpy/data/kinetics/family.py @@ -1676,21 +1676,18 @@ def is_molecule_forbidden(self, molecule): return True # forbid vdw multi-dentate molecules for surface families - surface_sites = [] - if "surface" in self.label.lower(): - # Within the surface_monodentate_to_vdw_bidentate, allow (don't forbid) - # vdW in multi-dentate molecules if at least one bond to the surface - # is covalent (not vdW). - if "surface_monodentate_to_vdw_bidentate" in self.label.lower() and molecule.get_num_atoms('X') > 1: - surface_sites = [atom.atomtype.label for atom in molecule.atoms if 'X' in atom.atomtype.label] - if all(site == 'Xv' for site in surface_sites): + if "surface" in self.label.lower() and molecule.is_multidentate(): + if "surface_monodentate_to_vdw_bidentate" in self.label.lower(): + # Within the surface_monodentate_to_vdw_bidentate family, allow + # (don't forbid) vdW in multi-dentate molecules if at least one + # bond to the surface is covalent (not vdW). + if not molecule.has_covalent_surface_bond(): return True else: # for all other families, forbid multi-dentate molecules with any vdW bonds - if molecule.get_num_atoms('X') > 1: - for atom in molecule.atoms: - if atom.atomtype.label == 'Xv': - return True + for atom in molecule.atoms: + if atom.atomtype.label == 'Xv': + return True return False From 4ab11a902c8aa794552c1057d7063b2c4453a5db Mon Sep 17 00:00:00 2001 From: Richard West Date: Tue, 19 May 2026 10:44:47 -0400 Subject: [PATCH 20/27] Add has_vdw_surface_bond method on Molecule. Also add tests for it, and use it in reaction family is_molecule_forbidden method. --- rmgpy/data/kinetics/family.py | 5 ++-- rmgpy/molecule/molecule.pxd | 2 ++ rmgpy/molecule/molecule.py | 12 +++++++++ test/rmgpy/molecule/moleculeTest.py | 38 +++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 3 deletions(-) diff --git a/rmgpy/data/kinetics/family.py b/rmgpy/data/kinetics/family.py index 03e00bd7480..5600904118e 100644 --- a/rmgpy/data/kinetics/family.py +++ b/rmgpy/data/kinetics/family.py @@ -1685,9 +1685,8 @@ def is_molecule_forbidden(self, molecule): return True else: # for all other families, forbid multi-dentate molecules with any vdW bonds - for atom in molecule.atoms: - if atom.atomtype.label == 'Xv': - return True + if molecule.has_vdw_surface_bond(): + return True return False diff --git a/rmgpy/molecule/molecule.pxd b/rmgpy/molecule/molecule.pxd index 219bb6acb3f..6bbbef015a5 100644 --- a/rmgpy/molecule/molecule.pxd +++ b/rmgpy/molecule/molecule.pxd @@ -208,6 +208,8 @@ cdef class Molecule(Graph): cpdef bint has_covalent_surface_bond(self) + cpdef bint has_vdw_surface_bond(self) + cpdef bint is_surface_site(self) cpdef remove_atom(self, Atom atom) diff --git a/rmgpy/molecule/molecule.py b/rmgpy/molecule/molecule.py index 048f26f8734..f28a08e73ae 100644 --- a/rmgpy/molecule/molecule.py +++ b/rmgpy/molecule/molecule.py @@ -1231,6 +1231,18 @@ def has_covalent_surface_bond(self): return True return False + def has_vdw_surface_bond(self): + """ + Return True if any bond in this molecule connects a surface site (X) via a van der Waals bond. + """ + cython.declare(atom=Atom, bond=Bond) + for atom in self.atoms: + if atom.is_surface_site(): + for bond in atom.bonds.values(): + if bond.is_van_der_waals(): + return True + return False + def contains_surface_site(self): """ Returns ``True`` iff the molecule contains an 'X' surface site. diff --git a/test/rmgpy/molecule/moleculeTest.py b/test/rmgpy/molecule/moleculeTest.py index fe696255fa2..738f085fbce 100644 --- a/test/rmgpy/molecule/moleculeTest.py +++ b/test/rmgpy/molecule/moleculeTest.py @@ -3191,6 +3191,44 @@ def test_has_covalent_surface_bond(self): gas = Molecule().from_smiles("CCO") assert not gas.has_covalent_surface_bond() + def test_has_vdw_surface_bond(self): + """Test Molecule.has_vdw_surface_bond() detects any vdW X bond.""" + # X present but only physisorbed via a vdW bond + vdw_only = Molecule().from_adjacency_list( + """ +1 X u0 p0 c0 {2,vdW} +2 H u0 p0 c0 {1,vdW} {3,S} +3 H u0 p0 c0 {2,S} +""" + ) + assert vdw_only.has_vdw_surface_bond() + + # X covalently bonded (chemisorbed) — no vdW bond + chemisorbed = Molecule().from_adjacency_list( + """ +1 H u0 p0 c0 {2,S} +2 X u0 p0 c0 {1,S} +""" + ) + assert not chemisorbed.has_vdw_surface_bond() + + # Two X atoms: one vdW, one covalent + mixed = Molecule().from_adjacency_list( + """ +1 X u0 p0 c0 {3,S} +2 X u0 p0 c0 {4,vdW} +3 C u0 p0 c0 {1,S} {4,S} {5,S} {6,S} +4 H u0 p0 c0 {2,vdW} {3,S} +5 H u0 p0 c0 {3,S} +6 H u0 p0 c0 {3,S} +""" + ) + assert mixed.has_vdw_surface_bond() + + # No surface sites at all + gas = Molecule().from_smiles("CCO") + assert not gas.has_vdw_surface_bond() + def test_get_relevant_cycles(self): """ Test the Molecule.get_relevant_cycles() raises correct error after deprecation. From 1d34efa52d3520667eabe9aabcffee976b56306f Mon Sep 17 00:00:00 2001 From: Richard West Date: Tue, 19 May 2026 22:16:11 -0400 Subject: [PATCH 21/27] Fix bug and add tests: unbonded X gives True for has_vdw_surface_bond The has_vdw_surface_bond now ALSO returns True for has LACK OF a bond (but an X and one other thing). I guess if you were to give it an adjacency list with just X atoms, this would return True, which may seem weird. (But so would the old test which was just for the presence of an Xv atom type) --- rmgpy/molecule/molecule.py | 8 +++++++- test/rmgpy/molecule/moleculeTest.py | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/rmgpy/molecule/molecule.py b/rmgpy/molecule/molecule.py index f28a08e73ae..f18b4feaa40 100644 --- a/rmgpy/molecule/molecule.py +++ b/rmgpy/molecule/molecule.py @@ -1233,14 +1233,20 @@ def has_covalent_surface_bond(self): def has_vdw_surface_bond(self): """ - Return True if any bond in this molecule connects a surface site (X) via a van der Waals bond. + Return True if any bond in this molecule connects a surface site (X) + via a van der Waals bond, or there's a surface site with no bonds + (but at least one other atom in the molecule). """ cython.declare(atom=Atom, bond=Bond) for atom in self.atoms: if atom.is_surface_site(): + if not atom.bonds: # if there are no bonds at all + if len(self.atoms) > 1: # and there's something besides the surface site + return True # then treat as vdW bonded for bond in atom.bonds.values(): if bond.is_van_der_waals(): return True + return False def contains_surface_site(self): diff --git a/test/rmgpy/molecule/moleculeTest.py b/test/rmgpy/molecule/moleculeTest.py index 738f085fbce..38447d63107 100644 --- a/test/rmgpy/molecule/moleculeTest.py +++ b/test/rmgpy/molecule/moleculeTest.py @@ -3229,6 +3229,20 @@ def test_has_vdw_surface_bond(self): gas = Molecule().from_smiles("CCO") assert not gas.has_vdw_surface_bond() + # An unbonded X atom counts as a vdW surface bond + unbonded = Molecule().from_adjacency_list( + """ +1 X u0 p0 c0 +2 H u0 p0 c0 {3,S} +3 H u0 p0 c0 {2,S} +""" + ) + assert unbonded.has_vdw_surface_bond() + + # vacant site alone is not a vdW surface bond + vacant = Molecule().from_adjacency_list("1 X u0 p0 c0") + assert not vacant.has_vdw_surface_bond() + def test_get_relevant_cycles(self): """ Test the Molecule.get_relevant_cycles() raises correct error after deprecation. From fccfdc7a183237d30d1ea2d42639862f6a71d850 Mon Sep 17 00:00:00 2001 From: Richard West Date: Tue, 19 May 2026 22:17:13 -0400 Subject: [PATCH 22/27] Avoid calling fails_species_constraints() twice in a row. Since Python 3.8 we can do this "walrus" operator := --- rmgpy/data/kinetics/family.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rmgpy/data/kinetics/family.py b/rmgpy/data/kinetics/family.py index 5600904118e..9e8fea3e575 100644 --- a/rmgpy/data/kinetics/family.py +++ b/rmgpy/data/kinetics/family.py @@ -1655,8 +1655,7 @@ def _generate_product_structures(self, reactant_structures, maps, forward, relab for struct in product_structures: if self.is_molecule_forbidden(struct): raise ForbiddenStructureException() - reason = fails_species_constraints(struct) - if reason: + if (reason := fails_species_constraints(struct)): raise ForbiddenStructureException( "Species constraints forbids product species {0}. Please " "reformulate constraints, or explicitly " From b596c03cbf66f8458a5a92d72cd7f0ed36012c77 Mon Sep 17 00:00:00 2001 From: Bjarne Kreitz Date: Wed, 20 May 2026 21:50:13 -0400 Subject: [PATCH 23/27] restructure is_molecule_forbbiden --- rmgpy/data/kinetics/family.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/rmgpy/data/kinetics/family.py b/rmgpy/data/kinetics/family.py index 9e8fea3e575..33d7cd603e3 100644 --- a/rmgpy/data/kinetics/family.py +++ b/rmgpy/data/kinetics/family.py @@ -1676,7 +1676,13 @@ def is_molecule_forbidden(self, molecule): # forbid vdw multi-dentate molecules for surface families if "surface" in self.label.lower() and molecule.is_multidentate(): - if "surface_monodentate_to_vdw_bidentate" in self.label.lower(): + allowed_vdw_families = [ + "surface_monodentate_to_vdw_bidentate", + "surface_dissociation_vdw_bidentate", + "surface_dissociation_vdw_bidentate_beta", + "surface_abstraction_vdw_bidentate_beta" + ] + if any(name in self.label.lower() for name in allowed_vdw_families): # Within the surface_monodentate_to_vdw_bidentate family, allow # (don't forbid) vdW in multi-dentate molecules if at least one # bond to the surface is covalent (not vdW). From 109a9d0bb11b8c4402b6eb038e37393c51d65637 Mon Sep 17 00:00:00 2001 From: Bjarne Kreitz Date: Wed, 3 Jun 2026 13:38:35 -0400 Subject: [PATCH 24/27] restructure is_molecule_forbidden to check for vdWBidentate in family name --- rmgpy/data/kinetics/family.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/rmgpy/data/kinetics/family.py b/rmgpy/data/kinetics/family.py index 33d7cd603e3..fd38834f0c9 100644 --- a/rmgpy/data/kinetics/family.py +++ b/rmgpy/data/kinetics/family.py @@ -1676,16 +1676,10 @@ def is_molecule_forbidden(self, molecule): # forbid vdw multi-dentate molecules for surface families if "surface" in self.label.lower() and molecule.is_multidentate(): - allowed_vdw_families = [ - "surface_monodentate_to_vdw_bidentate", - "surface_dissociation_vdw_bidentate", - "surface_dissociation_vdw_bidentate_beta", - "surface_abstraction_vdw_bidentate_beta" - ] - if any(name in self.label.lower() for name in allowed_vdw_families): - # Within the surface_monodentate_to_vdw_bidentate family, allow - # (don't forbid) vdW in multi-dentate molecules if at least one - # bond to the surface is covalent (not vdW). + if "vdwbidentate" in self.label.lower(): + # Within vdWBidentate families, allow vdW in + # multi-dentate molecules if at least one bond to the surface + # is covalent. if not molecule.has_covalent_surface_bond(): return True else: From bdf7053e04d8414539745916131d21b028ac2251 Mon Sep 17 00:00:00 2001 From: Richard West Date: Wed, 3 Jun 2026 23:58:29 -0400 Subject: [PATCH 25/27] TEMPORARY: set RMG_DATABASE_BRANCH: formate_families drop this commit before merging to main. merge at same time as https://github.com/ReactionMechanismGenerator/RMG-database/pull/729 --- .github/workflows/CI.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index fee682f772f..4c3d695aad4 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -37,7 +37,7 @@ concurrency: env: # if running on RMG-Py but requiring changes on an un-merged branch of RMG-database, replace # main with the name of the branch - RMG_DATABASE_BRANCH: main + RMG_DATABASE_BRANCH: formate_families # RMS branch to use for ReactionMechanismSimulator installation RMS_BRANCH: for_rmg # RMS mode used for install_rms.sh From e1573dacab1836eccad22ab3ef7b1c1e345ff5f5 Mon Sep 17 00:00:00 2001 From: Richard West Date: Thu, 4 Jun 2026 10:53:16 -0400 Subject: [PATCH 26/27] Improve diagnostics for "Family had N reactants?" kinetics test failure The dispatch else-branch in kinetics_check_sample_can_react raised a terse ValueError ("Family had N reactants?: ...") that named neither the family nor the underlying cause, making CI failures hard to diagnose. This branch is reached whenever the number of reactant roots that produced at least one usable sample molecule does not match the family template, and it raised before the logging loop ran, discarding any per-sample errors already queued. Add an explicit check comparing the number of reactant roots that yielded samples against the family template's reactant count. On a mismatch, append a descriptive, per-root message logged together with the queued per-sample errors (and then raises the standard "Error Occurred. See log for details."), naming the family and the specific reactant root(s) with no usable sample. Co-Authored-By: Claude Opus 4.8 (1M context) --- test/database/databaseTest.py | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/test/database/databaseTest.py b/test/database/databaseTest.py index ff3e83f70e1..5e772bc46b6 100644 --- a/test/database/databaseTest.py +++ b/test/database/databaseTest.py @@ -1577,7 +1577,22 @@ def make_error_message(reactants, message=""): output += "\n" + s.to_adjacency_list(label=s.to_smiles()) return output - if len(sample_reactants) == 1 == len(family.forward_template.reactants): + expected_reactants = [str(r) for r in family.forward_template.reactants] + roots_with_samples = [str(k) for k in sample_reactants.keys()] + roots_without_samples = [r for r in expected_reactants if r not in roots_with_samples] + if len(sample_reactants) != len(family.forward_template.reactants): + # One or more reactant roots produced no usable sample molecule (every candidate was + # forbidden by is_molecule_forbidden, or raised UnexpectedChargeError/ + # ImplicitBenzeneError during make_sample_molecule). Record a descriptive error so it + # is logged below alongside any per-sample errors, instead of raising opaquely here. + test1.append( + f"In family {family_name}, {len(roots_without_samples)} reactant root(s) produced " + f"no usable sample molecule: {roots_without_samples}. The family template expects " + f"{len(expected_reactants)} reactant(s) {expected_reactants}; only these root(s) " + f"yielded samples: {roots_with_samples}. Check the group definitions for the " + f"missing reactant root(s)." + ) + elif len(sample_reactants) == 1: reactants = list(sample_reactants.values())[0] for reactant in reactants: try: @@ -1670,7 +1685,13 @@ def make_error_message(reactants, message=""): species = rmgpy.species.Species(index=1, molecule=[molecule]) species.generate_resonance_structures() else: - raise ValueError(f"Family had {len(sample_reactants)} reactants?: " f"{', '.join(map(str,sample_reactants.keys())) }") + # Reactant count matches the template but is not 1, 2, or 3 (RMG only supports up to + # trimolecular). This is not expected for any well-formed family. + raise ValueError( + f"In family {family_name}, the number of sampled reactant roots " + f"({len(sample_reactants)}) matches the template reactant count but is not 1, 2, " + f"or 3, which is unexpected: {roots_with_samples}." + ) # print out entries skipped from exception we can't currently handle if skipped: From 056291bb01b8a257d5861112e7d2d21bcda05251 Mon Sep 17 00:00:00 2001 From: JacksonBurns Date: Mon, 8 Jun 2026 14:42:12 -0400 Subject: [PATCH 27/27] Revert "TEMPORARY: set RMG_DATABASE_BRANCH: formate_families" This reverts commit 5e967e6598ed19b62b30ba90674d85943c4652f9. --- .github/workflows/CI.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 4c3d695aad4..fee682f772f 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -37,7 +37,7 @@ concurrency: env: # if running on RMG-Py but requiring changes on an un-merged branch of RMG-database, replace # main with the name of the branch - RMG_DATABASE_BRANCH: formate_families + RMG_DATABASE_BRANCH: main # RMS branch to use for ReactionMechanismSimulator installation RMS_BRANCH: for_rmg # RMS mode used for install_rms.sh