Skip to content

Commit 9361356

Browse files
alongdclaude
andcommitted
adaptive levels: address PR review comments
Copy a no-copy species shared by reactions on different grains (a single override can't keep both consistent); reject 'inf' as an atom_range lower bound; fix inner loop variable shadowing, a stale docstring default, and inverted grain wording in the docs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0221521 commit 9361356

5 files changed

Lines changed: 83 additions & 26 deletions

File tree

arc/main.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,9 +1295,11 @@ def process_adaptive_levels(adaptive_levels: list | None) -> dict | None:
12951295
f'keys, got {entry} which is a {type(entry)} in:\n{adaptive_levels}')
12961296
atom_range = entry['atom_range']
12971297
if not isinstance(atom_range, (list, tuple)) or len(atom_range) != 2 \
1298-
or not all(isinstance(a, int) or a in ('inf', float('inf')) for a in atom_range):
1299-
raise InputError(f'The "atom_range" of each adaptive levels entry must be a 2-length list of integers '
1300-
f'with an optional "inf" upper bound, got {atom_range} in:\n{adaptive_levels}')
1298+
or not isinstance(atom_range[0], int) \
1299+
or not (isinstance(atom_range[1], int) or atom_range[1] in ('inf', float('inf'))):
1300+
raise InputError(f'The "atom_range" of each adaptive levels entry must be a 2-length list of an integer '
1301+
f'lower bound and an integer or "inf" upper bound, got {atom_range} '
1302+
f'in:\n{adaptive_levels}')
13011303
atom_range = (atom_range[0], 'inf' if atom_range[1] in ('inf', float('inf')) else atom_range[1])
13021304
levels = entry['levels']
13031305
if not isinstance(levels, dict):

arc/main_test.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,11 @@ def test_process_adaptive_levels(self):
433433
# atom_range is not a 2-length list.
434434
with self.assertRaises(InputError):
435435
process_adaptive_levels([{'atom_range': [5], 'levels': {'sp': 'b3lyp/6-311+g(d,p)'}}])
436+
# 'inf' is only allowed as the upper bound.
437+
with self.assertRaises(InputError):
438+
process_adaptive_levels([{'atom_range': [float('inf'), 'inf'], 'levels': {'sp': 'b3lyp/6-311+g(d,p)'}}])
439+
with self.assertRaises(InputError):
440+
process_adaptive_levels([{'atom_range': ['inf', 10], 'levels': {'sp': 'b3lyp/6-311+g(d,p)'}}])
436441
# The last range does not end with 'inf'.
437442
with self.assertRaises(InputError):
438443
process_adaptive_levels([{'atom_range': [1, 5], 'levels': {'sp': 'wb97xd/def2tzvp'}},

arc/scheduler.py

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3963,19 +3963,21 @@ def _apply_adaptive_reaction_levels(self):
39633963
39643964
Heavy atoms are conserved across a reaction, so the reaction-wide heavy-atom count (the sum over the reactants,
39653965
equal to the TS supermolecule count) keys a single adaptive level for the whole reaction. A reaction
3966-
participant whose own heavy-atom count lands it on a *different* adaptive grain than the reaction would
3967-
otherwise be evaluated at an inconsistent level, mixing levels of theory across the barrier. To avoid this,
3968-
for each such participant whose ``thermo_at_own_level`` is ``True`` (the default), an autonomous relabeled
3969-
copy of the species is created from the outset and used by the reaction (evaluated at the reaction-wide
3970-
level), while the original species is left to compute its own thermochemistry at its own granular level. If
3971-
``thermo_at_own_level`` is ``False``, the participant itself is evaluated at the reaction-wide level (no copy).
3966+
participant whose own heavy-atom count lands it on a *different* (finer) adaptive grain than the reaction
3967+
would otherwise be evaluated at an inconsistent level, mixing levels of theory across the barrier. To avoid
3968+
this, each such participant whose ``thermo_at_own_level`` is ``False`` (the default) is itself evaluated at
3969+
the reaction-wide level (no copy). If ``thermo_at_own_level`` is ``True``, an autonomous relabeled copy of
3970+
the species is created from the outset and used by the reaction (evaluated at the reaction-wide level),
3971+
while the original species is left to compute its own thermochemistry at its own granular level. A copy is
3972+
also created for a no-copy participant that is shared across reactions landing on different grains, since a
3973+
single per-species override cannot keep both reactions internally consistent.
39723974
39733975
This runs once during setup, before the reactions are processed, so each reaction is defined with its copies
39743976
from the start (its label, reactants, and products are kept mutually consistent for ``check_attributes`` and
39753977
restart).
39763978
"""
3977-
for i, rxn in enumerate(self.rxn_list):
3978-
rxn_index = rxn.index if rxn.index is not None else i
3979+
for rxn_i, rxn in enumerate(self.rxn_list):
3980+
rxn_index = rxn.index if rxn.index is not None else rxn_i
39793981
reactant_species = [self.species_dict[label] for label in rxn.reactants if label in self.species_dict]
39803982
if len(reactant_species) != len(rxn.reactants) \
39813983
or any(spc.number_of_heavy_atoms is None for spc in reactant_species):
@@ -3985,24 +3987,30 @@ def _apply_adaptive_reaction_levels(self):
39853987
reaction_n_heavy = sum(spc.number_of_heavy_atoms for spc in reactant_species)
39863988
reaction_range = self._adaptive_atom_range(reaction_n_heavy)
39873989
for participants in (rxn.reactants, rxn.products):
3988-
for i, label in enumerate(participants):
3990+
for pos, label in enumerate(participants):
39893991
spc = self.species_dict.get(label)
39903992
if spc is None or spc.number_of_heavy_atoms is None \
39913993
or self._adaptive_atom_range(spc.number_of_heavy_atoms) == reaction_range:
39923994
continue
39933995
if not spc.thermo_at_own_level:
3994-
spc.adaptive_lot_n_heavy = reaction_n_heavy
3995-
continue
3996+
if spc.adaptive_lot_n_heavy is None \
3997+
or self._adaptive_atom_range(spc.adaptive_lot_n_heavy) == reaction_range:
3998+
spc.adaptive_lot_n_heavy = reaction_n_heavy
3999+
continue
4000+
logger.warning(f'Species {label} participates in reactions on different adaptive level '
4001+
f'grains, creating a dedicated copy of it for reaction {rxn.label} to keep '
4002+
f'the reaction internally consistent.')
39964003
copy_label = check_label(f'{label}_TS{rxn_index}')[0]
3997-
copy_spc = spc.copy()
3998-
copy_spc.label = copy_label
3999-
copy_spc.adaptive_lot_n_heavy = reaction_n_heavy
4000-
copy_spc.compute_thermo = False
4001-
copy_spc.include_in_thermo_lib = False
4002-
self.species_list.append(copy_spc)
4003-
self.species_dict[copy_label] = copy_spc
4004-
self.initialize_output_dict(copy_label)
4005-
participants[i] = copy_label
4004+
if copy_label not in self.species_dict:
4005+
copy_spc = spc.copy()
4006+
copy_spc.label = copy_label
4007+
copy_spc.adaptive_lot_n_heavy = reaction_n_heavy
4008+
copy_spc.compute_thermo = False
4009+
copy_spc.include_in_thermo_lib = False
4010+
self.species_list.append(copy_spc)
4011+
self.species_dict[copy_label] = copy_spc
4012+
self.initialize_output_dict(copy_label)
4013+
participants[pos] = copy_label
40064014
rxn.label = rxn.arrow.join([rxn.plus.join(rxn.reactants), rxn.plus.join(rxn.products)])
40074015

40084016
def initialize_output_dict(self, label: str | None = None):

arc/scheduler_test.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1325,6 +1325,48 @@ def test_thermo_at_own_level_default_no_copy(self):
13251325
self.assertFalse(any('_TS' in label for label in sched.species_dict))
13261326
self.assertEqual(sched.species_dict['CH4'].adaptive_lot_n_heavy, 2)
13271327

1328+
def test_shared_species_across_grains_gets_copy(self):
1329+
"""Test that a no-copy species shared by reactions on different grains gets a copy for the second reaction"""
1330+
oh = ARCSpecies(label='OH', smiles='[OH]')
1331+
h2o = ARCSpecies(label='H2O', smiles='O')
1332+
rxn1 = ARCReaction(label='CH4 + OH <=> CH3 + H2O',
1333+
r_species=[ARCSpecies(label='CH4', smiles='C'), oh],
1334+
p_species=[ARCSpecies(label='CH3', smiles='[CH3]'), h2o])
1335+
rxn2 = ARCReaction(label='C3H8 + OH <=> nC3H7 + H2O',
1336+
r_species=[ARCSpecies(label='C3H8', smiles='CCC'), oh],
1337+
p_species=[ARCSpecies(label='nC3H7', smiles='[CH2]CC'), h2o])
1338+
project_directory = os.path.join(ARC_PATH, 'Projects', 'adaptive_shared_delete')
1339+
self.addCleanup(shutil.rmtree, project_directory, ignore_errors=True)
1340+
species_list = rxn1.r_species + rxn1.p_species + [rxn2.r_species[0], rxn2.p_species[0]]
1341+
sched = Scheduler(project='adaptive_shared',
1342+
ess_settings=self.ess_settings,
1343+
species_list=species_list,
1344+
rxn_list=[rxn1, rxn2],
1345+
opt_level=Level(repr='b3lyp/6-31g(d,p)'),
1346+
sp_level=Level(repr='b3lyp/6-311+g(d,p)'),
1347+
freq_level=Level(repr='b3lyp/6-31g(d,p)'),
1348+
adaptive_levels={(1, 1): {('sp',): Level(repr='ccsd(t)-f12/cc-pvtz-f12')},
1349+
(2, 3): {('sp',): Level(repr='dlpno-ccsd(t)/def2-tzvp')},
1350+
(4, 'inf'): {('sp',): Level(repr='b3lyp/6-311+g(d,p)')}},
1351+
project_directory=project_directory,
1352+
job_types=initialize_job_types(),
1353+
testing=True)
1354+
1355+
# rxn1 (2 heavy atoms) set the shared wells' overrides; rxn1 itself is unchanged.
1356+
self.assertEqual(rxn1.label, 'CH4 + OH <=> CH3 + H2O')
1357+
self.assertEqual(sched.species_dict['OH'].adaptive_lot_n_heavy, 2)
1358+
# rxn2 (4 heavy atoms) lands on a different grain, so the shared wells got dedicated copies.
1359+
self.assertEqual(set(rxn2.reactants), {'C3H8', 'OH_TS1'})
1360+
self.assertEqual(set(rxn2.products), {'nC3H7', 'H2O_TS1'})
1361+
self.assertEqual(rxn2.label,
1362+
rxn2.arrow.join([rxn2.plus.join(rxn2.reactants), rxn2.plus.join(rxn2.products)]))
1363+
for copy_label in ['OH_TS1', 'H2O_TS1']:
1364+
self.assertEqual(sched.species_dict[copy_label].adaptive_lot_n_heavy, 4)
1365+
self.assertFalse(sched.species_dict[copy_label].compute_thermo)
1366+
# Unshared rxn2 wells just took the rxn2 override, no copies.
1367+
self.assertEqual(sched.species_dict['C3H8'].adaptive_lot_n_heavy, 4)
1368+
self.assertEqual(sched.species_dict['nC3H7'].adaptive_lot_n_heavy, 4)
1369+
13281370
def test_unimolecular_no_copy(self):
13291371
"""Test that a reaction whose well shares the reaction's grain gets no copies"""
13301372
r = [ARCSpecies(label='nC3H7', smiles='[CH2]CC')]

docs/source/advanced.rst

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -283,9 +283,9 @@ levels (defined separately, e.g., via ``opt_level``, or using ARC's defaults).
283283
When a species participates in a reaction, all of the reaction's species (the TS and the
284284
reactant/product wells) are energy-evaluated at a single, reaction-consistent level keyed
285285
by the largest participant (the TS), so the barrier is computed at one consistent level.
286-
By default (the per-species ``thermo_at_own_level`` flag, ``False``) a well that lands on a
287-
coarser grain than its reaction is evaluated directly at the reaction-wide level, and its
288-
thermochemistry uses that same (coarser) level. Set ``thermo_at_own_level=True`` on a species
286+
By default (the per-species ``thermo_at_own_level`` flag, ``False``) a well whose own size
287+
falls on a finer grain than its reaction's is evaluated directly at the reaction-wide
288+
(coarser) level, and its thermochemistry uses that same level. Set ``thermo_at_own_level=True`` on a species
289289
to instead compute its thermochemistry at its own size-appropriate (granular) adaptive level:
290290
an autonomous, relabeled copy of the species is then created and used by the reaction (at the
291291
reaction-wide level), while the original keeps its own level for thermochemistry.

0 commit comments

Comments
 (0)