Skip to content

Fix arkane lonepair adjlist main#919

Open
calvinp0 wants to merge 3 commits into
mainfrom
fix_arkane_lonepair_adjlist_main
Open

Fix arkane lonepair adjlist main#919
calvinp0 wants to merge 3 commits into
mainfrom
fix_arkane_lonepair_adjlist_main

Conversation

@calvinp0

@calvinp0 calvinp0 commented Jul 22, 2026

Copy link
Copy Markdown
Member

This pull request makes several improvements to how Arkane input and output handling is performed, focusing on robust detection of Arkane run success, improved SMILES and adjacency list handling for edge cases, and enhanced logging and test coverage. The most important changes are as follows:

Arkane Run Success Signal and Stderr Handling:

  • The criterion for a successful Arkane run is now the presence of output.py rather than the absence of stderr output. Cosmetic or unrelated stderr lines no longer cause false failures; instead, only missing output.py is treated as a failure. Real errors in stderr are logged for visibility but do not block successful runs if output is present.
  • Introduced helper functions _classify_arkane_stderr and _summarize_arkane_stderr to filter cosmetic warnings from stderr and summarize real errors for logging.

SMILES and Adjacency List Handling:

  • When generating Arkane input, if a species' SMILES string does not round-trip to the correct multiplicity (e.g., singlet carbenes like [CH2]), the input now uses an adjacency list instead of SMILES to avoid Arkane errors. Normal species still use SMILES. [1] [2]

Testing Improvements:

  • Added tests to verify that lone-pair singlet species are written using adjacency lists, and that normal species continue to use SMILES.
  • Added comprehensive tests for the new Arkane run success logic, covering cases with cosmetic stderr, real errors, missing output, and pre-flight checks. Also added direct tests for the new stderr classification and summarization helpers.

Imports and Minor Refactoring:

  • Updated imports to include AtomTypeError and Molecule where needed for the new logic.

These changes make Arkane integration more robust, prevent false negatives due to non-critical stderr output, and ensure edge cases in species representation are handled correctly.

Copilot AI review requested due to automatic review settings July 22, 2026 10:57

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.

@calvinp0
calvinp0 force-pushed the fix_arkane_lonepair_adjlist_main branch from 71eb59d to ba9508a Compare July 22, 2026 11:06
calvinp0 added 3 commits July 22, 2026 14:32
SMILES cannot encode a lone-pair singlet (e.g. singlet carbene [CH2], mult 1, C u0 p1):
Arkane re-perceives SMILES('[CH2]') as a u2 biradical, and u2+multiplicity=1 violates
Hund's rule when save_thermo_lib writes/reloads the adjacency list, crashing the whole
thermo step (reaction 01). When a species' SMILES doesn't round-trip to the same
multiplicity, emit the lossless adjacency list instead. Validated: RMG parses the u0 p1
carbene adjlist cleanly (the call that crashed).
Extracted from the arcbench commit ca7ce14 ("Expand output.yml calculation
provenance"): the stderr-classification machinery only, none of that commit's
output.yml provenance changes. Makes run_arkane's success signal the presence of
output.py rather than stderr-emptiness, and adds _classify_arkane_stderr plus the
_ARKANE_STDERR_IGNORABLE_PHRASES cosmetic-noise filter. stderr is now advisory:
non-cosmetic lines are logged (WARNING when output.py exists, ERROR when not) but
don't gate the return value. Adds TestRunArkaneOutputPySignal and
TestClassifyArkaneStderr.
When Arkane emits errors but still produces output.py (e.g. a first attempt that failed on the
Eckart submerged-barrier ValueError and was then retried with tunneling off), run_arkane dumped the
entire multi-line Python traceback into arc.log at WARNING. Log the salient exception line plus a
count instead ("ValueError: ... [+N more stderr line(s)]"), preferring the real Python exception
over the conda wrapper's non-zero-exit line. Same condensing applied to the genuine-failure ERROR
path (full traceback remains in the run dir's stderr.log). Adds tests.

(Cherry-picked from arcbench 7568c61. Also reconciles the inherited
TestRunArkaneOutputPySignal.test_real_error_with_output_returns_true_and_warns assertion from the
old 'non-cosmetic lines' wording to 'still produced output.py' to match the condensed WARNING
message this commit introduces — the same alignment arcbench made in fix_stale_test_expectations.)
@calvinp0
calvinp0 force-pushed the fix_arkane_lonepair_adjlist_main branch from ba9508a to b0547f6 Compare July 22, 2026 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants