Skip to content

Repair the Gaussian ESS troubleshooting ladder and route construction - #1006

Open
calvinp0 wants to merge 4 commits into
mainfrom
feature_gaussian_trsh_remedies
Open

Repair the Gaussian ESS troubleshooting ladder and route construction#1006
calvinp0 wants to merge 4 commits into
mainfrom
feature_gaussian_trsh_remedies

Conversation

@calvinp0

Copy link
Copy Markdown
Member

Repairs the Gaussian troubleshooting path, which a 500-reaction benchmark showed was frequently resubmitting jobs unchanged.

arc/job/trsh.py — the ESS remedy ladder

Fixes an SCF qc/no_xqc oscillation that consumed the entire retry budget without ever reaching the rest of the SCF ladder. Adds a non-retryable-error gate so genuine dead ends refuse immediately instead of burning a resubmit. Reclassifies three errors that were wrongly treated as fatal:

error was now
ZMat (l716) abandoned retry via opt=(cartesian)
l202 OptOrientation abandoned retry via nosymm
GL401 basis projection abandoned retry as CheckFile (drop the checkfile)

The MaxOptCycles ladder is reworked to recompute the Hessian before changing step algorithm, and to be transition-state aware — GDIIS/GEDIIS are skipped for saddle points, and a fine TS opt gets an early Cartesian rung. That requires the scheduler to pass species.is_ts through, which is the one-line arc/scheduler.py commit.

arc/job/adapters/gaussian.py — route construction

Makes the no_xqc opt-out actually reachable; stops silently dropping int=(Acc2E=14) on non-fine opt/IRC jobs; collapses two conflicting DFT integration grids into a single integral=() keyword.

Gates guess=mix to bi-radical singletsmultiplicity == 1 and number_of_radicals > 1 — the only configuration with an α-β symmetry left to break. This supersedes two earlier predicates: an is_ts-based one on this branch, and #973's not all(restricted_flags), which would still emit the keyword for every ordinary doublet and triplet. #973 is closed in favour of this.

Evidence from ARC's own archived logs: arc/testing/restart/1_restart_thermo/calcs/freq_a19031.out is a closed-shell singlet (9 alpha electrons 9 beta electrons, no u prefix) whose L401 prints Mixing orbitals ... Coef= 7.07106781D-01 7.07106781D-01 — so g16 does apply guess=mix on a restricted reference rather than ignoring it; it perturbs a good Harris guess for no benefit. Systematic across 24 fixtures: restricted → one mixing line, unrestricted → two. Note number_of_radicals is user-input-only (arc/species/species.py:210), so the correct gate rarely fires today; measuring it properly needs a wavefunction stability check, tracked separately.

arc/imports.py

Skips a developer's ~/.arc overlays under pytest, so local runs see the same settings as CI.

Deliberately dropped during the rebase

This branch previously classified Gaussian galloc failures as MemoryOverallocation and halved the memory reservation. main's 5b5b11b9 landed a different remedy for the same failure — classify as GaussianMemoryAllocation and step the %mem fraction down while holding the reservation — and its message explicitly rebuts halving as "equally wrong, and strictly worse". That commit is later and is already on the target, so this branch's version was dropped in full, along with its two tests, which shared method names with main's and would otherwise have silently shadowed them. arc/testing/trsh/gaussian/galloc.out is byte-identical on both sides, so the duplicate fixture went too.

Verification

Rebased from 157 behind with zero unexplained content loss: the changed-line multiset of old-base..old-head versus main..new-head shows 0 only-in-new lines and 170 only-in-old, all of them the deliberately dropped galloc commit. Tests cannot prove a rebase didn't drop a hunk; this can.

Restructured 15 commits → 4 so no file is touched by more than one commit; git diff between the pre- and post-restructure heads was empty, so the regrouping moved lines between commits without changing one.

One post-rebase adaptation: test_doublet_ts_has_no_guess_mix built a multiplicity-2 TS from an O/H/H geometry (10 electrons), which main now rejects under multiplicity/electron-count parity. Replaced with an H + H₂ abstraction TS (3 electrons).

109 passed across trsh_test, gaussian_test, imports_test, scheduler_test. Full arc/job/: 1156 passed, 36 skipped, 5 pre-existing torch_ani_test environmental failures.

🤖 Generated with Claude Code

Copilot AI lite review requested due to automatic review settings August 22, 2026 07:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codecov

codecov Bot commented Aug 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.67%. Comparing base (45d73a0) to head (4cf9fb0).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1006      +/-   ##
==========================================
+ Coverage   64.60%   64.67%   +0.07%     
==========================================
  Files         119      119              
  Lines       39785    39831      +46     
  Branches    10307    10320      +13     
==========================================
+ Hits        25703    25761      +58     
+ Misses      11105    11097       -8     
+ Partials     2977     2973       -4     
Flag Coverage Δ
functionaltests 64.67% <ø> (+0.07%) ⬆️
unittests 64.67% <ø> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@calvinp0
calvinp0 force-pushed the feature_gaussian_trsh_remedies branch from 9879b13 to 8987718 Compare August 23, 2026 08:56
arc/imports.py always loads arc/settings/settings.py as the baseline and then
layers a developer's personal ~/.arc/{settings,submit,inputs}.py on top with a
top-level dict update, which shallow-replaces whole dicts rather than merging
them. A personal ~/.arc/settings.py that defines its own `servers` therefore
deletes the repo's `server1` fixture, and every test that relies on it fails
locally with KeyError: 'server1' while passing in CI - a difference between a
developer's machine and CI that has no diagnostic value and repeatedly costs
time to rediscover.

Add _local_overlays_disabled() and gate all three overlay loads on it. The
overlays are skipped when pytest is loaded ('pytest' in sys.modules, true from
collection onward) or when ARC_IGNORE_LOCAL_SETTINGS=1 is set, so a test run
sees exactly the settings CI sees, and the env var gives an explicit escape
hatch outside pytest. Production runs are unaffected: neither trigger fires,
and the overlay loading itself is unchanged.
A benchmark run of ~500 reactions exposed several ways in which the Gaussian
branch of trsh_ess_job() burned its retry budget without ever changing the job
it resubmitted. This commit reworks that ladder in arc/job/trsh.py, with tests
in arc/job/trsh_test.py that fail on the previous behaviour.

SCF qc/no_xqc oscillation
    The quadratic-convergence remedy was guarded on the token 'no_qc', which is
    never appended to ess_trsh_methods; the token actually recorded by
    trsh_keyword_no_qc() (and consumed by the Gaussian adapter) is 'no_xqc'.
    An l508 failure therefore dropped 'scf=(qc)' and recorded 'no_xqc', the next
    SCF failure re-added 'scf=(qc)', and the cycle repeated - never reaching the
    rest of the SCF ladder (NDamp, NoDIIS, guess=INDO, Fermi, Noincfock,
    NoVarAcc). Align the three guards to 'no_xqc', count a dropped 'scf=(qc)' as
    attempted for the last-resort gate that previously required 'scf=(qc)' to
    still be present, and make the 'no_xqc' append idempotent.

Refuse genuinely non-retryable errors
    trsh_keyword_intaccuracy fired on every Gaussian error, so input/method
    errors that no resubmit can fix picked up a spurious int=(Acc2E=14) step and
    wasted a resubmit before hitting all_attempted. Add
    GAUSSIAN_NON_RETRYABLE_KEYWORDS and a refusal branch in the shared if/elif
    chain so those classes set couldnt_trsh immediately with a clear error.
    Deliberately conservative: SCF, opt-cycles, internal-coordinate,
    negative-eigenvalue, memory, checkfile and the generic 'Unknown' class
    (which the scheduler retries on another node) are untouched.

Three errors reclassified out of that refusal, each with a real remedy
    ZMat (l716, "angle in z-matrix outside the allowed range"): ARC always
    submits Cartesian geometries, so this is never a bad input - it is the
    collinear-angle degeneracy that Cartesian optimization sidesteps. Make
    trsh_keyword_cartesian fire on 'ZMat' as well as 'InternalCoordinateError'.
    OptOrientation (l202, standard orientation / point group changed
    mid-optimization): a symmetry glitch that nosymm cures, the same remedy
    l101/l103 already use. Add 'NoSymm' to the l202 keyword list so
    trsh_keyword_nosymm fires exactly once.
    GL401 basis-set projection failure: raised when guess=read reads a
    checkpoint built with a different basis. Reclassify it as ['CheckFile'],
    mirroring the sibling checkpoint-data-missing cases, so dropping the
    checkfile removes guess=read and the job restarts from a fresh SCF guess.
    'BasisSet' then leaves the non-retryable set entirely: the remaining
    BasisSet paths (GL301 "atomic number out of range", "unrecognized basis
    set") are already caught by the dedicated BasisSet refuse branch, so keeping
    it there would be dead and misleading.
    In each case a recurrence still terminates via 'all_attempted'.

TS-aware opt-cycle ladder with Hessian recomputation
    Rework the MaxOptCycles ladder to recompute the Hessian before flipping the
    step algorithm, and to know whether it is optimizing a saddle point:

        maxcycle=200 -> [cartesian, fine TS only] -> recalcfc=5 -> calcall
                     -> RFO -> [GDIIS -> GEDIIS, minimization only]

    Recomputing force constants is the first-line remedy for a stuck
    optimization and is essential for a TS opt, where following the single
    negative eigenvalue depends on Hessian quality. GDIIS/GEDIIS are
    minimization-oriented step accelerators that can walk a TS opt downhill into
    a nearby minimum and lose the saddle, so they are skipped when is_ts is
    True while RFO eigenvector-following is kept. is_ts is threaded from the
    scheduler through trsh_ess_job into trsh_keyword_opt_maxcycles and
    prioritize_opt_methods, which now keeps a single step algorithm and
    collapses the force-constant family to the most aggressive directive
    (calcall > recalcfc=* > calcfc).
    A fine TS opt that dead-ends on the l9999 "optimization stopped"
    oscillation additionally gets opt=(cartesian) as an early rung, right after
    the cheap maxcycle bump and before the expensive Hessian escalation: that
    failure is a redundant-internal-coordinate oscillation Cartesian
    coordinates sidestep, and it was previously losing chemically sensible TS
    guesses. Ground-state opts and coarse TS opts are unchanged. This also
    fixes a latent cartesian-loss in mixed GL103 + MaxOptCycles histories,
    where the opt-route rewrite could clobber a standalone opt=(cartesian).

Cleanup, no behaviour change
    trsh_keyword_inaccurate_quadrature: remove the final elif. It guarded on
    'int=ultrafine', a token never added to ess_trsh_methods, so the guard was
    always true; the branch never advanced ess_trsh_methods and only re-appended
    keywords that combine_parameters already dedupes, firing solely on the
    terminal retry that trsh_ess_job discards via 'all_attempted'.
    trsh_keyword_unconverged: correct a copy-paste docstring.

arc/testing/trsh/gaussian/l401_projection.out is a new fixture for the GL401
reclassification. The new tests pin the escalation order and the termination of
the InaccurateQuadrature and MaxOptCycles ladders, simulate the SCF retry cycle
end to end, and assert that each reclassified error now yields its remedy while
the remaining non-retryable classes still refuse.
The MaxOptCycles remedy ladder in arc/job/trsh.py now branches on whether the
job optimizes a transition state: GDIIS/GEDIIS are skipped for a saddle point
(they can walk the optimization downhill into a nearby minimum), and a fine TS
opt gets opt=(cartesian) as an early rung. That decision cannot be made inside
trsh_ess_job, which sees only the job status and the software.

Supply it from the one place that knows: the scheduler already holds the
ARCSpecies for the label being troubleshot, so pass
self.species_dict[label].is_ts alongside the is_h and is_monoatomic flags it
already forwards. No other scheduler behaviour changes.
…ss=mix

Four defects in the route the Gaussian adapter renders, all of which made a
troubleshooting resubmit either a no-op or actively wrong. Tests live in
arc/job/adapters/gaussian_test.py and assert the specific keyword rather than a
whole-route golden wherever the route is otherwise unrelated.

The qc -> xqc upgrade could not be opted out of
    A requested scf=(qc) is upgraded to scf=(xqc), and an l508 failure is
    supposed to record 'no_xqc' to stop that. The check read
    'no_xqc' in self.args['trsh'].values(), which can never match the shape the
    scheduler builds ({'trsh': [keyword_list]}), so the opt-out never fired and
    the SCF ladder oscillated between qc and no_xqc. Decide from
    ess_trsh_methods instead.

int=(Acc2E=14) was silently dropped on non-fine jobs
    integral_algorithm was only emitted under `if self.fine`, so a non-fine opt
    or IRC whose only remedy was int=(Acc2E=14) produced a route byte-identical
    to the un-troubleshot one - a guaranteed no-op resubmit. Capture
    acc2e_requested and emit a bare integral=(Acc2E=14) through the route's fine
    slot for non-fine opt/IRC jobs. Fine jobs still fold Acc2E into the
    ultrafine grid, and non-fine jobs without the remedy are unchanged.

Two conflicting DFT integration grids in one route
    The InaccurateQuadrature remedy escalates to a finer grid recorded as
    'int=grid=NNNMMM', but a fine job also emits integral=(grid=ultrafine, ...).
    Both are the same Gaussian Int keyword, so the route carried two grid specs
    (ultrafine = (99,590) against the remedy's 300590 = (300,590)) and the finer
    grid was either rejected or silently overridden. Fold the remedy grid into
    the single integral=() keyword, replacing ultrafine, and drop the standalone
    int=grid= token. The grid is sourced from ess_trsh_methods so it persists
    while the ladder keeps escalating, and the non-fine opt/IRC path emits it
    too. job_18 loses its duplicated grid; job_20/21 now keep grid=300590
    instead of reverting to ultrafine; job_19, which has no grid remedy, is
    unchanged, confirming the change is scoped to InaccurateQuadrature routes.
    Relatedly, the base route's calcfc is dropped when the MaxOptCycles ladder
    supplies recalcfc/calcall, so opt=() never carries conflicting Hessian
    directives; the job_23/job_24 route fixtures are updated to the new ladder.

guess=mix is now gated on Ms = 0 broken symmetry only
    The adapter emitted guess=mix for every polyatomic species with no
    checkfile. The gate is now

        elif any(spc.multiplicity == 1 and spc.number_of_radicals is not None
                 and spc.number_of_radicals > 1
                 for spc in self.species):

    This predicate deliberately supersedes two earlier attempts. An earlier
    commit on this branch gated on `spc.is_ts or (multiplicity == 1 and
    number_of_radicals > 1)`; draft PR #973 hoisted is_restricted(self) and
    gated on `not all(restricted_flags)`. Both are replaced.

    g16 does not ignore guess=mix on a restricted reference - it applies it.
    arc/testing/restart/1_restart_thermo/calcs/freq_a19031.out is a closed-shell
    singlet (9 alpha, 9 beta electrons; route `#P guess=mix wb97xd/def2tzvp`,
    no `u` prefix) whose L401 prints a single "Mixing orbitals, IMix= 1 ...
    Coef= 7.07106781D-01 7.07106781D-01", then IOpCl= 0 and a normal E(RwB97XD).
    Across the fixture tree a restricted reference prints one mixing line, an
    unrestricted one prints two (alpha +0.7071, beta -0.7071), and a job without
    the keyword prints the Harris functional line instead. On a restricted
    reference the keyword rotates HOMO into LUMO within the single orbital set:
    it is not inert, it perturbs a good Harris guess for no benefit. #973's log
    text "guess=mix only acts on an unrestricted reference" is therefore false
    and is not carried over.

    is_restricted is not usable as the gate. is_species_restricted() early-
    returns True for force_field, composite and semiempirical method types
    before any multiplicity check, so it answers "should the route omit the `u`
    prefix", not "is the reference restricted".
    arc/testing/composite/TS0_composite_2043.out is the counterexample:
    multiplicity 2 at cbs-qb3 with no `u` in the route, yet Gaussian prints two
    mixing lines and SCF Done: E(UB3LYP). The new predicate reads multiplicity
    and number_of_radicals directly and never consults is_restricted, which is
    left untouched.

    The physical rule is that guess=mix is wanted only where the reference is
    genuinely unrestricted and Ms = 0, i.e. a bi-radical singlet: only Ms = 0
    still has an alpha-beta symmetry left to break, which is Gaussian's own
    documented use case ("producing UHF wavefunctions for singlet states"). For
    Ms != 0 systems Nalpha != Nbeta already, so guess=mix degenerates into a
    pure spatial symmetry-breaking request, which for delocalised radicals
    (allyl, benzyl, NO2) drives the classic artificial-localisation artifact.
    Doublets, triplets and non-singlet TSs - which the earlier `spc.is_ts`
    disjunct still admitted - therefore no longer get it.

    Honest consequence: number_of_radicals is user-input-only (ARC does not
    attempt to determine it), so where the user did not declare it the correct
    gate emits guess=mix never. That is accepted deliberately; no heuristic is
    invented to derive it. Whether singlet TSs deserve an unrestricted reference
    is a separate physics question under separate review.

    input_dict['job_type_1'] is rendered once and reused for every --link1--
    section while ${restricted} is substituted per species, so a route-wide
    guess keyword lands on every section. The keyword stays route-wide and the
    predicate collapses with any(), the honest collapse for "does any species
    here need a broken-symmetry seed". Making guess per-species in the
    run_multi_species path would require restructuring the whole route-wide
    assembly (opt=, irc=, scf=, SCRF=, integral=, plus the Fix OPT / Fix SCF /
    Fix IRC recombination that follows) and is out of scope. The pre-existing
    `self.species[0].number_of_atoms > 1` guard is likewise left as found.

    guess=read (checkfile) and guess=INDO (troubleshooting) keep their existing
    precedence and ordering.

    TestGaussianAdapterGuessMixGating covers: bi-radical singlet -> guess=mix;
    closed-shell singlet, doublet and triplet -> none; singlet TS with
    number_of_radicals=None -> none (#973's target); doublet TS -> none (the
    earlier predicate's target); bi-radical singlet TS -> guess=mix; open-shell
    species at cbs-qb3 decided by multiplicity; checkfile -> guess=read; trsh
    guess=INDO -> guess=INDO only. The checkfile fixture lives in a
    tempfile.mkdtemp() directory registered with addCleanup rather than in
    ARC_TESTING_PATH/test_GaussianAdapter, which another module's tearDownClass
    rmtree's and which caused intermittent failures under
    `pytest arc/ -n 6 --dist worksteal`.
@calvinp0
calvinp0 force-pushed the feature_gaussian_trsh_remedies branch from 8987718 to 4cf9fb0 Compare August 23, 2026 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants