CREST Adapter - #807
Open
calvinp0 wants to merge 266 commits into
Open
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
calvinp0
force-pushed
the
crest_adapter
branch
from
January 27, 2026 12:39
d6a83b4 to
c231a43
Compare
calvinp0
force-pushed
the
crest_adapter
branch
2 times, most recently
from
February 2, 2026 09:49
3e88c36 to
7674f5f
Compare
calvinp0
force-pushed
the
crest_adapter
branch
3 times, most recently
from
February 2, 2026 14:31
9c25f3d to
51368be
Compare
calvinp0
force-pushed
the
crest_adapter
branch
2 times, most recently
from
February 4, 2026 20:14
9be935e to
eab7647
Compare
… N ranks, was Y GB across M)
…ocks determine_ess_status() mined the Orca SCF block for an initial/final energy pair to run its divergence heuristic. Two ways that crashed on a normally terminated run: 1. The scan for the first SCF iteration energy assumed the line immediately following the incremental-Fock marker holds one. Orca may print a SOSCF header and a separator in between, and the separator holds a single token, so `forward_lines[j + 1].split()[1]` raised IndexError. The loop was also unbounded and could only terminate by raising. 2. `scf_energy_last_iteration` was assigned only inside the 'TOTAL SCF ENERGY' branch but read whenever an initial energy was found, so a log with a parseable iteration line and no 'TOTAL SCF ENERGY' header raised UnboundLocalError. Neither exception is caught by any caller, so either one aborts the whole run. Cherry-picked from fix_orca_ess_status (b79d011).
determine_ess_status() dispatches on an exact software name. OrcaNEBAdapter passed 'orca_neb', matched no branch, and fell through to the terminal `return '', list(), '', ''`, so a normally terminated NEB run carried an empty status rather than 'done'. Since 35d9bd3 the tsg ingestion gate requires ess_succeeded for .log output, so that empty status silently discarded every Orca NEB TS guess. Measured on the 500-reaction benchmark: orca_neb contributed guesses in 10 of 16 reactions before 2026-07-15 and in 0 of 15 after. Override ess_software to 'orca' on OrcaNEBAdapter, and use it at the remaining determine_ess_status call site that still passed job_adapter. Note that Orca NEB now takes part in Orca's ESS error handling like any other Orca job; in particular a 'Syntax' keyword raises JobError, which Scheduler.end_job turns into a re-run. Cherry-picked from fix_orca_ess_status (a0e1a6a).
test_determine_ess_status_orca_scf_energies_unavailable built its two logs as
inline strings. Replace them with fixtures derived from real Orca output:
arc/testing/neb/neb_res.out truncated at two points that leave the SCF block
incomplete, so both carry genuine Orca formatting (the SOSCF header and the
one-token S-O-S-C-F separator that caused the original IndexError) rather than
a hand-written approximation.
No log already under arc/testing/ exhibits either shape -- the corpus was
collected from runs that failed in interesting ways, and a run that stops
before emitting TOTAL SCF ENERGY looks like infrastructure noise -- so the
fixtures are derived rather than captured.
Each pins one half of the fix:
neb_scf_no_iteration_line.out marker + SOSCF header + column header, no
numeric row. Pre-fix this raised IndexError on
the separator; the bounded scan must instead
run off the end without assigning an energy.
neb_scf_no_total_energy.out a parseable iteration row but no TOTAL SCF
ENERGY block, so the initial energy is
assigned and the final one is not. With the
scan fixed but the guard reverted this still
raises, so it isolates the guard.
Verified red with arc/job/trsh.py reverted to its pre-fix state, green with it
restored. Drops the _write_temp_log helper.
Two changes to the TS-guess completion path. The ingestion gate added for QST2 requires a successful ESS run before a .log output is read. QST2 and Orca NEB both write a file called input.log, so the gate caught NEB as well -- but the two differ: a failed QST2 job has nothing usable in its log, whereas NEB writes its geometry before the post-processing that may then fail. Orca NEB contributed guesses in 10 of 16 benchmark reactions before that gate and none of 15 after. Key the exemption on the adapter rather than on a file extension several unrelated adapters share, so a failed QST2 job stays excluded. troubleshoot_ess already knows about tsg jobs -- it records un-troubleshootable errors onto the TSGuess -- but no call site ever passed it one, so that branch was unreachable and a guess lost to a recoverable failure was dropped with only a warning. Call it for an errored TS-guess job, guarded on times_rerun so a persistent failure is attempted once.
_select_ch3_anchors() promised a second anchor B forming a non-linear B-X-A angle, but never tested for colinearity and never fell back to hydrogens. For a linear heavy-atom skeleton (e.g., propyne, CH3-C#CH) every heavy candidate is colinear with the X->A axis, so _construct_local_axes() raised 'Anchors are colinear; cannot define unique plane.', map_rxn() returned None, and the whole reaction could not be atom mapped (surfaced by a Retroene benchmark reaction, C=C=CCC <=> C#CC + C=C, once the species carried optimized geometries). Now anchor selection walks candidates in priority order (heavy neighbor of A, any other neighbor of A, the XH3 hydrogens, any remaining atom) and returns the first that passes the same non-colinearity criterion _construct_local_axes() enforces, so selection can never hand it anchors it will reject. _construct_local_axes() itself remains strict. Also, _map_xh3_group() now rejects a (None, None) anchor tuple (a non-empty tuple is truthy, so the previous guard missed it) and returns None instead of propagating a ValueError, and map_hydrogens() falls back to an in-order assignment of the three equivalent XH3 hydrogens when the geometric refinement fails, so the resulting map stays a complete permutation.
…arity guard (cherry picked from commit 6b4e87d)
(cherry picked from commit 79b771d on fix_job_log_quality)
(cherry picked from commit 15095d7 on fix_job_log_quality)
… and reduce memory (cherry picked from commit fb4719a on feature_gaussian_trsh_remedies)
(cherry picked from commit 6d7181a on fix_job_log_quality)
Register 'crest' as a TS-search adapter for intra_NO2_ONO_conversion and add the corresponding CREST constraint branch to the seed hub. The family recipe is BREAK_BOND *1-*2 / FORM_BOND *1-*3, i.e. the carbon-nitrogen bond breaks, the carbon-oxygen bond forms, and the nitrogen-oxygen bond is retained while one nitro oxygen migrates onto the carbon. The constraint specification pins all three distances plus the C-N-O angle, so CREST metadynamics samples spectator conformations without tearing the nitro group apart. The family produces no heuristic seeds, so CREST reaches it through the backup-seed route, which carries no generator metadata. The recipe atoms are therefore taken from the reaction's RMG family label map; explicit seed metadata (either C/N/O roles or the '*1'/'*2'/'*3' labels) still wins when present. A validator rejects assignments that are not three distinct in-range indices with C, N and O element symbols in their roles, in which case the constraint derivation returns None and CREST skips the seed. Also skip the three-center terminal-terminal distance in constraints.inp when it is already an explicit distance pair, which is the case here (C-O is the forming bond). (cherry picked from commit 6815bab)
(cherry picked from commit a703b52 on fix_job_log_quality)
(cherry picked from commit b6fd09c on fix_job_log_quality)
(cherry picked from commit dd7cf4a on fix_job_log_quality)
(cherry picked from commit fc6ef14 on fix_job_log_quality)
check_irc_species_and_rxn() pessimistically set ts_checks['IRC'] = False before performing any comparison, and one path (the bond-list fallback failing to obtain the reaction's connectivity) returned while leaving that False in place. A False IRC verdict therefore conflated "the IRC endpoints were compared and do not match the reactants/products" with "the IRC endpoints could not be compared at all", which makes False unusable as a gate. The check now starts from None (unknown, the ts_checks default) and only writes False where a comparison was actually carried out and did not match: after a failed isomorphism comparison, and after a failed bond-list comparison. The no-comparison path leaves None and logs the reason, including the exception.
…hem silently A TS whose IRC endpoints re-perceive as something other than the reaction's reactants and products does not describe that reaction, yet ARC still fitted and published an Arrhenius expression for it with nothing in the output indicating that validation had failed (benchmark reaction nitroethane <=> ethyl nitrite, family intra_NO2_ONO_conversion: ts_checks IRC False, k off by ~9 orders of magnitude at 1000 K). The rate is deliberately still computed and reported - it is diagnostically valuable and it is how this class of bug is found - but every artifact carrying it is now labeled, via the new common.get_ts_validation_comment() helper: - Arkane parse_reaction_kinetics(): a 'ts_validation' key on reaction.kinetics and the marker appended to the Arrhenius comment. - processor.compare_rates(): a 'ts_validation' field in output/RMG_kinetics.yml. - plotter.save_kinetics_lib(): the marker as the Arrhenius comment and at the top of the entry's longDesc in the RMG kinetics library. - plotter.draw_kinetics_plots(): the marker in the rate_plots.pdf plot title. Each of these also logs an error naming the reaction. Only a ts_checks['IRC'] value of False (checked and failed) triggers the marker; None (not checked, e.g. job_types irc: false) and True are left untouched.
…he wells The IRC verdict was written into ``TS.ts_checks['IRC']`` but never read by any production code path: ``check_ts()`` explicitly exempted 'IRC' from ``ts_passed_checks()``, and ``Scheduler.check_irc_species()`` only computed the verdict and dropped it. A TS whose two IRC endpoints both optimize to the same well was therefore accepted, and a rate coefficient that does not describe the requested reaction was published. A positively failed IRC check now rejects the TS and triggers a search for a different TS guess, mirroring the existing treatment of a failed normal mode displacement check. The check is three-valued: only ``False`` rejects. ``None`` means the check was not performed (e.g., IRC jobs were not requested) and never rejects anything, in ``ts_passed_checks()`` as well. Rejection reuses ``switch_ts()``, which resets the TS checks, the output paths and job types, the rotors, and deletes the IRC species spawned from the rejected guess, so the newly chosen guess re-runs opt/freq/sp/IRC from a clean state. Termination is bounded by ``chosen_ts_list``: once every guess was tried, ``determine_most_likely_ts_conformer()`` sets ``ts_guesses_exhausted``, and the TS is marked unconverged instead of being switched again. Also guard the deletion of an empty ``running_jobs`` entry in the main loop, since rejecting a TS while processing the opt job of one of its IRC species removes that species' entry. Cherry-picked into arcbench, where check_irc_species() already called the logging-only Scheduler.report_irc_verdict(). Composed rather than duplicated: report_irc_verdict() stays the single log site (it gained an optional ts_checks override and now returns the verdict), and process_irc_verdict() delegates the logging to it and performs only the enforcement.
GCN inference is stochastic and was never seeded, so the same reaction with byte-identical inputs produced entirely different TS guesses between runs. Add TS_SEARCH_RANDOM_SEED to settings (default 1, matching the existing RDKit randomSeed=1 convention, overridable via the ~/.arc overlay) and hand it to the GCN subprocess, which runs under the ts_gcn env where ARC's own process-level seeding cannot reach: --seed in the direct/incore mode, the 'seed' key of input.yml in the batch/queue mode. The script seeds python-random, NumPy and PyTorch (CPU + CUDA) immediately before each inference, and ARC additionally exports PYTHONHASHSEED into the child environment, since that one only takes effect before the child interpreter starts. Each repetition uses base seed + repetition index, so the repetitions still sample different starting points while the whole set of guesses is reproducible. CREST is not seeded: the installed CREST 2.12 exposes no seed flag at all (its only random-related option, -samerand, applies to xtbiff, not to the GFN2-xTB metadynamics), so its non-determinism cannot be controlled from ARC.
cluster_tsgs() kept the first-seen member of each near-duplicate cluster, so the survivor depended on the order the guesses were appended. That order is not stable: process_completed_tsg_queue_jobs() assigns an index at arrival time and re-clusters immediately, so orca_neb/qst2 jobs completing in a different PBS order swapped indices and could flip which member survived - and the survivor's geometry is what gets DFT-optimized. Traverse the guesses in ascending index order and keep the lowest-index member as the cluster representative. Index rather than energy: at clustering time most guesses still have energy is None, so an energy rule would be undefined and itself order-dependent. Surviving guesses are not renumbered - their indices are provenance - so the numbering may have gaps, which the clustering log message now states along with which duplicates were absorbed into which survivor. Two supporting fixes without which the lowest-index rule is not well defined: the 'cluster' bookkeeping now accumulates across the repeated clustering passes driven by arriving queue jobs instead of being reset to the survivor's own index each pass, and new indices come from get_next_tsg_index() rather than len(ts_guesses), which after a clustering pass could re-issue an index that a survivor or an absorbed guess already held.
``TSGuess.index`` is a stable identity (what clustering orders by, what ``chosen_ts`` / ``chosen_ts_list`` refer to, and what keys the TS guess report); a list position is not. Four sites treated the two as interchangeable ints: * ``ARCSpecies.process_xyz`` never set ``index`` on a user TS guess, so it stayed ``None`` -- an index-less guess can never be matched by the ``chosen_ts`` selection loop. Assign one, continuing past the highest index in use (clustering removes guesses but preserves the survivors' indices, so the list length can collide with an index already taken). The redundant positional re-subscript is dropped; ``TSGuess.__init__`` already forces ``success`` for a user guess. * ``Scheduler.troubleshoot_ess`` indexed ``ts_guesses`` with the number parsed out of a ``tsg<i>`` job name. That number identifies the TS-search *adapter* that was dispatched, not a guess: an adapter may contribute several guesses or none, so this annotated an unrelated guess, and raised ``IndexError`` -- aborting the whole run -- whenever the adapter number exceeded the number of guesses generated so far. Match on the adapter that produced each guess instead. * ``Scheduler.run_ts_conformer_jobs`` reported ``ts_guesses[0]`` as the provenance of the lone successful guess while optimizing ``successful_tsgs[0]``'s coordinates. Several adapters append ``success=False`` guesses, so those are not the same object and ``chosen_ts_method`` could name a method that produced no geometry. * ``output._get_ts_imag_freq`` subscripted ``ts_guesses`` with ``chosen_ts``, reporting a different guess's imaginary frequency whenever the two numberings diverged. None of these change which TS guess is selected. (cherry picked from commit bed4eac)
ARCSpecies.chosen_ts stores a TSGuess.index (an identity assigned when the guess is appended to the species), while TSGuess.conformer_index is the position of the guess among the *successful* guesses (the conformer optimization job index). check_negative_freq() matched the chosen TS guess by conformer_index, and additionally relied on the loop variable leaking out of the for loop, so a no-match left 'tsg' bound to the last guess rather than to None. Since check_imaginary_frequencies(None) returns True by design (a guess whose freqs were not computed yet must not be disqualified during pre-screening), a TS with zero, two or three imaginary frequencies could be accepted as converged while the real frequencies were never recorded. Match by TSGuess.index via a new get_chosen_tsg() helper, and treat a no-match as an unverified frequency check: log a warning naming the label, chosen_ts and the available indices, and look for a different TS conformer instead of silently accepting the geometry. Also drop the same identity clobber in the pipe ts_opt ingestion, which overwrote TSGuess.index with the conformer index. (cherry picked from commit 56d2c03)
_initialize_adapter() and JobAdapter's local path setup both did
if not os.path.isdir(path):
os.makedirs(path)
which is a check-then-act race: two adapters initializing against the same
project directory, or two processes sharing a project, can both pass the
isdir() check and the loser gets FileExistsError from makedirs().
Pass exist_ok=True so the create is idempotent. The isdir() guard is kept so
the common case still short-circuits.
(cherry picked from commit eeab94b)
pyproject.toml runs the suite with `-n auto --dist=worksteal` and CI uses
`-n 6`, but several adapter test modules build their scratch project directory
from a fixed path under arc/testing/. setUpClass runs once per worker that
receives any test of a class, and the teardowns rmtree that tree, so one worker
could delete input files out from under another worker's running test. The same
collision happens across modules that happened to pick the same directory name
(common_test.py and gaussian_test.py both used test_GaussianAdapter).
Derive each scratch directory from PYTEST_XDIST_WORKER (falling back to 'main'
when running serially) and give the colliding modules distinct base names.
Measured on this branch, running adapter_test + common_test + orca_test +
molpro_test + orca_neb_test three times each:
before: 8 failed, 37 passed, 9 errors
8 failed, 34 passed, 12 errors <- three tests lost to the race
8 failed, 37 passed, 9 errors
after: 8 failed, 37 passed, 9 errors (identical across all three runs)
The residual failures/errors are an unrelated local ess_settings gap ('server1'
is not configured on this machine) and are unchanged by this commit; the point
is that the results stop varying between runs.
(cherry picked from commit 93f281f)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addition of CREST Adapter that complements the heuristic adapter.
This pull request adds support for the CREST conformer and transition state (TS) search method to the ARC project, along with several related improvements and code cleanups. The most important changes include integrating CREST as a TS search adapter, updating configuration and constants, and enhancing the heuristics TS search logic for better provenance tracking and code clarity.
CREST Integration:
JobEnum(arc/job/adapter.py), included CREST in the list of adapters and RMG family mapping, and registered it as a default incore adapter (arc/job/adapters/common.py,arc/job/adapters/ts/__init__.py). [1] [2] [3] [4]arc/job/adapters/ts/crest_test.py).Makefile). [1] [2]Constants and Configuration:
angstrom_to_bohrconversion constant to both Cython and Python constants modules (arc/constants.pxd,arc/constants.py). [1] [2] [3]Heuristics TS Search Enhancements and Refactoring:
arc/job/adapters/ts/heuristics.py). [1] [2] [3] [4]arc/job/adapters/ts/heuristics.py). [1] [2] [3] [4] [5] [6] [7].