feat: kekulize aromatic ions, delocalized ring anions, and connected organometallics - #248
Open
fbaensch-beilstein wants to merge 9 commits into
Open
feat: kekulize aromatic ions, delocalized ring anions, and connected organometallics#248fbaensch-beilstein wants to merge 9 commits into
fbaensch-beilstein wants to merge 9 commits into
Conversation
…kulization retry Supersedes the one-shot ReInitBnStruct retry. On BNS_ALTBOND_ERR, restore a snapshot of the pre-search atoms (taken only when an aromatic electron source is present, while ring bonds are still flexible type-4 BOND_ALTERN), relax the electron sources, then tear down and rebuild the network from a fresh AllocateAndInitBnStruct so vertex capacities re-derive from the relaxed atoms, and retry the conversion once. This is the general revert-to-flexible mechanism: it fixes charge-placement invariance (delocalized ring anions) and connected organometallic aromatics (ferrocene, cobaltocene) under MolecularInorganics, in addition to the plain aromatic ions. Failure-path-only; 0 regression diffs over the 4190-structure CI corpus.
Un-xfail the methyl-cyclopentadienyl-anion charge-placement invariance test (all five ring -1 placements now give one identical InChI). Add passing -MolecularInorganics tests for charge-explicit ferrocene and cobaltocene anion. The neutral heavy-atom ferrocene fixture stays xfail (no ring charge to relax) with a precise reason; the neutral Cp radical stays xfail (separate FIX_AROM_RADICAL path).
…n retry Addresses code-review findings on the flexible-bond rebuild retry: - The retry restored a pre-search snapshot that re-installed the FIX_AROM_RADICAL-neutralized radical state and never restored it, so a structure with BOTH a neutralized aromatic doublet radical AND a charged aromatic electron source reaching the retry would lose the radical. Mirror the normal path's stored_radicals restoration after the retry search (guarded by FIX_AROM_RADICAL). Latent (no such co-occurrence on the CI corpus), inert for every structure that does not reach the retry. - On snapshot inchi_calloc failure, set BNS_OUT_OF_RAM and exit instead of silently degrading to the no-retry path (consistent with the other allocation sites in this function).
fbaensch-beilstein
changed the base branch from
extract-aromaticity-utils
to
dev
August 19, 2026 11:32
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.
Stacked on #239 (base branch
extract-aromaticity-utils, which adds thearomaticity.{c,h}module this builds on). GitHub will retarget this todevautomatically once #239 merges. Supersedes #234; refs #154, #82.What this does
Aromatic systems that previously failed with
Error -9986 (Cannot process aromatic bonds)now kekulize into valid InChIs. InChI has no "aromatic" bond order — type-4 bonds are kekulized into localized single/double bonds by a balanced-network search that needs a perfect double-bond matching. Odd rings whose charge/radical center contributes an empty orbital, lone pair, or single electron (not a double bond) have no such matching and fail.The fix adds a Hückel-informed relaxation on the failure path only, then rebuilds the network from the original flexible bonds and retries once:
aromaticity.{c,h}—is_aromatic_electron_source(a ring atom that can't take a double bond: ring-coordination 2 excluding metal bonds, spare valence, charged or doublet-radical) andrelax_aromatic_electron_sources(moves one valence unit to an implicit H). Fully unit-tested; noBN_STRUCTdependency.mark_alt_bonds_and_taut_groups(ichi_bns.c), onBNS_ALTBOND_ERR: restore a pre-search snapshot (flexible type-4BOND_ALTERNbonds + original valences), relax the electron sources, tear down and rebuild the network from a freshAllocateAndInitBnStruct(so vertex capacities re-derive from the relaxed atoms), and retry the conversion once. Restoring the flexible bonds lets the search itself find the matching — so charge position stops mattering and metal-bonded rings resolve in place.Because it runs only after the first kekulization already errored, every structure that kekulized before is byte-identical.
Now processes correctly
InChI=1S/C3H3/c1-2-3-1/h1-3H/q+1InChI=1S/C5H5/c1-2-4-5-3-1/h1-5H/q-1InChI=1S/C7H7/c1-2-4-6-7-5-3-1/h1-7H/q+1InChI=1S/C6H7/c1-6-4-2-3-5-6/h2-5H,1H3/q-1(charge-placement invariance)-MolecularInorganics)InChI=1B/C10H10Fe/…/q-2-MolecularInorganics)InChI=1B/C10H10Co/…/q-1Deferred (strict
xfail, flip loudly when fixed)FIX_AROM_RADICALpath, before this hook.-MolecularInorganicspath is the one addressed here.Verification
test_aromaticitycases).test_aromatic_ions.py7 passed / 2 xfailed; full suite 19 passed / 3 skipped / 9 xfailed, no failures.Guarantee: no regression on any structure that already produced an InChI; new InChIs appear only for inputs that previously errored with -9986. The one-shot retry either produces a valid kekulization or preserves the original error.
Relationship to #234
Supersedes #234 (
fix/aromatic-ion-valence), whose charge-center relaxation was the starting point; itstest_aromatic_ions.pycases are carried over here. #234 can be closed in favor of this.