Make the RMG family set configurable via settings, resolved in get_all_families - #978
Open
calvinp0 wants to merge 3 commits into
Open
Make the RMG family set configurable via settings, resolved in get_all_families#978calvinp0 wants to merge 3 commits into
calvinp0 wants to merge 3 commits into
Conversation
calvinp0
force-pushed
the
feature_configurable_rmg_family_set
branch
2 times, most recently
from
August 13, 2026 16:13
a68e891 to
24becfe
Compare
calvinp0
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #978 +/- ##
==========================================
+ Coverage 64.46% 64.57% +0.11%
==========================================
Files 119 119
Lines 39636 39725 +89
Branches 10276 10302 +26
==========================================
+ Hits 25550 25654 +104
+ Misses 11102 11080 -22
- Partials 2984 2991 +7
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 was referenced Aug 13, 2026
calvinp0
force-pushed
the
feature_configurable_rmg_family_set
branch
3 times, most recently
from
August 18, 2026 11:59
361976b to
c73b1ea
Compare
calvinp0
force-pushed
the
feature_configurable_rmg_family_set
branch
from
August 22, 2026 07:58
c73b1ea to
27f32a6
Compare
…ection and radicals
ARC hard-coded the 'default' RMG family set. It is the signature default of
get_all_families() and of every entry point above it, so there was no way to ask ARC to
consider anything else short of naming a set at each call site. That set is RMG's
recommended list, which is right for mechanism generation but excludes families ARC's TS
adapters declare support for, so a run targeting specific reactions could not reach them
at all.
A new top-level setting, rmg_family_set, now supplies that value, and it is resolved
inside get_all_families() -- the single sink every path funnels through, and which already
owned an in-body fallback for its own default. get_reaction_family_products() and
check_family_name() propagate None to mean "not specified".
Resolving it in the sink rather than in each signature is the point. Threading the setting
through the signatures instead leaves get_all_families()'s own default at the literal
'default', so every call site that does not pass the setting on keeps returning the curated
set. Measured on a deployed installation whose ~/.arc/settings.py asked for 'all':
settings['rmg_family_set'] read back as 'all', an explicit get_all_families('all') returned
100 families, and a bare get_all_families() returned 55; check_family_name() was exactly
such a bare call site.
Setting rmg_family_set to 'all' is only useful if 'all' can reach the families in question,
and it could not: it unioned the family sets named in RMG's recommended.py, and families
such as Intra_RH_Add_Exocyclic and Intra_RH_Add_Endocyclic appear in none of them despite
shipping in the database. get_all_families() now also unions the families that exist as RMG
database directories, listed by the new get_rmg_family_directories(), which counts a
directory as a family only when it holds a groups.py template. The union is de-duplicated,
which also removes the 24 duplicate labels the recommended sets alone produced.
The directories are appended after the recommended sets, so a family reachable only through
them is always positioned last and wins only where no recommended family matched. Every
candidate still has to reproduce the products the reaction asserts, so such a family cannot
introduce a transformation, only propose a mechanism for one already stated. That ordering
is a contract rather than an accident, so it is stated in the docstring and pinned by a
test.
This redefines what 'all' means, and 'all' is not only reachable through the setting: the
Linear TS adapter asks for it unconditionally whenever the configured set yields no product
dicts. An installation that never touches the setting is therefore affected, which a test
over that path records -- 1,4-cyclohexadiene <=> benzene + H2 has no family under the
shipped default and resolves to H2_Loss under that retry.
get_reaction_family_products()'s docstring claimed 'all' excludes surface families outright,
and two further statements repeated it. 'all' skips family sets whose label contains
'surface' and surface family directories, but a set with a non-surface label can still list
one, as electrochem does for the Surface_Proton_Electron_Reduction_* families. The three
statements now say what the code does, and a test pins it.
Determinism. get_all_families() built its candidate list from the set literals in
RMG-database's recommended.py (via ast.literal_eval) and from os.listdir(). Both iteration
orders are unspecified: the set order depends on PYTHONHASHSEED, so the same reaction on the
same code could be assigned a different family from one process to the next. Measured on
[CH]1C=Cc2ccccc21 <=> [CH]1C=CC2C3=C1C=CC32, ARC reported Intra_Diels_alder_monocyclic at
seeds 0/1/13 and Intra_R_Add_Exocyclic at 2/5; [CH2]C(C=C)OS >> [O]C(C=C)CS flipped between
intra_OH_migration and intra_substitutionS_isomerization. About 10% of the reactions in the
500-reaction benchmark selection match more than one family, so this is not a corner case.
recommended.py is now read with ast.parse instead of ast.literal_eval, so each family set is
returned as a tuple holding its labels in the order they are written, rather than as a set
whose iteration order the hash seed decides. On top of that, get_all_families() sorts by
label within each of its tiers - the families the requested recommended set(s) contribute,
then the families that exist as an RMG database directory, then ARC's families - and
de-duplicates keeping the first occurrence. Sorting within a tier rather than relying on the
declaration order of recommended.py keeps ARC's answer independent of the textual layout of
an external database file, where a purely cosmetic reordering upstream would otherwise
silently reassign families. Sorting across the tiers instead would discard the tier contract
above. Both properties hold together: the family list is byte-identical at PYTHONHASHSEED 0,
1, 2, 3, 7 and 13, and no directory-only family precedes a recommended one.
An rmg_family_set list that mixes family set names with family labels reached the
de-duplication as a list nested inside a list, which is unhashable there; it is now
flattened. A family set name in such a list still matches no family, as before.
Gating. When several families match a reaction, the first entry of the family match list
decides which family ARC uses, so the family label order is the tie-break. That order is
lexical within each tier, which systematically favours the concerted families: RMG names the
pericyclic families early in ASCII and the radical addition families late. The matches are
now ordered and filtered before the lexical order is consulted. A match discovered in the
reverse direction is dropped whenever a forward match exists, since its atom label map
belongs to the flipped reaction. If the reactants carry unpaired electrons, only the matches
whose family recipe gains or loses a radical are kept, provided that leaves at least one and
drops at least one. What remains is ordered by the number of bonds the recipe forms or
breaks, then by the number of bonds it changes the order of, then by the previous label
order.
The direction gate needs no recipe, only the discovery direction, so it runs before the
recipes are read. A family whose groups.py cannot be parsed then costs only the radical gate
and the bond-change ordering, which do need the recipes, instead of silently disabling the
direction gate as well; that degradation is now reported as a warning rather than at debug
level. Reading the recipes after the gate also skips the recipes of the matches it dropped.
No ordering rule is chemically meaningful when several families genuinely match, so the
tie-break is made reproducible and visible rather than pretending to be principled; a
reaction that needs a specific family should pin it.
Tests cover both directions: that the shipped default leaves get_all_families() unchanged,
and that the configured set governs get_all_families() and check_family_name() when no set
is named at the call site. 2-methyl-1-butene <=> 1,1-dimethylcyclopropane is covered end to
end as a family reachable only through the database directories, and OH + HO2 <=> H2O2 + O
as a reaction that matches both a recommended family and a directory-only one, where the
recommended family wins.
Rejecting tautomers in check_product_isomorphism's InChI fallback.
check_product_isomorphism falls back to comparing the standard InChI and the
multiplicity when resonance-aware graph isomorphism fails. Standard InChI carries
a mobile-H layer that deliberately collapses tautomers onto one string, so the
fallback accepted a family-generated product that is a different molecule from
the one the user specified:
NNNN=NN (H2N-NH-NH-N=N-NH2) accepted as NNNNN=N (H2N-NH-NH-NH-N=NH)
NN=NN (H2N-N=N-NH2) accepted as NNN=N (H2N-NH-N=NH)
NC=O (formamide) accepted as N=CO (imidic acid)
all three pairs sharing a standard InChI. In benchmark reaction 95 ARC selected
that dict as product_dicts[0] and built a TS guess, an atom map and an NMD check
for a reaction that is not in the input.
The fallback's legitimate job is reconciling Lewis structures perceived from XYZ
against those perceived from SMILES, e.g. O=C=C(O)C=O and O=C[C-](O)C#[O+],
which no resonance structure of either makes graph-isomorphic to the other. Such
pairs differ only in bond orders and formal charges and place every hydrogen on
the same heavy atom, whereas tautomers move a hydrogen between heavy atoms. The
fallback therefore now also requires the two candidates to be isomorphic under
Molecule.is_isomorphic(strict=False), ARC's existing electron-agnostic
comparison: VF2 with strict=False compares only Atom.element and ignores bond
orders, charges, radical electrons and lone pairs, while explicit hydrogens
remain graph vertices, so hydrogen placement is still compared. save_order=True
keeps the call from perturbing the atom order of the caller's molecules.
Gating instead on the per-heavy-atom hydrogen-count multiset was rejected as
insufficient: H2N-NH-N=N-NH-NH2 and H2N-NH-NH-N=N-NH2 are different molecules
that share a standard InChI and carry the same hydrogen counts on every heavy
atom. The InChI FixedH layer was rejected because ARC's to_inchi has no options
passthrough, so it would mean new plumbing through the vendored molecule package
for a comparison that graph isomorphism already answers exactly.
Because that connectivity comparison is a necessary condition for a match, it is
made first and the InChI is generated only for a candidate/species pair it has
already admitted. Molecule.is_isomorphic itself checks the fingerprint and the
multiplicity before running VF2 and returns False on either mismatch, so it
subsumes the separate molecular-formula gate and the separate multiplicity
comparison the fallback used to make, and those are dropped rather than
duplicated. The InChI comparison itself is unchanged and still required.
This is what the ordering costs when it is the other way round. In benchmark run
re_run_dmat/kfir_rxn_13108,
O=[C]C#CC=O + c1cnon1 <=> O=CC#CC=O + N1=C[C]=NO1 (H_Abstraction)
the template generates [N-]1O[NH+]=C=C1, a C->N tautomer of the oxadiazole
reactant. It is valence-legal, so RMG builds it and to_smiles() succeeds, but its
cumulated C=C=N inside a five-membered ring is geometrically impossible and
neither RDKit nor OpenBabel will make an InChI for it. It shares a formula and a
multiplicity with c1cnon1, so the old formula gate let it through to to_inchi,
where Molecule.to_inchi's logger.exception and translator._write's logger.error
each fired before check_product_isomorphism swallowed the exception and correctly
dropped the candidate. ARC resolves the family four times per reaction, so a
healthy run logged eight ERROR blocks with full tracebacks, roughly 24 lines,
for a candidate no InChI was ever needed to reject. Per family resolution the
count goes from 30 InChI generations and 2 failures to 0 and 0, with n_dicts,
the resolved family, the products and both label maps unchanged.
The p_species InChIs are also no longer precomputed for the whole list behind one
try/except that returned False for the entire call. That made a single
InChI-hostile user-specified product reject every candidate and silently kill
family recognition for the reaction, which is the same defect as above pointed at
the user's input instead of the template's output. The lookup is now per pair, so
a molecule no backend can convert only removes itself from consideration, and
results are memoized per call, including failures, so no molecule is converted
twice. The old formula gate's spc.mol is None guard is dropped rather than
carried over: check_product_isomorphism dereferences spc.mol unconditionally when
it augments singlet biradicals, well before the fallback is reached, so on both
sides of this change a None mol raises AttributeError there and the guard in the
fallback was unreachable.
Neither comparison modifies the molecules it is given, which is what makes the
reordering a reordering of two independent conditions rather than a change in
what the function does to its arguments. is_isomorphic(save_order=True,
strict=False) was measured to leave atom order, charges, radical electrons, lone
pairs and bond orders untouched. to_inchi is the one that does mutate: it routes
through to_rdkit_mol(sanitize=True) with save_order defaulting to False, so
sort_atoms permutes the vertices of the molecule it is handed and never restores
them. On the accepted pair O=C[C-](O)C#[O+] / O=C=C(O)C=O it turned the caller's
product from O C C O C O H H into O O O C C C H H, and it did the same to the
user's spc.mol. _get_inchi therefore converts a deep copy, as arc/output.py
already does at its own to_inchi call site. Over 16 molecules, including the
anions to_inchi cannot convert at all, the copy yields a byte-identical result in
16 of 16 for +0.015 ms (0.179 to 0.194 ms median), and the atom order of both
products and p_species is left untouched for every input, matching or not. A
26-case sweep over matching, non-matching, tautomer, biradical, multiplicity and
InChI-hostile inputs gives identical verdicts, with the single intended exception
of the InChI-hostile user product described above, which goes from a wrong False
to True.
Searched for an existing helper before adding one: arc/output.py has a private
_safe(fn, default) with the same shape, but it is an output-layer private and
importing it into arc/family would invert the layering, and arc/species/converter
.py::pybel_to_inchi converts a pybel molecule rather than guarding ARC's own
to_inchi. No memoizing or failure-tolerant InChI accessor exists, so _get_inchi is
added beside its concept.
Seven regression tests: the tautomer pairs above and the equal-hydrogen-count
pair must be rejected (both verified failing before this change), the
Lewis-structure pair must still be accepted through the fallback, which the test
pins by asserting no resonance structure of either candidate is graph-isomorphic
to the other, the benchmark's [N-]1O[NH+]=C=C1 against c1cnon1 must be rejected
without to_inchi being called at all (verified failing before this change with
1 != 0), [CH3+] must not match [CH3-] and C=[CH+] must not match C=[CH-], pairs
the connectivity comparison and the multiplicity both admit so that the /q layer
of the standard InChI is the only thing separating them, _get_inchi must give
each molecule its own cache entry, and check_product_isomorphism must leave the
atom order of both its arguments untouched on a pair the fallback accepts. The
last three each fail against a mutant of this implementation that respectively
drops the InChI comparison, collapses the cache onto one key, or converts in
place; the nine tests the file already had survive all three mutants.
…inned family ARC re-determines each reaction's family by generating family product dicts and taking the first one. Setting `family` recorded the label but left `product_dicts` holding matches from every family that matched, in an order that nothing pinned. Everything downstream that pairs a family recipe with an atom label map - reactive bonds, atom mapping, TS guess construction - could therefore combine the pinned family's recipe with another family's labels. On [CH2]C(C=C)OS >> [O]C(C=C)CS with intra_OH_migration pinned and PYTHONHASHSEED=2, ARC reported C0-O4 as a breaking bond; there is no C0-O4 bond in the reactant. `product_dicts` is now restricted by the new restrict_product_dicts_to_family() to a single family - the pinned one, or the family of the first match when none is pinned - so a recipe is only ever paired with labels its own family generated. A pinned family that matches none of the generated dicts raises ReactionError: continuing would mean deriving the transition state from either a foreign family or from nothing at all, both silently. When no family is pinned and several match, a warning lists them all, so the tie-break is visible rather than implicit. Assigning `family` restricts dicts that were already generated rather than discarding them, so that the Linear TS adapter's wider-family-set retry, which sets `product_dicts` and then `family`, keeps the dicts that retry recovered. Assigning a family now also invalidates `family_own_reverse`, which the new get_family_own_reverse() derives from that family's `ownReverse` declaration on demand, instead of leaving it at False until the user restates it. get_product_dicts() and determine_family() take `rmg_family_set=None` to mean "not specified", so the configured setting is resolved in get_all_families() rather than being frozen into these signatures. determine_family()'s shortcut to the cached product_dicts property keys off rmg_family_set being None rather than equal to 'default', so an explicitly requested set is always honoured even when it matches the configured one. Tests cover that the configured family set governs get_product_dicts() and determine_family() when no set is named at the call site, that a pinned family restricts the product dicts and that a pinned family matching none of them raises, and that family_own_reverse follows the assigned family.
…ly matches Two failures around make_bond_changes() are guarded here. It raises KeyError when a family recipe references a label the atom label map lacks, which map_rxn() did not catch although it caught four other exception types around the same call. Restricting a reaction's product dicts to a single family narrows this without closing it, because a family can match with label maps that carry different label sets - intra_H_migration on [CH2]CCC yields both a four-label and a three-label map - so the call is now guarded as well. Its charge-separating branch subtracted two radical electrons from an atom that may hold one, and a lone pair from an atom that may hold none, which yields u-1/c-1 atoms and, once such an atom reaches RDKit, "OverflowError: can't convert negative value to unsigned int". That branch now declines the change rather than producing the invalid count. Note that this is a guard, not a reproduced fix: in all 40 donor/acceptor combinations tried here the following mol.update() raised AtomTypeError and the molecule was restored from its copy, so the invalid state never survived the function. The guard is therefore a no-op wherever the result is rejected, and only takes effect where it would not have been. The Linear TS adapter's wider family-set scan stops asking for reverse-discovered matches. A reverse-discovered r_label_map indexes the flipped reaction: measured on CH4 + OH, its BREAK_BOND pairs are absent from the reactant graph and present in the product graph. Every consumer reads it as reactant indices, including get_expected_changing_bonds() in this adapter and map_rxn(), which cuts the reactants with it. The adapter branches on discovered_in_reverse to pick a strategy but never translates the index space, so the flag requested matches it cannot use.
calvinp0
force-pushed
the
feature_configurable_rmg_family_set
branch
from
August 22, 2026 09:38
27f32a6 to
43c81e4
Compare
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.
Base:
main. This branch absorbs #979 (deterministic ordering + authoritative pinning) and #982 (chemical gates on the family choice) — both were flattened into this PR and are now closed. Review this PR alone; nothing else is outstanding.Three commits, no file touched by more than one of them. Together they make the RMG family set configurable, make family determination reproducible, and make the family that wins a chemical choice rather than an alphabetical one.
1. The family set is configurable, and
'all'reaches the whole databasesettings['rmg_family_set'](new, ships as'default') had no equivalent before:'default'was the signature default ofget_all_families()and of every entry point above it, so ARC could not be asked to consider anything else short of naming a set at every call site. That excludes families ARC's own TS adapters declare support for.get_all_families()— the single sink every path funnels through, which already owned an in-body fallback — not threaded through signatures. Threading leaves the sink's literal'default'in place, so every caller that does not forward the value keeps returning the curated set;check_family_name()onmainwas exactly such a bare call site. Measured on a deployed installation whose~/.arc/settings.pyasked for'all':get_all_families('all')→ 100 families, bareget_all_families()→ 55.get_product_dicts(),determine_family(),get_reaction_family_products()andcheck_family_name()propagateNoneto mean "not specified".determine_family()'s shortcut to the cachedproduct_dictsproperty keys offrmg_family_set is Nonerather than== 'default', so an explicitly requested set is honoured even when it matches the configured one.'all'previously unioned only the sets named in RMG'srecommended.py; families such asIntra_RH_Add_ExocyclicandIntra_RH_Add_Endocyclicappear in none of them despite shipping in the database.get_all_families()now also unions the families that exist as RMG database directories, via the newget_rmg_family_directories(), counting a directory as a family only when it holds agroups.py. The union is de-duplicated, which also removes the 24 duplicate labels the recommended sets alone produced (measured locally:'default'→ 54,'all'→ 99, of which 11 are directory-only).Ordering is a contract, not an accident. Directory-only families are positioned last, so one wins only where no recommended family matched — i.e. only where the alternative was
family=None. Every candidate must still reproduce the products the reaction asserts, so such a family cannot introduce a transformation, only propose a mechanism for one already stated. Stated in theget_all_families()docstring and pinned bytest_directory_only_families_are_ordered_lastand, end to end, bytest_widening_keeps_the_recommended_family_when_both_matchon[OH] + [O]O <=> OO + [O], whereH_Abstraction(recommended) andSubstitution_O(directory-only) each match in isolation.This redefines
'all'for a stock install, because the Linear TS adapter asks for'all'unconditionally whenever the configured set yields no product dicts. Measured on 1,4-cyclohexadiene ⇌ benzene + H₂ at the shipped'default':origin/maingets 0 wider-scan product dicts andfamily=None; this branch gets 16 andH2_Loss. Pinned bytest_wider_family_set_scan_used_by_the_linear_ts_adapter.get_reaction_family_products()'s docstring claimed'all'excludes surface families outright; two further statements repeated it.'all'skips family sets whose label contains'surface'and surface family directories, but a non-surface-labelled set can still list one, aselectrochemdoes forSurface_Proton_Electron_Reduction_*. The three statements now say what the code does, and a test pins it.2. Determinism and authoritative pinning
PYTHONHASHSEEDrecommended.py(ast.literal_eval) and fromos.listdir()— neither iteration order is specifiedfamilyrecorded the label but leftproduct_dictsholding matches from every family that matchedfamily_own_reversewas effectively alwaysFalse__init__set it toFalseand the property only recomputed onNone, so the recompute branch was deadMeasured:
[CH]1C=Cc2ccccc21 <=> [CH]1C=CC2C3=C1C=CC32reportsIntra_Diels_alder_monocyclicat seeds 0/1/13 andIntra_R_Add_Exocyclicat 2/5;[CH2]C(C=C)OS >> [O]C(C=C)CSflips betweenintra_OH_migrationandintra_substitutionS_isomerization. Withintra_OH_migrationpinned andPYTHONHASHSEED=2, ARC reportedC0-O4as a breaking bond — there is noC0-O4bond in the reactant. About 10% of the 500-reaction benchmark selection matches more than one family.recommended.pyis read withast.parse, so each family set keeps its labels in written order (get_rmg_recommended_family_sets()now returnsdict[str, tuple[str, ...]]). A set whose elements are not all plain string literals is sorted instead.get_all_families()sorts by label within each tier — recommended, then database directories, then ARC's — and de-duplicates keeping the first occurrence. Sorting within a tier rather than relying onrecommended.py's textual layout keeps ARC's answer independent of a cosmetic upstream reordering; sorting across tiers would discard the ordering contract above. Both properties hold together:get_all_families()is sha256-identical acrossPYTHONHASHSEED0/1/2/3/7/13, and no directory-only family precedes a recommended one. Mutation-checked — reversing the tiers, sorting across them, or de-duplicating through aseteach fails an ordering test; leaving a tier unsorted fails the determinism test.ARCReaction.restrict_product_dicts_to_family()restrictsproduct_dictsto a single family — the pinned one, or the family of the first entry when none is pinned — so a recipe is only ever combined with labels its own family generated. A pinned family matching none of the generated dicts raisesReactionErrorrather than silently deriving the TS from a foreign family or from nothing. When nothing is pinned and several match, a warning names them all.familyrestricts already-generated dicts instead of discarding them, so the Linear TS adapter's wider-family-set retry (which setsproduct_dictsthenfamily) keeps what the retry recovered.family_own_reverse, which the newget_family_own_reverse()derives on demand fromReactionFamily(label=family).own_reverse— using theReactionFamilyreaction.pyalready imports at module level, so no import line changes and no newpy/unsafe-cyclic-importalert is raised (those are per imported name).from_dictno longer forcesFalsewhen the key is absent. Verified:own_reversematches theownReversedeclaration read fromgroups.pyfor every family inget_all_families(), 0 mismatches.rmg_family_setlist mixing set names with family labels reached de-duplication as a nested list, which is unhashable there; it is now flattened.3. Which family wins: direction and radical gates
New
prioritize_family_product_dicts(), applied insideget_reaction_family_products()before the label-sort tie-break. Deterministic is not correct: the label order systematically favours the concerted families, because RMG names the pericyclic families early in ASCII and the radical-addition families late.r_label_mapaddresses the flipped reaction.formed/brokenalone leavesH_Abstraction(2, 0) tied with families differing only inchanged.Both gates only choose among families that already matched, so they cannot invent a mechanism. The direction gate is first because it is a validity filter, not a preference — and because it needs no recipe, only
discovered_in_reverse. That ordering matters operationally too: a family whosegroups.pycannot be parsed now costs only the radical gate and the bond-change ordering, instead of silently disabling the direction gate as well, and that degradation is reported atwarningrather thandebug. Reading recipes after the gate also skips the recipes of dropped matches.Evidence. The benchmark records which family each reaction was generated from, over 281 ambiguous reactions:
Of the 130 reactions whose family changes, 110 move toward the recorded family, 5 away. The 5 are
Ketoenol→intra_H_migrationand are correct:Ketoenol/groups.pycontains nou1atom anywhere in its group tree — a closed-shell tautomerisation template — while all 5 reactants are delocalised doublets with spin density on the H-acceptor carbon. 4 of the 5 produce byte-identical formed and broken bonds either way. The 5th, 2-hydroxyallyl radical ([CH2]C(=C)O), is symmetry-degenerate and the gate measurably wins:Ketoenol's atom map scrambles the four CH₂ hydrogens across both allyl termini, giving formed 5 / broken 5, which no single imaginary mode can satisfy, against 1 / 1 forintra_H_migration. The recorded labels are internally inconsistent for this pair anyway (5Ketoenol/ 3intra_H_migrationacross 8 ambiguities, 6 of which give identical bonds).Contract change worth flagging.
get_reaction_family_products()previously returned every match; it now returns a filtered and ordered list, and the docstring says so. Measured onC=C[CH]CCC + CC=CCCC >> C=CC(CCC)C(C)[CH]CCCunder'all': 4 product dicts before (1 forwardR_Addition_MultipleBond+ 3 reverseRetroene), 1 after.R_Addition_MultipleBond/Retroeneis the single largest ambiguous combination in the benchmark pool (118 of 281), so the direction gate is the highest-volume change here.Downstream:
Disproportionation → H_Abstractionmoves 4 reactions from no TS adapters at all to heuristics/autotst/crest;Intra_2+2_cycloaddition_Cd → Intra_R_Add_*gains kinbot. Genuinely ambiguous pairs are left alone (Intra_R_Add_Endo/Exocyclic,Cl_/H_Abstraction,H_Abstraction/Substitution_O). Known limitation, deliberate: the radical gate testsmultiplicity > 1, so a singlet biradical does not trigger it.Chemistry review on the gates: passed, ship as written, with two non-blocking follow-ups neither introduced here —
get_number_of_atoms_in_reaction_zonedrops 4→3 underintra_H_migration, andintra_H_migration's emptychangedlist will interact with the NMD family-recipe work when the two meet.4. Two guards the above made reachable
linear.pystops asking for reverse-discovered matches.interpolate_addition()'s wider family-set rescue passeddiscover_own_reverse_rxns_in_reverse=True, but the adapter branches ondiscovered_in_reversewithout ever translating the index space. Measured on CH₄ + OH: every reverse-discoveredBREAK_BONDpair is absent from the reactant graph and present in the product graph (they are the H₂O O–H bonds), while every consumer —get_expected_changing_bonds()here, andmap_rxn(), which cuts the reactants with the map — reads it as reactant indices.arc/mapping/contains no reference todiscovered_in_reverseat all. The kwarg is removed and the misleading docstring atinterpolate_addition()corrected._strategy_ring_scissionis unaffected: the gate drops reverse matches only when a forward match exists, and a reverse-only reaction keeps all of them.make_bond_changes()is guarded twice.KeyErroris added tomap_rxn()'s except clause around it (which already caught four other types) —r_label_dict[action[1]]raises it when a recipe names a label the map lacks. Restricting product dicts to one family narrows this without closing it, because one family can match with different label sets (intra_H_migrationon[CH2]CCCyields both a four-label and a three-label map). Separately, its charge-separating branch subtracted two radical electrons from an atom that may hold one and a lone pair from an atom that may hold none, yieldingu-1/c-1atoms and, once such an atom reaches RDKit,OverflowError: can't convert negative value to unsigned int; that branch now declines the change. This is a guard, not a reproduced fix — in all 40 donor/acceptor combinations tried, the followingmol.update()raisedAtomTypeErrorand the molecule was restored from its copy, so the invalid state never survived the function. The guard is a no-op wherever the result is rejected.Notes for callers, and scope
get_product_dicts()orget_all_families()is now sensitive to process-global settings state. That is the fix, but it is a footgun for tests and scripts. Every test added here either patches the setting or passesrmg_family_set=explicitly.get_all_families()at the shipped'default'is byte-identical tomain;test_rmg_family_set_setting_ships_as_defaultasserts the shipped constant. What is not unchanged is'all', per §1.linear.pywider-scan block restores_family/_product_dicts/_atom_mapin afinallyonly after the whole weights loop, whileTSGuess(family=rxn.family, ...)is written during it.Intra_RH_Add_Endocyclic/ExocyclicandIntra_R_Add_ExoTetCyclichave 0 training reactions;Substitution_Ohas 136 andSubstitutionS148), and a second hand-maintained list would drift fromts_adapters_by_rmg_family, which ARC already maintains. A hand-picked list would also have excludedlone_electron_pair_bond, a family ARC declares support for and ships a passing test for (linear_test.py::test_interpolate_lone_electron_pair_bond).Reuse check
Searched for an existing settings-resolution helper by behaviour ("argument-or-fall-back-to-
settings[key]"), by name acrossdef resolve*/get_setting*, and for the rawor settings[...]idiom at call sites — nothing of the kind exists, so the fallback stays inline inget_all_families(), where one already was. Nothing walked the RMG database'skinetics/families(get_all_familiesscannedARC_FAMILIES_PATHfor ARC's own families only), soget_rmg_family_directories()is new and sits besideget_rmg_recommended_family_sets().get_family_own_reverse()follows theReactionFamily(label=...)call form already used at two pre-existing sites inreaction.pyand two inarc/mapping/engine.pyrather than introducing a fifth;ReactionFamily.__new__interns per(label, consider_arc_families), so nothing is re-read from disk. For the direction gate,arc/mapping/was searched for any existing reverse-aware label-map translation — there is none, which is the finding that decided it; no new helper was added there.Checks
origin/main: the same 5 pre-existingarc/job/adapters/torch_ani_test.pyfailures on both, 0 added, 0 removed.origin/main:test_wider_family_set_scan_used_by_the_linear_ts_adapter,test_widening_keeps_the_recommended_family_when_both_match,test_bare_calls_honour_the_rmg_family_set_setting,test_determine_family_reaches_a_directory_only_family.TestFamilyChoiceGates, each mutation-proved — removing either gate or the count tie-break fails a distinct test.test_a_radical_cyclization_is_not_labeled_as_a_cycloadditionexercises the real path end to end on pentadienyl cyclisation, whereIntra_2+2_cycloaddition_Cdsorts ahead ofIntra_R_Add_Exocyclicand the gate still resolves to the radical addition.-n 6the branch additionally tripsscheduler_test.py::test_initialize_output_dict; that test is order-dependent and fails identically onorigin/mainin isolation — a pre-existing xdist worker-distribution artefact.Supersedes
fix_family_set(916d9943), which carries the same feature without the sink fix.🤖 Generated with Claude Code
Also folds in
fix_product_isomorphism_tautomerscheck_product_isomorphism's InChI fallback compared standard InChI plus multiplicity — and standard InChI carries a mobile-H layer that deliberately collapses tautomers onto one string. So ARC accepted a family-generated product that is a different molecule from the one the user specified (NC=Oaccepted asN=CO). Observed on benchmark reaction 95, where the bad dict becameproduct_dicts[0]and drove a TS guess, an atom map and an NMD check for a reaction that is not in the input.The fallback now additionally requires
Molecule.is_isomorphic(save_order=True, strict=False)— ARC's existing electron-agnostic comparison, which ignores bond orders, charges and radicals while keeping explicit hydrogens as graph vertices. Lewis-structure pairs likeO=C=C(O)C=O/O=C[C-](O)C#[O+]still match; tautomers do not.Because that check is a necessary condition it runs first, so InChIs are generated only for pairs it already admits — 30 generations and 2 failures per family resolution drop to 0 and 0 — and the new
_get_inchiconverts a deep copy and memoizes per pair, so one InChI-hostile molecule no longer rejects every candidate. Seven regression tests.Folded here rather than shipped separately because it touches the same two files as this PR's first commit; apart, whichever landed second would have conflicted.