Skip to content

Skip 2D-graph isomorphism enforcement for TS species #876

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alongd merged 1 commit into from
May 9, 2026
Merged

Skip 2D-graph isomorphism enforcement for TS species #876

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions arc/scheduler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3099,9 +3099,12 @@ def check_directed_scan(self, label, pivots, scan, energies):
# a lower conformation was found
deg_increment = actions[1]
self.species_dict[label].set_dihedral(scan=scan, index=1, deg_increment=deg_increment)
is_isomorphic = self.species_dict[label].check_xyz_isomorphism(
allow_nonisomorphic_2d=self.allow_nonisomorphic_2d,
xyz=self.species_dict[label].initial_xyz)
if self.species_dict[label].is_ts:
is_isomorphic = True
else:
is_isomorphic = self.species_dict[label].check_xyz_isomorphism(
allow_nonisomorphic_2d=self.allow_nonisomorphic_2d,
xyz=self.species_dict[label].initial_xyz)
if is_isomorphic:
self.delete_all_species_jobs(label)
# Remove all completed rotor calculation information
Expand Down Expand Up @@ -3161,7 +3164,10 @@ def check_directed_scan_job(self, label: str, job: JobAdapter):
"""
if job.job_status[1]['status'] == 'done':
xyz = parser.parse_geometry(log_file_path=job.local_path_to_output_file)
is_isomorphic = self.species_dict[label].check_xyz_isomorphism(xyz=xyz, verbose=False)
if self.species_dict[label].is_ts:
is_isomorphic = True
else:
is_isomorphic = self.species_dict[label].check_xyz_isomorphism(xyz=xyz, verbose=False)
for rotor_dict in self.species_dict[label].rotors_dict.values():
if rotor_dict['pivots'] == job.pivots:
key = tuple(f'{dihedral:.2f}' for dihedral in job.dihedrals)
Expand Down Expand Up @@ -3394,9 +3400,12 @@ def troubleshoot_scan_job(self,
break
else:
# If 'change conformer' is not used, check for isomorphism.
is_isomorphic = self.species_dict[label].check_xyz_isomorphism(
allow_nonisomorphic_2d=self.allow_nonisomorphic_2d,
xyz=new_xyz)
if self.species_dict[label].is_ts:
is_isomorphic = True
else:
is_isomorphic = self.species_dict[label].check_xyz_isomorphism(
allow_nonisomorphic_2d=self.allow_nonisomorphic_2d,
xyz=new_xyz)
if is_isomorphic:
self.species_dict[label].final_xyz = new_xyz
# Remove all completed rotor calculation information.
Expand Down
152 changes: 151 additions & 1 deletion arc/scheduler_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"""

import unittest
from unittest.mock import patch
from unittest.mock import MagicMock, patch
import os
import shutil

Expand Down Expand Up @@ -1033,6 +1033,156 @@ def test_run_sp_monoatomic_dlpno(self, mock_run_job):
self.assertEqual(o_level.method, 'dlpno-ccsd(t)-f12')
self.assertEqual(o_level.cabs, 'cc-pvtz-f12-cabs')

def test_check_directed_scan_job_skips_isomorphism_for_ts(self):
"""check_directed_scan_job must not call check_xyz_isomorphism for a TS; is_isomorphic is recorded as True."""
ts_xyz = str_to_xyz("""N 0.91779059 0.51946178 0.00000000
H 1.81402049 1.03819414 0.00000000
H 0.00000000 0.00000000 0.00000000
H 0.91779059 1.22790192 0.72426890""")
ts_spc = ARCSpecies(label='TS_dirscan', is_ts=True, xyz=ts_xyz, multiplicity=1, charge=0,
compute_thermo=False)
ts_spc.rotors_dict = {0: {'pivots': [1, 2], 'directed_scan': {}}}

project_directory = os.path.join(ARC_PATH, 'Projects', 'arc_project_ts_iso_dirscan')
self.addCleanup(shutil.rmtree, project_directory, ignore_errors=True)
sched = Scheduler(project='test_ts_iso_dirscan', ess_settings=self.ess_settings,
species_list=[ts_spc],
opt_level=Level(repr=default_levels_of_theory['opt']),
freq_level=Level(repr=default_levels_of_theory['freq']),
sp_level=Level(repr=default_levels_of_theory['sp']),
ts_guess_level=Level(repr=default_levels_of_theory['ts_guesses']),
project_directory=project_directory,
testing=True,
job_types=self.job_types1,
)

job_mock = MagicMock()
job_mock.job_status = [None, {'status': 'done'}]
job_mock.local_path_to_output_file = '/fake/path.log'
job_mock.pivots = [1, 2]
job_mock.dihedrals = [45.0]
job_mock.ess_trsh_methods = []

with patch('arc.species.species.ARCSpecies.check_xyz_isomorphism') as mock_iso, \
patch('arc.scheduler.parser.parse_geometry', return_value=ts_xyz), \
patch('arc.scheduler.parser.parse_e_elect', return_value=-123.45):
sched.check_directed_scan_job(label='TS_dirscan', job=job_mock)

mock_iso.assert_not_called()
recorded = sched.species_dict['TS_dirscan'].rotors_dict[0]['directed_scan'][('45.00',)]
self.assertTrue(recorded['is_isomorphic'])

@patch('arc.scheduler.Scheduler.run_opt_job')
def test_troubleshoot_scan_job_skips_isomorphism_for_ts(self, mock_run_opt):
Comment thread
alongd marked this conversation as resolved.
"""troubleshoot_scan_job must not call check_xyz_isomorphism for a TS when applying 'change conformer'."""
ts_xyz = str_to_xyz("""N 0.91779059 0.51946178 0.00000000
H 1.81402049 1.03819414 0.00000000
H 0.00000000 0.00000000 0.00000000
H 0.91779059 1.22790192 0.72426890""")
new_xyz = str_to_xyz("""N 0.91000000 0.52000000 0.00000000
H 1.81000000 1.04000000 0.00000000
H 0.00000000 0.00000000 0.00000000
H 0.91000000 1.23000000 0.72000000""")
ts_spc = ARCSpecies(label='TS_trsh', is_ts=True, xyz=ts_xyz, multiplicity=1, charge=0,
compute_thermo=False)
ts_spc.rotors_dict = {0: {'pivots': [1, 2], 'scan': [3, 1, 2, 4], 'scan_path': '',
'invalidation_reason': '', 'success': None, 'symmetry': None,
'times_dihedral_set': 0, 'trsh_methods': [], 'trsh_counter': 0}}

project_directory = os.path.join(ARC_PATH, 'Projects', 'arc_project_ts_iso_trsh')
self.addCleanup(shutil.rmtree, project_directory, ignore_errors=True)
sched = Scheduler(project='test_ts_iso_trsh', ess_settings=self.ess_settings,
species_list=[ts_spc],
opt_level=Level(repr=default_levels_of_theory['opt']),
freq_level=Level(repr=default_levels_of_theory['freq']),
sp_level=Level(repr=default_levels_of_theory['sp']),
ts_guess_level=Level(repr=default_levels_of_theory['ts_guesses']),
project_directory=project_directory,
testing=True,
job_types=self.job_types1,
)
sched.trsh_ess_jobs = True
sched.trsh_rotors = True

job_mock = MagicMock()
job_mock.species_label = 'TS_trsh'
job_mock.rotor_index = 0
job_mock.torsions = [[3, 1, 2, 4]]
job_mock.job_name = 'scan_a200'

with patch('arc.species.species.ARCSpecies.check_xyz_isomorphism') as mock_iso, \
patch('arc.scheduler.Scheduler.delete_all_species_jobs'):
sched.troubleshoot_scan_job(job=job_mock, methods={'change conformer': new_xyz})

mock_iso.assert_not_called()
self.assertEqual(sched.species_dict['TS_trsh'].final_xyz, new_xyz)
mock_run_opt.assert_called_once()
Comment thread
alongd marked this conversation as resolved.

@patch('arc.scheduler.Scheduler.run_job')
def test_troubleshoot_scan_job_skips_isomorphism_for_ts_non_conformer(self, mock_run_job):
"""troubleshoot_scan_job must not call check_xyz_isomorphism for a TS in the non-'change conformer' branch."""
ts_xyz = str_to_xyz("""N 0.91779059 0.51946178 0.00000000
H 1.81402049 1.03819414 0.00000000
H 0.00000000 0.00000000 0.00000000
H 0.91779059 1.22790192 0.72426890""")
ts_spc = ARCSpecies(label='TS_trsh_nc', is_ts=True, xyz=ts_xyz, multiplicity=1, charge=0,
compute_thermo=False)
ts_spc.rotors_dict = {0: {'pivots': [1, 2], 'scan': [3, 1, 2, 4], 'scan_path': '',
'invalidation_reason': '', 'success': None, 'symmetry': None,
'times_dihedral_set': 0, 'trsh_methods': [], 'trsh_counter': 0}}

project_directory = os.path.join(ARC_PATH, 'Projects', 'arc_project_ts_iso_trsh_nc')
self.addCleanup(shutil.rmtree, project_directory, ignore_errors=True)
sched = Scheduler(project='test_ts_iso_trsh_nc', ess_settings=self.ess_settings,
species_list=[ts_spc],
opt_level=Level(repr=default_levels_of_theory['opt']),
freq_level=Level(repr=default_levels_of_theory['freq']),
sp_level=Level(repr=default_levels_of_theory['sp']),
ts_guess_level=Level(repr=default_levels_of_theory['ts_guesses']),
project_directory=project_directory,
testing=True,
job_types=self.job_types1,
)
sched.trsh_ess_jobs = True
sched.trsh_rotors = True

job_mock = MagicMock()
job_mock.species_label = 'TS_trsh_nc'
job_mock.rotor_index = 0
job_mock.torsions = [[2, 0, 1, 3]] # 0-indexed; torsions_to_scans yields the 1-indexed scan [3, 1, 2, 4]
job_mock.job_name = 'scan_a201'
job_mock.scan_res = 8.0
job_mock.xyz = ts_xyz
job_mock.level = Level(repr=default_levels_of_theory['scan'])
job_mock.local_path_to_output_file = '/fake/path.log'

with patch('arc.species.species.ARCSpecies.check_xyz_isomorphism') as mock_iso, \
patch('arc.scheduler.trsh_scan_job') as mock_trsh_scan:
mock_trsh_scan.return_value = [], 4.0

sched.troubleshoot_scan_job(job=job_mock, methods={'inc_res': None})

mock_iso.assert_not_called()
mock_trsh_scan.assert_called_once_with(
label='TS_trsh_nc',
scan_res=8.0,
scan=[3, 1, 2, 4],
scan_list=[[3, 1, 2, 4]],
methods={'inc_res': None},
log_file='/fake/path.log',
)

mock_run_job.assert_called_once()
_, kwargs = mock_run_job.call_args
self.assertEqual(kwargs['label'], 'TS_trsh_nc')
self.assertEqual(kwargs['xyz'], ts_xyz)
self.assertEqual(kwargs['level_of_theory'], job_mock.level)
self.assertEqual(kwargs['job_type'], 'scan')
self.assertEqual(kwargs['torsions'], [[2, 0, 1, 3]])
self.assertEqual(kwargs['scan_trsh'], [])
self.assertEqual(kwargs['trsh'], {'scan_res': 4.0})
self.assertEqual(kwargs['rotor_index'], 0)

@classmethod
def tearDownClass(cls):
"""
Expand Down
46 changes: 26 additions & 20 deletions arc/species/species.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1621,6 +1621,8 @@ def mol_from_xyz(self,
Important for TS searches and for identifying rotor indices.
This works by generating a molecule from xyz and using the
2D structure to confirm that the perceived molecule is correct.
For TSs, the perceived molecule is accepted without enforcing
2D-graph isomorphism, since TS connectivity is not strictly defined.
If ``xyz`` is not given, the species xyz attribute will be used.

Args:
Expand All @@ -1643,28 +1645,32 @@ def mol_from_xyz(self,
n_fragments=self.get_n_fragments(),
)
if perceived_mol is not None:
allow_nonisomorphic_2d = (self.charge is not None and self.charge) \
or self.mol.has_charge() or perceived_mol.has_charge() \
or (self.multiplicity is not None and self.multiplicity >= 3) \
or self.mol.multiplicity >= 3 or perceived_mol.multiplicity >= 3
isomorphic = self.check_xyz_isomorphism(mol=perceived_mol,
xyz=xyz,
allow_nonisomorphic_2d=allow_nonisomorphic_2d)
if not isomorphic:
logger.warning(f'XYZ and the 2D graph representation for {self.label} are not isomorphic.\nGot '
f'xyz:\n{xyz}\n\nwhich corresponds to {self.mol.copy(deep=True).to_smiles()}\n'
f'{self.mol.copy(deep=True).to_adjacency_list()}\n\nand: '
f'{self.mol.copy(deep=True).to_smiles()}\n'
f'{self.mol.copy(deep=True).to_adjacency_list()}')
raise SpeciesError(f'XYZ and the 2D graph representation for {self.label} are not compliant.')
if not self.keep_mol:
if is_mol_valid(perceived_mol, charge=self.charge, multiplicity=self.multiplicity, n_radicals=self.number_of_radicals):
if self.is_ts:
if not self.keep_mol:
self.mol = perceived_mol
else:
try:
order_atoms(ref_mol=perceived_mol, mol=self.mol)
except SanitizationError:
else:
allow_nonisomorphic_2d = (self.charge is not None and self.charge) \
or self.mol.has_charge() or perceived_mol.has_charge() \
or (self.multiplicity is not None and self.multiplicity >= 3) \
or self.mol.multiplicity >= 3 or perceived_mol.multiplicity >= 3
isomorphic = self.check_xyz_isomorphism(mol=perceived_mol,
xyz=xyz,
allow_nonisomorphic_2d=allow_nonisomorphic_2d)
if not isomorphic:
logger.warning(f'XYZ and the 2D graph representation for {self.label} are not isomorphic.\nGot '
f'xyz:\n{xyz}\n\nwhich corresponds to {self.mol.copy(deep=True).to_smiles()}\n'
f'{self.mol.copy(deep=True).to_adjacency_list()}\n\nand: '
f'{perceived_mol.copy(deep=True).to_smiles()}\n'
f'{perceived_mol.copy(deep=True).to_adjacency_list()}')
raise SpeciesError(f'XYZ and the 2D graph representation for {self.label} are not compliant.')
if not self.keep_mol:
if is_mol_valid(perceived_mol, charge=self.charge, multiplicity=self.multiplicity, n_radicals=self.number_of_radicals):
self.mol = perceived_mol
else:
try:
order_atoms(ref_mol=perceived_mol, mol=self.mol)
except SanitizationError:
self.mol = perceived_mol
Comment on lines +1648 to +1673
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

mol_from_xyz() no longer handles keep_mol=True: for TSs it does nothing (so atom order may stay inconsistent with xyz), and for non-TSs the order_atoms() fallback is now under if not self.keep_mol, which appears inverted relative to the docstring (“prevent the generation of a new Molecule object”). This likely breaks workflows that set keep_mol=True to preserve the existing Molecule while reordering atoms (e.g., E0-only species created with keep_mol=True). Consider restoring the previous branching: when keep_mol is True, attempt order_atoms(ref_mol=perceived_mol, mol=self.mol) (with a fallback to assigning self.mol = perceived_mol on sanitization failure); when keep_mol is False, assign self.mol = perceived_mol (optionally gated by is_mol_valid).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

@calvinp0 calvinp0 Apr 20, 2026

Choose a reason for hiding this comment

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

the order_atoms() fallback was already gated under if not self.kee_mol on main. So both self.mol = perceived_mol assignment and the order_atoms fallback sat inside the same if not self.keep_mol block. So, keep_mol=True has always been don't touch self.mol. The PR perserves that exactly for non-TSs.

The TS branch is deliberately different: a transition state is not a species in the RMG sense, and a 2D molecular graph cannot represent the partial forming/breaking bonds at a saddle point

else:
perceived_mol = perceive_molecule_from_xyz(xyz,
charge=self.charge,
Expand Down
40 changes: 40 additions & 0 deletions arc/species/species_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1684,6 +1684,46 @@ def test_mol_from_xyz(self):
radical_count += atom.radical_electrons
self.assertEqual(radical_count, 2)

def test_ts_mol_from_xyz_skips_isomorphism_enforcement(self):
"""Test that a TS accepts the perceived xyz-derived molecule even if a stored 2D graph disagrees."""
xyz = {'symbols': ('O', 'O', 'H', 'C', 'C', 'C', 'C', 'H', 'H', 'H', 'H', 'H', 'H', 'H', 'H', 'H', 'H'),
'isotopes': (16, 16, 1, 12, 12, 12, 12, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1),
'coords': ((1.570004, 0.919237, -0.063628), (2.346384, -0.215837, 0.116826),
(2.578713, -0.467096, -0.784756), (-0.673058, -1.117816, -1.045216),
(-0.76037, -0.036132, 0.001231), (-0.850886, -0.54288, 1.418092),
(-1.694638, 1.099253, -0.333714), (-1.612555, -1.681809, -1.088997),
(-0.491563, -0.700452, -2.03735), (0.122945, -1.827698, -0.811666),
(0.427835, 0.508898, -0.034066), (-0.034676, -1.231859, 1.641118),
(-1.795224, -1.080235, 1.568607), (-0.814118, 0.27711, 2.136337),
(-2.733958, 0.749029, -0.320335), (-1.610541, 1.910407, 0.391085),
(-1.494252, 1.501954, -1.327915))}
adj = """multiplicity 2
1 O u0 p2 c0 {2,S} {17,S}
2 O u1 p2 c0 {1,S}
3 C u0 p0 c0 {4,S} {5,S} {6,S} {7,S}
4 C u0 p0 c0 {3,S} {8,S} {9,S} {10,S}
5 C u0 p0 c0 {3,S} {11,S} {12,S} {13,S}
6 C u0 p0 c0 {3,S} {14,S} {15,S} {16,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}
11 H u0 p0 c0 {5,S}
12 H u0 p0 c0 {5,S} {17,vdW}
13 H u0 p0 c0 {5,S}
14 H u0 p0 c0 {6,S}
15 H u0 p0 c0 {6,S}
16 H u0 p0 c0 {6,S}
17 H u0 p0 c0 {1,S} {12,vdW}"""

mol_from_adj = Molecule().from_adjacency_list(adj)
self.assertTrue(any(bond.is_van_der_waals() for bond in mol_from_adj.get_all_edges()),
'Expected the test adjlist to contain at least one vdW bond.')

spc = ARCSpecies(label='TS0', adjlist=adj, xyz=xyz, is_ts=True, multiplicity=2, charge=0)
self.assertFalse(any(bond.is_van_der_waals() for bond in spc.mol.get_all_edges()),
'TS ARCSpecies.mol should not retain vdW bonds from the input adjlist.')

def test_consistent_atom_order(self):
"""Test that the atom order is preserved whether starting from SMILES or from xyz"""
xyz9 = """O -1.17310019 -0.30822930 0.16269772
Expand Down
3 changes: 3 additions & 0 deletions docs/source/TS_search.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ Outputs and validation
""""""""""""""""""""""
Validated TS results are reported in the project output (log files and generated artifacts),
together with the supporting calculations (optimization, frequency, and IRC).
ARC does not require TS geometries to be isomorphic with a stored 2D adjacency list, since a TS does not have a
single strict graph representation. Instead, TS validation relies on TS-specific checks such as the imaginary
frequency, normal mode displacement analysis, IRC results, and energetic consistency.

Reference
"""""""""
Expand Down
Loading
Loading