Place the retro-ene migrating H on the recipe acceptor - #1036
Open
calvinp0 wants to merge 1 commit into
Open
Conversation
RMG's Retroene recipe forms *1-*6, where *1 is the terminus of the multiple bond with *2 -- the carbonyl O of an ester -- and *6 is the migrating H. build_retroene_ts ignored the acceptor it was handed in forming_bonds and re-derived *3 (the ester O, disqualified by the Root group, which requires it to carry two heavy neighbours) structurally instead, so on CC(=O)OCC(C)C <=> CC(=O)O + C=C(C)C the H was placed 1.380 A from O3 while the bond the recipe actually forms, O2-H13, was left at 2.763 A. Passing the correct forming_bonds=[(2, 13)] produced byte-identical output. Read the acceptor from forming_bonds[0] and drop the carbonyl_c / carbonyl_o structural search, which also unblocks all-carbon retro-ene (*1 = C), where that search returned None and the builder produced no guess at all. That search was the only check on the caller's acceptor, so the recipe's own requirement replaces it: *1 must be two bonds from *3. The ring closure targeted (carbonyl_o, leaving_c), an O2...C4 contact that appears in no recipe action and was measured at 2.199 A. It cannot simply be deleted: without it the donor-acceptor separation stays at its reactant value (4.193 A on the ester, 4.850 A on 1-pentene), which is outside the window two_sphere_intersection can triangulate, so the H falls back to a collinear placement 2.68 A from the acceptor. Targeting the recipe forming pair (acceptor, mig_h) is not reliable either -- ring_closure_xyz reaches its target only approximately there, and the implied donor-acceptor separation came out molecule-dependent (2.785 A on isobutyl acetate but 2.954 A on ethyl acetate, the latter again untriangulable). The closure now targets the donor-acceptor separation directly, which ring_closure_xyz reproduces to within 0.03 A, at the value implied by a 126-degree bridge angle. That is the six-ring strain minimum for these seed distances: the ring-angle sum is 638.7 there against 646.4 at 152 degrees, and the donor angle *5 holds 91.5 degrees rather than collapsing to 76.5. Should the bridging-H distances shrink, the angle must rise with them. Because the closure now spans a 1-5 pair rather than a 1-3 one, a closure failure would leave the two spheres disjoint and emit a guess whose forming bond never formed; the builder now returns None in that case. The migrating H's two radii were hard-coded to C-H and O-H regardless of the elements actually present; they now come from the donor and acceptor symbols, which matters for the all-carbon case where the acceptor radius must be C-H (1.51 A) rather than O-H (1.38 A). The unreachable `or 1.09` / `or 0.97` fallbacks are gone -- get_single_bond_length returns 1.75 for an unknown pair and never a falsy value. The triangulation itself was a fourth open-coded copy of local_geometry.clean_migrating_h and now calls it. That helper picked the H's azimuth from the H's current position, which here is wherever ring_closure_xyz left it -- and since the donor is now a ring endpoint, its substituent rotation swings the migrating H away from the acceptor, leaving the H 91.6 degrees from where it closes the ring and 0.42 A out of the ring plane. clean_migrating_h takes an optional ref_pos, defaulting to the previous behaviour; the builder passes the point that puts the H opposite the rest of the ring, giving 180.0 degrees and 0.00 A. The docstring's ASCII ring drew *6-*5-*4-*1-*2-*3-*6, the true ring with the oxygens' roles rotated; it now draws *6-*5-*4-*3-*2-*1-*6. Both tests enshrined the old behaviour: the unit test passed forming_bonds=[(3, 13)] and the integration test asserted H13 approaching O3 and required d(O2-C4) < 2.5. They now check the recipe bond O2-H13, assert that O3-H13 and O2-C4 are *not* short, and check the bridge angle, the ring closure and both new guards. The integration test's byte-exact expected_ts golden is replaced by tolerance-based distance assertions, and the pins on the 2.5 A sigma stretch are widened so they no longer fix an arbitrary construction constant to 2%. A new test pins all-carbon retro-ene on 1-pentene.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1036 +/- ##
==========================================
+ Coverage 65.53% 65.59% +0.06%
==========================================
Files 120 120
Lines 40468 40460 -8
Branches 10430 10426 -4
==========================================
+ Hits 26519 26539 +20
+ Misses 10946 10927 -19
+ Partials 3003 2994 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
build_retroene_ts()placed the migrating hydrogen on the ester oxygen. The family recipe puts it onthe carbonyl oxygen.
The recipe
RMG-database/input/kinetics/families/Retroene/groups.py:20-28:*6is the migrant andFORM_BOND *1 *6bonds it to*1, the terminus of the multiple bond with*2— the carbonyl oxygen for an ester.*3, the ester oxygen, is disqualified by the Root group,which requires it to carry two heavy neighbours (
1 *3 R!H u0 {2,S} {3,[S,D]}). The remainingactions are the Ei product:
CHANGE_BOND *1 *2 -1breaks the C=O,CHANGE_BOND *2 *3 +1forms thenew one.
What the builder did
It used
forming_bondsonly to identify which hydrogen migrates, then re-derived the acceptorstructurally as the member of the breaking sigma bond not bonded to the donor — which is
*3byconstruction. Passing the correct
forming_bondsproduced byte-identical output, so the argument wasbeing discarded entirely.
For
CC(=O)OCC(C)C ⇌ CC(=O)O + C=C(C)C, whose family machinery generatesforming_bonds = [(2, 13)]:The guess reached the ESS.
linearis in the defaultts_adaptersandRetroeneis registered forit;
interpolate_additionends at_finalize_ts_guesses(..., path_spec=None), which applies onlycolliding_atoms.has_misdirected_migrating_hwould have rejected it — by 3.2 mÅ — but runs in adifferent pipeline. The remaining check is NMD, which takes the recipe first and demands the O2–H13
mode, so rejection came after a full TS optimisation and frequency job.
The fix
The acceptor comes from
forming_bonds[0]. Thecarbonyl_c/carbonyl_osearch is removed infavour of the recipe's own constraint, and the hard-coded
C–HandO–Hradii are taken from theactual elements.
Step 1, which stretches the breaking sigma by repositioning the leaving carbon, is unchanged — that
is a recipe bond.
Step 2's ring closure targets the donor–acceptor pair. It cannot be removed: without it the gap stays
at 4.193 Å for the ester and 4.850 Å for 1-pentene, outside
two_sphere_intersection's window, andthe hydrogen lands collinear 2.68 Å from the acceptor. Targeting the recipe pair directly is also
unusable, because
ring_closure_xyzonly approximates its target and the implied gap comes outmolecule-dependent — 2.785 Å on isobutyl acetate against 2.954 Å on ethyl acetate. Targeting
(acceptor, donor)reproduces its target to within 0.03 Å on every substrate tested.Making the donor a ring endpoint exposed
ring_closure_xyz's substituent rotation, which left thehydrogen 91.6° from closing the ring and 0.42 Å out of plane;
clean_migrating_hgained an optionalref_posso the azimuth is taken from the ring, giving 180.0° and 0.00 Å. A failed fold now returnsNonerather than a silently bad guess, and the caller's acceptor is checked for adjacency.The bridge angle is 126°. At the seed distances this builder currently works from, a wider angle
compresses the donor angle below its physical range — 152° drives it to 76.5° against a measured
95–105°.
Non-ester retro-ene
Taking the acceptor from the recipe also reaches substrates the structural search could not. 1-pentene
⇌ propene + ethylene previously returned
None; it now yields C···H···C at 1.510 / 1.510, sigma 2.500,bridge 125.7°.
Tests
Both existing tests encoded the old behaviour — the unit test hard-coded
forming_bonds=[(3, 13)],and the integration test asserted "H13 migrating from C5 toward O3" plus a requirement on the
spurious O2···C4 contact. Both are rewritten against the generated bonds, and the integration test's
byte-exact
expected_tsgolden is replaced with tolerance-based assertions on the distances thatmatter.
779 pass across
arc/job/adapters/ts/. Validated by mutation: seven mutants, each killed by a test —acceptor reverted to the ester oxygen, radii reverted to hard-coded C/H and O/H, Step 2 deleted,
Step 2 retargeted, the azimuth reference dropped, the triangulability guard removed, the adjacency
guard removed. The last of these initially survived, since no fixture fails ring closure, and is now
covered by a mocked test.
Reuse
Searched before writing:
get_code_contextfor existing migrating-hydrogen placement,git grep -nfor the
PAULING_DELTAuse sites, andarc/species/vectors.pyfor the geometry primitives. Theazimuth fix reuses
clean_migrating_hrather than adding a fourth open-coded copy of that placement.🤖 Generated with Claude Code
https://claude.ai/code/session_01MbGeU8wLpafo3YFzTky2ho