Skip to content

Commit a4cdeba

Browse files
committed
Restore live conformer jobs by name on restart, not the fossil 'conformer{i}'
The contract: a running conformer job lives in self.running_jobs under the same name the live path emits, '{job_type}_{i}' (e.g. 'conf_opt_0'). Every consumer parses that format -- get_i_from_job_name strips the 'conf_opt'/ 'conf_sp' prefix, and get_completed_incore_jobs routes on it into job_dict[label]['conf_opt'][i]. How it broke: restore_running_jobs emitted the fossil 'conformer{i}' instead. get_i_from_job_name returns None for it, so get_completed_incore_jobs fell into its fallback branch, derived an empty job-type from the underscore-less name ('conformer0'.split('_')[:-1] == []), and died with KeyError: '' on the first scheduling sweep -- crashing every ARC restart that had a live conformer job. The fix emits '{job_type}_{i}' from the same expression the live path uses, rather than a second hard-coded literal that must be kept in sync by hand. The job_dict was already reconstructed correctly (conf_opt keyed by int index), so the name was the only defect. Also corrects get_i_from_job_name's docstring, which still advertised the retired 'conformer12' format. The test drives a restart payload carrying a live conf_opt job through the real restore_running_jobs + get_completed_incore_jobs path: red with KeyError: '' on the unfixed code, green after.
1 parent b5fc24a commit a4cdeba

5 files changed

Lines changed: 110 additions & 6 deletions

File tree

arc/checks/common.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,22 @@
1515
TS_IRC_FAILED_MARKER = 'INVALID TS (failed IRC validation)'
1616

1717

18+
def get_conformer_job_name(job_type: str, i: int) -> str:
19+
"""
20+
Get the name of a conformer job, the inverse of ``get_i_from_job_name()``.
21+
This is the single definition of the conformer job name format,
22+
used both when spawning a job and when restoring one from a restart file.
23+
24+
Args:
25+
job_type (str): The conformer job type, e.g., 'conf_opt' or 'conf_sp'.
26+
i (int): The conformer index.
27+
28+
Returns:
29+
str: The conformer job name, e.g., 'conf_opt_3'.
30+
"""
31+
return f'{job_type}_{i}'
32+
33+
1834
def is_conformer_job(job_name: str) -> bool:
1935
"""
2036
Check whether a job name represents a conformer job.
@@ -50,7 +66,7 @@ def get_i_from_job_name(job_name: str) -> int | None:
5066
Get the conformer or tsg index from the job name.
5167
5268
Args:
53-
job_name (str): The job name, e.g., 'conformer12' or 'tsg5'.
69+
job_name (str): The job name, e.g., 'conf_opt_12', 'conf_sp_3', or 'tsg5'.
5470
5571
Returns:
5672
int | None: The corresponding conformer or tsg index.

arc/checks/common_test.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,16 @@ def test_get_i_from_job_name(self):
5757
self.assertEqual(common.get_i_from_job_name('conf_opt_3355'), 3355)
5858
self.assertEqual(common.get_i_from_job_name('tsg2'), 2)
5959

60+
def test_get_conformer_job_name(self):
61+
"""Test the get_conformer_job_name() function"""
62+
self.assertEqual(common.get_conformer_job_name('conf_opt', 0), 'conf_opt_0')
63+
self.assertEqual(common.get_conformer_job_name('conf_sp', 3), 'conf_sp_3')
64+
self.assertEqual(common.get_conformer_job_name('conf_opt', 3355), 'conf_opt_3355')
65+
self.assertTrue(common.is_conformer_job(common.get_conformer_job_name('conf_sp', 12)))
66+
for job_type in common.CONFORMER_JOB_TYPES:
67+
for i in [0, 7, 99999]:
68+
self.assertEqual(common.get_i_from_job_name(common.get_conformer_job_name(job_type, i)), i)
69+
6070
def test_is_ts_check_exempt(self):
6171
"""
6272
Test the is_ts_check_exempt() function.

arc/job/adapter.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
import numpy as np
2525

26+
from arc.checks.common import get_conformer_job_name
2627
from arc.common import ARC_PATH, get_logger, read_yaml_file, save_yaml_file, torsions_to_scans, convert_to_hours
2728
from arc.exceptions import JobError
2829
from arc.imports import local_arc_path, settings, submit_scripts
@@ -643,7 +644,7 @@ def _set_job_number(self):
643644
# 2. Set other related attributes job_name and job_server_name.
644645
self.job_server_name = self.job_server_name or 'a' + str(self.job_num)
645646
if self.conformer is not None and self.job_name is None:
646-
self.job_name = f'{self.job_type}_{self.conformer}'
647+
self.job_name = get_conformer_job_name(self.job_type, self.conformer)
647648
elif self.tsg is not None and (self.job_name is None or 'tsg_a' in self.job_name):
648649
if self.job_name is not None:
649650
logger.warning(f'Replacing job name {self.job_name} with tsg{self.conformer}')

arc/scheduler.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
import arc.parser.parser as parser
1919
from arc import plotter
20-
from arc.checks.common import get_i_from_job_name, is_conformer_job, sum_time_delta
20+
from arc.checks.common import get_conformer_job_name, get_i_from_job_name, is_conformer_job, sum_time_delta
2121
from arc.checks.ts import check_imaginary_frequencies, check_ts, check_irc_species_and_rxn
2222
from arc.common import (extremum_list,
2323
get_angle_in_180_range,
@@ -1098,7 +1098,7 @@ def run_job(self,
10981098
elif conformer is not None:
10991099
# Running a conformer DFT job. Append differently to job_dict.
11001100
self.running_jobs[label] = list() if label not in self.running_jobs else self.running_jobs[label]
1101-
self.running_jobs[label].append(f'{job_type}_{conformer}') # mark as a running job
1101+
self.running_jobs[label].append(get_conformer_job_name(job_type, conformer)) # mark as a running job
11021102
if 'conf_opt' not in self.job_dict[label]:
11031103
self.job_dict[label]['conf_opt'] = dict()
11041104
if 'conf_sp' not in self.job_dict[label] and job_type == 'conf_sp':
@@ -4131,7 +4131,10 @@ def restore_running_jobs(self):
41314131
and ('tsg' not in job_description or job_description['tsg'] is None):
41324132
self.running_jobs[spc_label].append(job_description['job_name'])
41334133
elif 'conformer' in job_description:
4134-
self.running_jobs[spc_label].append(f'conformer{job_description["conformer"]}')
4134+
# Emit the same name the live path uses (e.g. 'conf_opt_0'),
4135+
# not the fossil 'conformer{i}' that no consumer of running_jobs accepts.
4136+
self.running_jobs[spc_label].append(get_conformer_job_name(job_description['job_type'],
4137+
job_description['conformer']))
41354138
elif 'tsg' in job_description:
41364139
self.running_jobs[spc_label].append(f'tsg{job_description["tsg"]}')
41374140
for species in self.species_list:
@@ -4162,9 +4165,16 @@ def restore_running_jobs(self):
41624165
and ('tsg' not in job_description or job_description['tsg'] is None):
41634166
self.job_dict[spc_label][job_description['job_type']][job_description['job_name']] = job
41644167
elif 'conformer' in job_description and job_description['conformer'] is not None:
4168+
# File the job under its actual job_type ('conf_opt' or 'conf_sp'), the same
4169+
# key the live path uses (see run_job) and the same key get_completed_incore_jobs
4170+
# reads back -- filing a conf_sp job under 'conf_opt' would crash the first sweep
4171+
# with KeyError: 'conf_sp'.
4172+
conf_job_type = job_description['job_type']
41654173
if 'conf_opt' not in self.job_dict[spc_label].keys():
41664174
self.job_dict[spc_label]['conf_opt'] = dict()
4167-
self.job_dict[spc_label]['conf_opt'][int(job_description['conformer'])] = job
4175+
if conf_job_type == 'conf_sp' and 'conf_sp' not in self.job_dict[spc_label].keys():
4176+
self.job_dict[spc_label]['conf_sp'] = dict()
4177+
self.job_dict[spc_label][conf_job_type][int(job_description['conformer'])] = job
41684178
# don't generate additional conformers for this species
41694179
self.dont_gen_confs.append(spc_label)
41704180
elif 'tsg' in job_description and job_description['tsg'] is not None:

arc/scheduler_test.py

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,73 @@ def test_conformers(self):
240240
self.assertEqual(lines[11], '\n')
241241
self.assertEqual(lines[12], 'SMILES: CC\n')
242242

243+
def test_restore_running_jobs_conformer_reconnects(self):
244+
"""Restarting with a live conformer job must reconnect to it, not crash.
245+
246+
Regression for the restore-path job-name contract. During normal operation a running
247+
conformer job is stored in ``running_jobs`` as ``'{job_type}_{i}'`` (e.g. ``'conf_opt_0'``),
248+
and every consumer of ``running_jobs`` parses that format. ``restore_running_jobs`` used to
249+
emit the fossil ``'conformer{i}'`` instead, which ``get_i_from_job_name`` returns ``None``
250+
for; the first scheduling sweep after a restart (``get_completed_incore_jobs``) then fell
251+
into its fallback branch, derived an empty job-type from the underscore-less name, and died
252+
with ``KeyError: ''``. This drives a restart payload carrying a live conformer job through
253+
the real restore + sweep path and asserts the reconnection instead of the crash.
254+
"""
255+
label = 'methylamine'
256+
xyz = """C -0.57422867 -0.01669771 0.01229213
257+
N 0.82084044 0.08279104 -0.37769346
258+
H -1.05737005 -0.84067772 -0.52007494
259+
H -1.10211468 0.90879867 -0.23383011
260+
H -0.66133128 -0.19490562 1.08785111
261+
H 0.88047852 0.26966160 -1.37780789
262+
H 1.27889520 -0.81548721 -0.22940984"""
263+
spc = ARCSpecies(label=label, smiles='CN', xyz=xyz)
264+
sched = Scheduler(project='project_test_restore_conf', ess_settings=self.ess_settings,
265+
species_list=[spc], composite_method=None,
266+
conformer_opt_level=Level(repr=default_levels_of_theory['conformer']),
267+
opt_level=Level(repr=default_levels_of_theory['opt']),
268+
freq_level=Level(repr=default_levels_of_theory['freq']),
269+
sp_level=Level(repr=default_levels_of_theory['sp']),
270+
scan_level=Level(repr=default_levels_of_theory['scan']),
271+
ts_guess_level=Level(repr=default_levels_of_theory['ts_guesses']),
272+
project_directory=self.project_directory, testing=True,
273+
job_types=self.job_types1,
274+
orbitals_level=default_levels_of_theory['orbitals'], adaptive_levels=None)
275+
# Two live conformer jobs -- a conf_opt and a conf_sp -- serialized exactly as ARC writes
276+
# them into the restart file. conf_sp jobs can equally be in flight during a restart, and are
277+
# routed differently on read-back (get_completed_incore_jobs reads job_dict[label]['conf_sp']).
278+
conf_opt_job = job_factory(job_adapter='gaussian', project='project_test_restore_conf',
279+
ess_settings=self.ess_settings, species=[spc], xyz=xyz,
280+
job_type='conf_opt', conformer=0,
281+
level=Level(repr={'method': 'wb97xd', 'basis': 'def2svp'}),
282+
project_directory=self.project_directory, job_num=901)
283+
conf_sp_job = job_factory(job_adapter='gaussian', project='project_test_restore_conf',
284+
ess_settings=self.ess_settings, species=[spc], xyz=xyz,
285+
job_type='conf_sp', conformer=0,
286+
level=Level(repr={'method': 'wb97xd', 'basis': 'def2svp'}),
287+
project_directory=self.project_directory, job_num=902)
288+
sched.restart_dict = {'running_jobs': {label: [conf_opt_job.as_dict(), conf_sp_job.as_dict()]}}
289+
sched.running_jobs = dict()
290+
sched.job_dict = dict()
291+
292+
sched.restore_running_jobs()
293+
# Each conformer job is filed under its own job_type keyed by its integer index -- a conf_sp
294+
# job under 'conf_sp', not 'conf_opt'. Filing conf_sp under 'conf_opt' would crash the sweep
295+
# below with KeyError: 'conf_sp'.
296+
self.assertIn('conf_opt', sched.job_dict[label])
297+
self.assertIn(0, sched.job_dict[label]['conf_opt'])
298+
self.assertIn('conf_sp', sched.job_dict[label])
299+
self.assertIn(0, sched.job_dict[label]['conf_sp'])
300+
301+
# The first scheduling sweep after a restart reproduces the production crash on the unfixed
302+
# code: get_i_from_job_name('conformer0') is None, the fallback derives an empty job-type
303+
# from the underscore-less name, and self.job_dict[label][''] raises KeyError: ''.
304+
sched.get_completed_incore_jobs()
305+
self.assertEqual(sched.completed_incore_jobs, list())
306+
307+
# And the restored names are the live '{job_type}_{i}' format, not the fossil 'conformer{i}'.
308+
self.assertEqual(sched.running_jobs[label], ['conf_opt_0', 'conf_sp_0'])
309+
243310
def test_check_negative_freq(self):
244311
"""Test the check_negative_freq() method"""
245312
label = 'C2H6'

0 commit comments

Comments
 (0)