Reach RDKit parity across every scalar descriptor - #14
Open
nkwork9999 wants to merge 1 commit into
Open
Conversation
Cross-checking duckSMILES against RDKit 2026.03.5 over 214 molecules turned up defects in nearly every layer of the library. This fixes all of them and adds the parity suite that pins the behaviour down. Aromaticity - Replace the 6-ring single/double counting with a Hueckel 4n+2 electron count over each smallest ring, then over each fused ring system. Every ring is judged against a snapshot of the original bond orders, so a fused neighbour that was rewritten first can no longer change the answer -- Kekule naphthalene used to report one aromatic ring. - Fill implicit hydrogens before perceiving aromaticity, otherwise a Kekule pyrrole nitrogen loses its hydrogen to the aromatic valence. Canonical SMILES - Add canonical_ranks: refinement of graph-only atom invariants with deterministic tie-breaking. The writer now orders neighbours by rank, so re-spelling a molecule cannot change the output. 16 of 45 test molecules used to canonicalise differently depending on atom order. - Record the written neighbour order per atom and recompute tetrahedral parity on output, so @/@@ follows the molecule and not the spelling. - Carry cis/trans marks on bonds and normalise them into a canonical frame anchored on the lowest-ranked substituent. - Emit ring-opening digits at the atom itself (c1ccccc1), and bracket an atom whenever the bare form would imply the wrong hydrogen count. Ring perception - Keep the relevant cycles -- those not expressible as a GF(2) sum of strictly shorter cycles -- instead of a deduplicated per-bond scan. This matches RDKit's symmetrised SSSR: bicyclo[2.2.2]octane has three rings, adamantane four. Descriptor definitions, matched to RDKit's own SMARTS - H-bond acceptors: aromatic nitrogen only counts with two connections and no hydrogen; acid hydroxyls and amide nitrogens are excluded. - H-bond donors: O/S need exactly one hydrogen and no charge. - Rotatable bonds: full Strict pattern, including the trivial-rotor and conjugated-linkage exclusions. - Crippen: test H3 before H2 so an aromatic N-H scores +0.2142. - Murcko: retain exocyclic double-bonded atoms, replace severed bonds with hydrogens, and drop stereo only when the neighbour set changed. - Formula gains its charge suffix; Li/Be/He gain monoisotopic masses. SMARTS engine - R<n> is ring-membership count, not "is in a ring". - Support dot-separated multi-component patterns, which brings QED's last two structural alerts into play. FFI - ds_canonical_smiles returned -1 for a small buffer, ds_mol_formula and ds_add_hydrogens silently truncated. All three follow the sizing protocol now, and the fixed-size C++ buffers retry on the heap. Morgan - Share the common ring perception, break dedup ties by canonical rank, and avalanche the invariant before folding. Fingerprints are now spelling-invariant and pairwise Tanimoto tracks RDKit at rho > 0.95. Tests - scripts/gen_rdkit_fixtures.py regenerates the fixtures; 214 molecules and 1188 equivalent spellings under crates/smiles/tests/. - 47 integration tests over descriptor parity, the canonical-form contract, Kekule/aromatic equivalence, scaffold rules, fingerprints and buffer contracts. Canonical and scaffold output is compared structurally rather than byte-wise, since two canonicalisers may legitimately choose different spellings of the same molecule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Cross-checked duckSMILES against RDKit 2026.03.5 over 214 molecules and 1188 equivalent spellings. Every scalar descriptor now agrees, and the suite that pins it down ships with the change.
What was wrong
R<n>treated as "in a ring"; dot-separated patterns unsupported, disabling two QED alerts.ds_canonical_smilesreturned -1 on a small buffer;ds_mol_formulaandds_add_hydrogenstruncated silently.What changed
canonical_ranksrefines graph-only atom invariants with deterministic tie-breaking; the writer orders neighbours by rank, recomputes tetrahedral parity against the emitted order, and normalises cis/trans marks into a canonical frame.Tests
scripts/gen_rdkit_fixtures.pyregenerates every fixture. 47 integration tests cover descriptor parity, the canonical-form contract (order invariance, idempotence, round-trip, distinctness, stereo), Kekulé/aromatic equivalence, scaffold rules, fingerprints and buffer contracts.Canonical and scaffold outputs are compared structurally rather than byte-wise: two canonicalisers may legitimately choose different spellings of the same molecule, so RDKit's spelling is fed back through
ds_canonical_smilesand required to land on ours.546 unit tests plus 47 integration tests pass, with zero compiler warnings.
🤖 Generated with Claude Code