From 154ff88c191458235ed4b83cfdc807251993efc5 Mon Sep 17 00:00:00 2001 From: Calvin Pieters Date: Sat, 8 Aug 2026 22:14:17 +0300 Subject: [PATCH 1/4] Forward save_order to the dispatched resonance generation methods _generate_resonance_structures() accepted a save_order argument, used it for its own isomorphism checks, and then invoked every generation method as method(molecule), so each dispatched method fell back to its own save_order=False default. On the keep_isomorphic=True path this silently discarded the Clar structures of charge-separated polycyclic aromatics. 1-nitronaphthalene returned 3 structures instead of 7, 1-nitroanthracene 3 instead of 9, 1-nitropyrene 3 instead of 5, and 1-azidonaphthalene 2 instead of 4. The deletions are the dominant resonance contributors by Clar's rule, and they were selected by an accident of bookkeeping. filter_structures() runs charge_filtration(), which calls stabilize_charges_by_proximity() (arc/molecule/filtration.py:300); that heuristic ranks structures by walking pairs of charged atoms under the guard "atom2.sorting_label > atom1.sorting_label" (filtration.py:314). sorting_label defaults to -1 and is assigned by Molecule.sort_atoms(), which was reached only from to_rdkit_mol(save_order=False) (arc/molecule/converter.py:34) by way of get_aromatic_rings() - that is, only from inside the Clar step. So the Clar structures were the only ones whose labels were initialised, they were the only ones the guard let the heuristic measure, and it popped exactly them. Instrumenting 1-nitronaphthalene: 8 structures reach stabilize_charges_by_proximity(), the 4 with initialised sorting labels are precisely the 4 one-sextet Clar structures, and precisely those 4 are popped; none of the 3 survivors is a Clar structure. With save_order honoured no structure carries an initialised label, nothing is popped, and 4 of the 7 survivors are Clar structures. The loss also violated the de-duplication invariant n(keep_isomorphic=True) >= n(keep_isomorphic=False): 1-nitronaphthalene gave 3 < 4, 1-nitroanthracene 3 < 5 and 1-azidonaphthalene 2 < 4. The invariant is restored for all three. Thirteen distinct methods are reachable through this dispatch loop and 2 of them accept save_order: generate_clar_structures() and generate_aromatic_resonance_structure(), both dispatched by generate_resonance_structures() itself. The other 11 take only the molecule, so a bare method(molecule, save_order=save_order) would raise TypeError. generate_optimal_aromatic_resonance_structures() also accepts save_order but is not reachable here: it appears only in populate_resonance_algorithms(features=None), whose sole consumer is generate_isomorphic_resonance_structures(), and that invokes algo(isomer) directly rather than through this loop. _SAVE_ORDER_METHODS lists all three algorithms that accept the argument, not only the 2 this loop reaches, so that it reads as a complete inventory. Membership is a module-level tuple rather than a runtime signature inspection: test_save_order_aware_algorithms() derives both sides from the actual signatures and asserts set equality, so an algorithm that gains or loses save_order fails the test rather than drifting silently. The tuple also fails closed - a stale entry raises TypeError or reddens CI, whereas a signature probe that stopped returning real signatures, through a Cython binding=False switch or a decorator wrapping a generator, would answer False for every method and quietly reinstate this bug. This mirrors the fix for the identical defect in RMG-Py, which this module is vendored from and which still carries it at rmgpy/molecule/resonance.py:311, so the two implementations do not conflict on a re-vendor. The atom order matters because several consumers of the resonance structures are positional. ARCSpecies.set_mol_list() (arc/species/species.py:1084-1091) repairs the order with order_atoms_in_mol_list(), which is not a reliable backstop: update_molecule() rebuilds both molecules with every bond single and drops charges, radicals and lone pairs, so the isomorphism it solves sees only elements and connectivity. The two terminal nitro oxygens of 1-nitronaphthalene are equivalent under that reduction, so the repair can satisfy itself with a graph automorphism instead of the identity. Over 12 permuted input orders it reported success every time, and in 5 of them it left 2 of the 4 structures holding the O- at the index where the reference holds the neutral O=. Elements per index still matched, so no downstream element-and-connectivity check could notice. With save_order honoured the structures arrive already ordered and the repair is a no-op in all 12. set_mol_list() also replaces the whole resonance list with [self.mol] when the repair returns False (arc/species/species.py:1090-1091), so re-sorted structures could collapse mol_list to a single entry and cost rotor and conformer coverage. The other positional consumers are arc/species/conformers.py:222 and ARCSpecies.reconcile_mol_multiplicity() (arc/species/species.py:1534-1536), which adopts resonance[0]. Index 0 is not necessarily the input molecule: mark_unreactive_structures() promotes the input to index 0 only if it survived filtration, and otherwise appends it at the end with reactive=False. C[S+]([O-])SC and [O-][S+]=O both leave the input at index 1. ARCReaction.get_changed_bonds() is a fourth positional consumer, but branch fix_changed_bonds_resonance fixes it at the caller by locating its atoms via atom.id, so once both changes are on main they overlap completely at that one call site. The independent value of this change is the other three consumers and the restored Clar structures. filtration.filter_structures() is still called with a hard-coded save_order=True at generate_resonance_structures():281 rather than with the caller's value. That argument is inert there: filter_structures() only forwards it to mark_unreactive_structures(), which spends it on an is_isomorphic() call over deep copies, so it cannot reach the returned structures. Passing the caller's value instead produces no difference at all over 12 molecules x 2 caller values. ARC diverges from RMG-Py at this line and the divergence is left alone rather than churned. The structure sets are otherwise unchanged for aromatic species. Verified over aliphatics, aromatics, heteroaromatics, polycyclic aromatics and radicals: with keep_isomorphic=True the atom-ID-keyed fingerprints of the generated structures are identical for both values of save_order. That parity is not yet universal, and the remaining gap is a separate defect this change does not address. Vertex.sorting_label initialises to -1 and filtration.py compares labels with a strict >, so the two sorting_label-dependent heuristics see nothing to compare when no structure has been sorted. A handful of charged non-aromatics still return fewer structures with save_order=True than without -- [O]N=O 4 vs 2, C=N[O] 3 vs 2, [CH2]N=O 3 vs 2, NC=O 2 vs 1 -- and that behaviour is byte-identical on main, so it is pre-existing rather than introduced here. With keep_isomorphic=False the isomorphic de-duplication keeps whichever of two symmetry-equivalent structures happens to be generated first, so for naphthalene the representative of one isomorphism class differs between the two atom orders while the counts and the isomorphism classes agree; that is pre-existing behaviour and is unchanged on main. --- arc/molecule/resonance.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/arc/molecule/resonance.py b/arc/molecule/resonance.py index 87ee756ddb..1c71ad061e 100644 --- a/arc/molecule/resonance.py +++ b/arc/molecule/resonance.py @@ -271,6 +271,8 @@ def _generate_resonance_structures(mol_list, method_list, keep_isomorphic=False, if True, only remove structures that give is_identical=True copy: if False, append new resonance structures to the input list (default) if True, make a new list with all the resonance structures + save_order: if True, the atom order is maintained; it is forwarded to every method in + ``method_list`` that is listed in ``_SAVE_ORDER_METHODS`` """ cython.declare(index=cython.int, molecule=Graph, new_mol_list=list, new_mol=Graph, mol=Graph, input_charge=cython.int, x=Vertex) @@ -296,7 +298,10 @@ def _generate_resonance_structures(mol_list, method_list, keep_isomorphic=False, charge_span = molecule.get_charge_span() if octet_deviation <= min_octet_deviation + 2 and charge_span <= min_charge_span + 1: for method in method_list: - new_mol_list.extend(method(molecule)) + if method in _SAVE_ORDER_METHODS: + new_mol_list.extend(method(molecule, save_order=save_order)) + else: + new_mol_list.extend(method(molecule)) if octet_deviation < min_octet_deviation: # update min_octet_deviation to make this criterion tighter min_octet_deviation = octet_deviation @@ -960,6 +965,13 @@ def generate_clar_structures(mol, save_order=False): return mol_list +_SAVE_ORDER_METHODS = ( + generate_optimal_aromatic_resonance_structures, + generate_aromatic_resonance_structure, + generate_clar_structures, +) + + # Define this helper function at the module level (outside the cpdef method): def _aromatic_ring_sort_key(ring): sum_ids = 0 From b9eb91a26e56668f609f2335469f0d859d8c5aeb Mon Sep 17 00:00:00 2001 From: Calvin Pieters Date: Sat, 8 Aug 2026 22:14:17 +0300 Subject: [PATCH 2/4] Test that save_order and the Clar structures survive resonance generation The existing save_order tests use monocyclic aromatics, which never reach generate_clar_structures(), so they passed while the argument was being dropped. test_resonance_of_polycyclic_aromatics_without_changing_atom_order() covers naphthalene, 1-ethynylnaphthalene and anthracene. It pins the structure counts (3, 4 and 4 at keep_isomorphic=False), asserts that save_order=True leaves the atom IDs, elements and neighbour sets in the input order, and asserts that save_order=False does re-sort at least one structure, which is what fails on main. Structure-set preservation is the property that actually guards the change, and it is asserted on the atom-ID-keyed fingerprint - sorted (id, element, charge, radical electrons, lone pairs) per atom plus sorted (lower id, higher id, bond order) per bond - so it is independent of the atom order it is meant to police. It is asserted as exact set equality between save_order=True and save_order=False at keep_isomorphic=True. At keep_isomorphic=False the isomorphic de-duplication keeps whichever of two symmetry-equivalent structures is generated first, so for naphthalene the representative of one isomorphism class legitimately differs between the two atom orders; that path is therefore asserted as equal counts plus a pairwise isomorphism match instead. The weakening is confined to the symmetric case and is pre-existing behaviour on main. test_clar_structures_of_charge_separated_polycyclic_aromatics() is the regression test for the discarded Clar structures. 1-nitronaphthalene yields 7 structures at keep_isomorphic=True against 3 on main, and the test also pins the de-duplication invariant n(keep_isomorphic=True) >= n(keep_isomorphic=False), which main violates at 3 < 4. SaveOrderMethodsTest covers _SAVE_ORDER_METHODS, which is what decides whether save_order reaches a method at all. Both sides of the comparison are derived from the real signatures - the dispatchable algorithms being populate_resonance_algorithms() plus generate_aromatic_resonance_structure(), which is only ever reached through the hardcoded single-element method_list in generate_resonance_structures() - so neither direction of drift can pass. An algorithm that gains save_order without being listed would keep running on its own save_order=False default, and one that is listed but no longer accepts it would raise a TypeError that Molecule.generate_resonance_structures() catches, collapsing the result to a single structure. Dropping a member from the tuple and adding a non-accepting one were both confirmed to fail the assertion. --- arc/molecule/resonance_test.py | 82 +++++++++++++++++++++++++++++++++- 1 file changed, 80 insertions(+), 2 deletions(-) diff --git a/arc/molecule/resonance_test.py b/arc/molecule/resonance_test.py index 5668e08378..1200a9cd29 100644 --- a/arc/molecule/resonance_test.py +++ b/arc/molecule/resonance_test.py @@ -1,11 +1,29 @@ #!/usr/bin/env python3 # encoding: utf-8 +import inspect import unittest from arc.molecule.molecule import Molecule -from arc.molecule.resonance import (_clar_optimization, _clar_transformation, generate_clar_structures, - generate_kekule_structure, generate_optimal_aromatic_resonance_structures, generate_resonance_structures) +from arc.molecule.resonance import (_SAVE_ORDER_METHODS, _clar_optimization, _clar_transformation, + generate_aromatic_resonance_structure, generate_clar_structures, generate_kekule_structure, + generate_optimal_aromatic_resonance_structures, generate_resonance_structures, + populate_resonance_algorithms) + + +def resonance_fingerprint(mol): + """ + Return an atom order independent fingerprint of ``mol``, keyed on the atom IDs. + + The fingerprint is a tuple of a sorted tuple of + (id, element symbol, charge, radical electrons, lone pairs) per atom, + and a sorted tuple of (lower id, higher id, bond order) per bond. + """ + atoms = tuple(sorted((atom.id, atom.element.symbol, atom.charge, atom.radical_electrons, atom.lone_pairs) + for atom in mol.atoms)) + bonds = tuple(sorted((min(bond.atom1.id, bond.atom2.id), max(bond.atom1.id, bond.atom2.id), bond.order) + for bond in mol.get_all_edges())) + return atoms, bonds class ResonanceTest(unittest.TestCase): @@ -1160,6 +1178,66 @@ def test_resonance_without_changing_atom_order2(self): atom2_nb = {nb.id for nb in list(atom2.bonds.keys())} self.assertEqual(atom1_nb, atom2_nb) + def test_resonance_of_polycyclic_aromatics_without_changing_atom_order(self): + """Test that save_order is honored by the Clar structures of polycyclic aromatic molecules""" + for smiles, num_structures in [('c1ccc2ccccc2c1', 3), ('C#Cc1ccc2ccccc2c1', 4), ('c1ccc2cc3ccccc3cc2c1', 4)]: + mol = Molecule(smiles=smiles) + mol.assign_atom_ids() + original_order = [atom.id for atom in mol.atoms] + neighbors = [{neighbor.id for neighbor in atom.bonds.keys()} for atom in mol.atoms] + + ordered = generate_resonance_structures(mol.copy(deep=True), save_order=True) + self.assertEqual(len(ordered), num_structures) + for res_mol in ordered: + self.assertEqual([atom.id for atom in res_mol.atoms], original_order) + for index, atom in enumerate(res_mol.atoms): + self.assertEqual(atom.element.symbol, mol.atoms[index].element.symbol) + self.assertEqual({neighbor.id for neighbor in atom.bonds.keys()}, neighbors[index]) + + sorted_ = generate_resonance_structures(mol.copy(deep=True), save_order=False) + self.assertEqual(len(sorted_), num_structures) + self.assertTrue(any([atom.id for atom in res_mol.atoms] != original_order + for res_mol in sorted_)) + for res_mol in ordered: + self.assertTrue(any(res_mol.copy(deep=True).is_isomorphic(other.copy(deep=True)) + for other in sorted_)) + + kept_ordered = generate_resonance_structures(mol.copy(deep=True), keep_isomorphic=True, save_order=True) + kept_sorted = generate_resonance_structures(mol.copy(deep=True), keep_isomorphic=True, save_order=False) + self.assertEqual({resonance_fingerprint(res_mol) for res_mol in kept_ordered}, + {resonance_fingerprint(res_mol) for res_mol in kept_sorted}) + + def test_clar_structures_of_charge_separated_polycyclic_aromatics(self): + """Test that the Clar structures of a charge separated polycyclic aromatic survive filtration""" + mol = Molecule(smiles='[O-][N+](=O)c1cccc2ccccc12') + mol.assign_atom_ids() + + non_isomorphic = generate_resonance_structures(mol.copy(deep=True), keep_isomorphic=False) + identical = generate_resonance_structures(mol.copy(deep=True), keep_isomorphic=True) + + self.assertEqual(len(non_isomorphic), 4) + self.assertEqual(len(identical), 7) + self.assertGreaterEqual(len(identical), len(non_isomorphic)) + + ordered = generate_resonance_structures(mol.copy(deep=True), keep_isomorphic=True, save_order=True) + sorted_ = generate_resonance_structures(mol.copy(deep=True), keep_isomorphic=True, save_order=False) + self.assertEqual({resonance_fingerprint(res_mol) for res_mol in ordered}, + {resonance_fingerprint(res_mol) for res_mol in sorted_}) + + +class SaveOrderMethodsTest(unittest.TestCase): + """ + Contains unit tests for _SAVE_ORDER_METHODS. + """ + + def test_save_order_aware_algorithms(self): + """Test that _SAVE_ORDER_METHODS lists exactly the dispatchable algorithms taking save_order""" + dispatchable = set(populate_resonance_algorithms()) + dispatchable.add(generate_aromatic_resonance_structure) + accepts_save_order = {method for method in dispatchable + if 'save_order' in inspect.signature(method).parameters} + self.assertEqual(set(_SAVE_ORDER_METHODS), accepts_save_order) + class ClarTest(unittest.TestCase): """ From f2fa66e07abe8fb07b71f48ac05062f82fe184cb Mon Sep 17 00:00:00 2001 From: Calvin Pieters Date: Mon, 17 Aug 2026 14:00:34 +0300 Subject: [PATCH 3/4] Stop the charge filtration heuristics keying on sorting_label charge_filtration, find_unique_sites_in_charged_list and stabilize_charges_by_proximity identify atoms by sorting_label, which the isomorphism machinery leaves either unset or holding a stale permutation that no longer matches the vertex order. Under save_order the structures of one species arrive in a mixed state -- for 4-nitrophenoxy, 12 of the 18 structures reaching charge_filtration carry the unset value and the other 6 a stale permutation, against 18 of 18 correctly labelled without save_order. The proximity heuristic only counts a charged pair when atom2.sorting_label > atom1.sorting_label, so it measures a real distance for the few labelled structures and zero for the rest, then pops everything above the minimum. For charged aromatic radicals such as 4-nitrophenoxy that removed every aromatic structure. Use the atom's position in mol.vertices instead, via a new get_atom_indices, and drop the label comparison in stabilize_charges_by_proximity for itertools.combinations, guarding the disconnected case where find_shortest_path returns None. Over a 189-species corpus this takes the number of species whose distinct structures differ between the two save_order settings from 10 to 0, and removes a run-to-run nondeterminism: 4-nitrophenoxy under save_order=True returned 9 or 7 structures depending on the process and now returns 11 in every run, matching save_order=False. It also restores the three worked examples in this module's own docstring, all of which save_order=True contradicted: NO2 2 -> 4, CH2NO 2 -> 3, NH2CHO 1 -> 2. No structure is lost anywhere in the corpus, under either save_order setting. Two further corrections, both needed to keep that true: Parenthesise the multiple bond check in charge_filtration. index2 > index1 and bond.is_double() or bond.is_triple() parses as (A and B) or C, so a triple bond satisfied the condition from both sides and the reversed index pair was recorded as well (N#S recorded [(0, 1), (1, 0)]). find_unique_sites_in_charged_list, the only reader of mul_bond_sorting_list, looks up ordered pairs only, so the reversed entries were never queried and this changes no results; the sibling check in find_unique_sites_in_charged_list is already parenthesised, so the two read as if they disagree. Compare the similar-charge distance against the similar-charge maximum in stabilize_charges_by_proximity. The second pass tested distances[0], the cumulative opposite-charge distance, against the maximum of distances[1]. Rule 4 keeps the structures whose like charges are furthest apart, so it must test distances[1]. The mismatch was unreachable while the heuristic was inert; computing real distances makes it pop every structure of a salt whose opposite-charge pairs are all cross-fragment, so [Li+].[Li+].[O-][O-] and O=C([O-])[O-].[NH4+].[NH4+] raised ResonanceError. Corrected, both return a structure under either save_order setting, and no other species in the corpus changes. --- arc/molecule/filtration.py | 73 ++++++++++++++++++++++++-------------- 1 file changed, 46 insertions(+), 27 deletions(-) diff --git a/arc/molecule/filtration.py b/arc/molecule/filtration.py index aa7eaf8d00..2a4e863196 100644 --- a/arc/molecule/filtration.py +++ b/arc/molecule/filtration.py @@ -14,6 +14,8 @@ which is quite like http://www.chem.ucla.edu/~harding/IGOC/R/resonance_contributor_preference_rules.html) """ +import itertools + from arc.common import get_logger from arc.exceptions import ResonanceError from arc.molecule.element import PeriodicSystem @@ -206,16 +208,18 @@ def charge_filtration(filtered_list, charge_span_list): filtered_list = [filtered_mol for index, filtered_mol in enumerate(filtered_list) if charge_span_list[index] == min_charge_span] # the minimal charge span layer # Find the radical and multiple bond sites in all filtered_list structures: - rad_sorting_list = [] # sorting_label for radical sites - mul_bond_sorting_list = [] # sorting_label for multiple bond sites in the form of (atom1, atom2) tuples + rad_sorting_list = [] # atom indices for radical sites + mul_bond_sorting_list = [] # atom indices for multiple bond sites in the form of (atom1, atom2) tuples for mol in filtered_list: - for atom in mol.vertices: - if atom.radical_electrons and int(atom.sorting_label) not in rad_sorting_list: - rad_sorting_list.append(int(atom.sorting_label)) + indices = get_atom_indices(mol) + for index1, atom in enumerate(mol.vertices): + if atom.radical_electrons and index1 not in rad_sorting_list: + rad_sorting_list.append(index1) for atom2, bond in atom.bonds.items(): + index2 = indices[id(atom2)] # check if bond is multiple, store only from one side (atom1 < atom2) for consistency - if atom2.sorting_label > atom.sorting_label and bond.is_double() or bond.is_triple(): - mul_bond_sorting_list.append((int(atom.sorting_label), int(atom2.sorting_label))) + if index2 > index1 and (bond.is_double() or bond.is_triple()): + mul_bond_sorting_list.append((index1, index2)) # Find unique radical and multiple bond sites in charged_list and append to unique_charged_list: unique_charged_list = [] for mol in charged_list: @@ -240,16 +244,28 @@ def charge_filtration(filtered_list, charge_span_list): return filtered_list +def get_atom_indices(mol): + """ + Return a mapping of ``id(atom)`` to the atom's position in ``mol.vertices``. + + The mapping identifies an atom consistently across the structures of a single species only + while those structures share a common atom order. + """ + return {id(atom): index for index, atom in enumerate(mol.vertices)} + + def find_unique_sites_in_charged_list(mol, rad_sorting_list, mul_bond_sorting_list): """ A helper function for reactive site discovery in charged species """ - for atom in mol.vertices: - if atom.radical_electrons and int(atom.sorting_label) not in rad_sorting_list: + indices = get_atom_indices(mol) + for index1, atom in enumerate(mol.vertices): + if atom.radical_electrons and index1 not in rad_sorting_list: return [mol] for atom2, bond in atom.bonds.items(): - if (atom2.sorting_label > atom.sorting_label and (bond.is_double() or bond.is_triple()) - and (int(atom.sorting_label), int(atom2.sorting_label)) not in mul_bond_sorting_list + index2 = indices[id(atom2)] + if (index2 > index1 and (bond.is_double() or bond.is_triple()) + and (index1, index2) not in mul_bond_sorting_list and not (atom.is_sulfur() and atom2.is_sulfur())): # We check that both atoms aren't S, otherwise we get [S.-]=[S.+] as a structure of S2 triplet return [mol] @@ -301,6 +317,7 @@ def stabilize_charges_by_proximity(mol_list): """ Only keep structures that obey the charge proximity rule. Opposite charges will be as close as possible to one another, and vice versa. + Charged atoms in different molecular fragments have no path between them and are not counted. """ indices_to_pop = [] charge_distance_list = [] # indices match mol_list @@ -308,29 +325,31 @@ def stabilize_charges_by_proximity(mol_list): # Try finding well-defined pairs of formally-charged atoms to apply the proximity principle # (opposite charges will be as close as possible to one another, and vice versa) cumulative_opposite_charge_distance = cumulative_similar_charge_distance = 0 - for atom1 in mol.vertices: - if atom1.charge: - for atom2 in mol.vertices: - if atom2.charge and atom2.sorting_label > atom1.sorting_label: - # found two charged atoms - if (atom1.charge > 0) ^ (atom2.charge > 0): # xor - # they have opposing signs when ONLY one is positive - cumulative_opposite_charge_distance += len(find_shortest_path(atom1, atom2)) - else: - # they have similar signs - cumulative_similar_charge_distance += len(find_shortest_path(atom1, atom2)) + charged_atoms = [atom for atom in mol.vertices if atom.charge] + for atom1, atom2 in itertools.combinations(charged_atoms, 2): + # found two charged atoms + path = find_shortest_path(atom1, atom2) + if path is None: + continue + if (atom1.charge > 0) ^ (atom2.charge > 0): # xor + # they have opposing signs when ONLY one is positive + cumulative_opposite_charge_distance += len(path) + else: + # they have similar signs + cumulative_similar_charge_distance += len(path) charge_distance_list.append([cumulative_opposite_charge_distance, cumulative_similar_charge_distance]) - min_cumulative_opposite_charge_distance = min([distances[0] for distances in charge_distance_list] - or [0]) # in Python 3 use `min(list, default=0)` + min_cumulative_opposite_charge_distance = min((distances[0] for distances in charge_distance_list), + default=0) for i, distances in enumerate(charge_distance_list): # after generating the charge_distance_list, iterate through it and mark structures to pop if distances[0] > min_cumulative_opposite_charge_distance: indices_to_pop.append(i) - max_cumulative_similar_charge_distance = max([distances[1] for i, distances in - enumerate(charge_distance_list) if i not in indices_to_pop] or [0]) + max_cumulative_similar_charge_distance = max((distances[1] for i, distances in + enumerate(charge_distance_list) if i not in indices_to_pop), + default=0) for i, distances in enumerate(charge_distance_list): - if distances[0] < max_cumulative_similar_charge_distance: + if distances[1] < max_cumulative_similar_charge_distance: indices_to_pop.append(i) for i in reversed(range(len(mol_list))): # pop starting from the end, so indices won't change if i in indices_to_pop: From c31521e5f8d053b5b2b9d160fc3515e56ccfa0c1 Mon Sep 17 00:00:00 2001 From: Calvin Pieters Date: Mon, 17 Aug 2026 14:25:16 +0300 Subject: [PATCH 4/4] Test that the charge filtration heuristics ignore the atom order Covers the nitroaromatic radicals whose aromatic structures were dropped under save_order and the NO2 / CH2NO / NH2CHO examples this module's docstring documents, asserting the same structure count and the same number of aromatic structures for either save_order setting. Covers the salts whose charged atoms sit in different molecular fragments, which raised TypeError from the charge proximity heuristic or were filtered away entirely. Adds a unit test for get_atom_indices. --- arc/molecule/filtration_test.py | 54 +++++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/arc/molecule/filtration_test.py b/arc/molecule/filtration_test.py index 6c583fde90..9409b4f2fa 100644 --- a/arc/molecule/filtration_test.py +++ b/arc/molecule/filtration_test.py @@ -4,7 +4,8 @@ import unittest from arc.molecule.filtration import (get_octet_deviation_list, get_octet_deviation, filter_structures, - charge_filtration, get_charge_span_list, aromaticity_filtration) + charge_filtration, get_atom_indices, get_charge_span_list, + aromaticity_filtration) from arc.molecule.molecule import Molecule from arc.molecule.resonance import generate_resonance_structures, analyze_molecule @@ -126,7 +127,7 @@ def test_radical_site(self): Molecule().from_adjacency_list(adj3)] for mol in mol_list: - mol.update() # the charge_filtration uses the atom.sorting_label attribute + mol.update() filtered_list = charge_filtration(mol_list, get_charge_span_list(mol_list)) self.assertEqual(len(filtered_list), 2) @@ -213,7 +214,7 @@ def test_electronegativity(self): Molecule().from_adjacency_list(adj7)] for mol in mol_list: - mol.update() # the charge_filtration uses the atom.sorting_label attribute + mol.update() filtered_list = charge_filtration(mol_list, get_charge_span_list(mol_list)) self.assertEqual(len(filtered_list), 4) @@ -301,6 +302,53 @@ def test_aromaticity(self): filtered_list = aromaticity_filtration(mol_list, analyze_molecule(mol_list[0])) self.assertEqual(len(filtered_list), 3) + def test_get_atom_indices(self): + """Test that atoms are mapped to their position in mol.vertices""" + mol = Molecule().from_smiles('[O]N=O') + indices = get_atom_indices(mol) + self.assertEqual(len(indices), len(mol.vertices)) + for index, atom in enumerate(mol.vertices): + self.assertEqual(indices[id(atom)], index) + + def test_charge_filtration_independent_of_atom_order(self): + """Test that the charge filtration heuristics return the same structures for either atom order""" + for smiles, expected, aromatic in (('[O]c1ccc([N+](=O)[O-])cc1', 11, 2), + ('[CH2]c1ccc([N+](=O)[O-])cc1', 11, 2), + ('[O]c1ccccc1[N+](=O)[O-]', 11, 2), + ('[O]c1cccc([N+](=O)[O-])c1', 9, 2), + ('[O]N=O', 4, 0), + ('C=N[O]', 3, 0), + ('NC=O', 2, 0), + ): + sorted_list = generate_resonance_structures(Molecule().from_smiles(smiles), + keep_isomorphic=True, save_order=False) + saved_list = generate_resonance_structures(Molecule().from_smiles(smiles), + keep_isomorphic=True, save_order=True) + self.assertEqual(len(sorted_list), expected, msg=smiles) + self.assertEqual(len(saved_list), expected, msg=smiles) + for mol_list in (sorted_list, saved_list): + self.assertEqual(sum(1 for mol in mol_list + if any(bond.is_benzene() for bond in mol.get_all_edges())), + aromatic, msg=smiles) + + def test_charge_filtration_of_disconnected_ions(self): + """Test that a species whose charges sit in disconnected components is filtered + + Charged atoms in different molecular fragments have no path between them, so the charge + proximity heuristic does not count them, and a species whose only opposite-charge pairs + are of that kind is not filtered out entirely. + """ + for save_order in (False, True): + for smiles, expected in (('[Li+].[OH-]', 1), + ('[NH4+].[O-]N=O', 1), + ('O=C([O-])[O-].[NH4+].[NH4+]', 1), + ('[Li+].[Li+].[O-][O-]', 1), + ): + mol_list = generate_resonance_structures(Molecule().from_smiles(smiles), + save_order=save_order) + self.assertEqual(len(mol_list), expected, msg=smiles) + self.assertTrue(mol_list[0].get_charge_span()) + if __name__ == '__main__': unittest.main(testRunner=unittest.TextTestRunner(verbosity=2))