Skip to content

Compare the IRC bond-list fallback as condensed graphs of reaction, independent of the TS atom order - #1063

Open
calvinp0 wants to merge 1 commit into
mainfrom
fix_irc_fallback_atom_order
Open

calvinp0 wants to merge 1 commit into
mainfrom
fix_irc_fallback_atom_order

Conversation

@calvinp0

@calvinp0 calvinp0 commented Oct 1, 2026

Copy link
Copy Markdown
Member

The IRC bond-list fallback compared TS indices with reactant indices

When molecule perception of the two IRC endpoints fails, check_irc_species_and_rxn falls back to comparing bond lists. The endpoint bond lists are in the TS's atom order (the IRC and the endpoint optimizations keep it), while rxn.get_bonds() is in the reactants' atom order, and the two were compared index by index. No TS method guarantees that the TS lists its atoms in reactant order: AutoTST builds from SMILES, KinBot in direction R and the linear adapter for B + C → A use the product's order, and a user guess is arbitrary. A correct TS from any of these got ts_checks['IRC'] = False whenever perception failed.

The fallback now compares condensed graphs of reaction up to isomorphism. The endpoint graph has an edge for every bond in either endpoint, labelled by which endpoints hold it; the reaction graph is built the same way from rxn.get_bonds() (reactant and product bonds, both in reactant indices). One match with element-labelled atoms and labelled edges, in both endpoint orientations, decides the verdict. Matching the two sides as one graph also rejects an IRC whose endpoints are both the same well, which an independent check of each side would accept when the reactant and product graphs are isomorphic (e.g. an intramolecular H shift in ethyl). Endpoints with identical bond sets are rejected outright. The perception path, the verdict semantics and the logging are unchanged.

_check_equal_bonds_list, now unused, is removed with its test.

Tests (arc/checks/ts_test.py, perception patched off)

  • A TS and endpoints in reversed atom order are accepted; on main this gives False.
  • Ethyl 1,2-H shift: the true endpoint pair is accepted in both orders and in a permuted atom order; the same well at both ends is rejected.
  • H + CH4 ⇌ CH3 + H2, where the products list their elements in a different order than the reactants, is accepted, plain and permuted.
  • A wrong endpoint pair is still rejected.

arc/checks/ts_test.py: 39 passed.

Searched before writing: no existing helper compares bond graphs in arc/ (linear_utils/isomerization.py has an inline, specialised networkx matcher). _get_condensed_graph_of_reaction and _find_cgr_isomorphism have the same names and signatures as in #1059, so the two branches reduce to one copy when both are merged.

🤖 Generated with Claude Code

When molecule perception of the IRC endpoints failed, the fallback compared
the endpoint bond lists, which are in the TS atom order, directly against
rxn.get_bonds(), which is in the reactant atom order. No TS method guarantees
the TS lists its atoms in reactant order (AutoTST, KinBot direction R and the
linear B+C->A guesses use their own order, and user guesses are arbitrary),
so a correct TS could get ts_checks['IRC'] = False.

The fallback now builds the condensed graph of reaction of the endpoints, with
an edge for every bond in either endpoint labelled by which endpoints hold it,
and matches it against the condensed graph of the reaction built from
rxn.get_bonds(), up to isomorphism with element-labelled atoms and in both
endpoint orientations. Comparing the two sides as one graph means an IRC whose
endpoints are both the same well is rejected, which matters for reactions whose
reactant and product graphs are isomorphic, such as an intramolecular H shift in
ethyl. Endpoints with identical bond sets are rejected outright. The verdict
semantics and logging are unchanged, and the perception path is untouched.

The unused _check_equal_bonds_list and its test are removed.
@codecov

codecov Bot commented Oct 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.09%. Comparing base (d9f47ab) to head (3461be1).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1063      +/-   ##
==========================================
- Coverage   67.12%   67.09%   -0.04%     
==========================================
  Files         123      123              
  Lines       43602    43671      +69     
  Branches    11144    11150       +6     
==========================================
+ Hits        29270    29303      +33     
- Misses      11194    11215      +21     
- Partials     3138     3153      +15     
Flag Coverage Δ
functionaltests 67.09% <ø> (-0.04%) ⬇️
unittests 67.09% <ø> (-0.04%) ⬇️

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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant