Ring puckering conformer search (WIP) - #932
Open
alongd wants to merge 18 commits into
Open
Conversation
- _ring_z_displacements now validates the input shape (N x 3, N >= 3) and raises RingPuckerError when the fitted ring-plane normal is degenerate (e.g. collinear or otherwise ill-defined points), instead of silently producing NaNs. - Add validate_ring_order(), called from puckering_amplitude and cremer_pople_params, which checks that consecutive ring atoms (including the closure bond) are within a plausible bonding distance, catching scrambled connectivity order before it silently corrupts the Q/label. - Document the ring-connectivity-order precondition prominently in the module docstring and in the docstrings of the affected public functions.
- classify_pucker now returns 'planar' whenever the total puckering amplitude Q is below PLANARITY_AMPLITUDE_THRESHOLD (0.1 Angstrom), instead of assigning a spurious definite pucker label to a flat ring. - Tighten the 6-ring chair window to theta <= 30 or theta >= 150 (was <= 45 / >= 135) and add an explicit boat/twist-boat window of abs(theta - 90) <= 30; anything in between is now honestly labeled 'half-chair' instead of being lumped into 'chair'. - Replace the bankers-rounding phi bin selection (round(phi / width)) with an explicit half-open interval helper (_half_open_bin_index) plus a small numerical tolerance, so boundary-value phi angles bin deterministically.
- Replace test_rdkit_cyclohexane_classifies_as_chair, whose RDKit embedding seed (0) was cherry-picked to force a chair result, with a multi-seed test that only asserts each conformer gets an honest, valid pucker label and that at least one of several seeds settles into a chair with the expected amplitude. - Add a test built from a hand-crafted, hardcoded idealized chair cyclohexane geometry that does not go through ideal_pucker_geometry, to catch a shared sign/phase bug that could hide identically in both the geometry generator and the classifier.
Generate global-minimum-oriented ring conformers for monocyclic 5-/6-membered rings by morphing each ring to every canonical Cremer-Pople pucker state (both ring-flip poles), then optimizing under a two-stage force field: the ring atoms are frozen so the seed pucker survives, then the whole molecule is polished unconstrained. This recovers puckers (e.g. the chair global minimum) that an unconstrained embed would miss from an unfavorable base geometry. Fused, bridged, and spiro rings are gated out; non-converged geometries are dropped.
Add pucker_state_id (phase-resolved pucker id, order-dependent diagnostic) and pucker_label_counts (coarse label Counter) for scoring ring-pucker coverage of a conformer ensemble.
Score a ring-conformer ensemble against the global-minimum intent: the primary signal is global_min_hit / min_delta_e vs a reference minimum, matched with a symmetry-aware best-fit RMSD (RDKit GetBestRMS) so ring-flip and pseudorotation poles are not counted as distinct minima. ARC's own distance-matrix dedup count and Cremer-Pople label counts are reported as honestly-named secondary diagnostics (arc_dedup_*), since they are not symmetry-reduced. The metric is side-effect-free and reports an explicit status.
…r flow Ring-pucker seed geometries for saturated 5-/6-membered monocyclic rings are now fed into deduce_new_conformers() as additional base geometries, alongside the diastereomer bases, so each pucker state gets its exocyclic torsions enumerated by generate_conformer_combinations(). This recovers the global minimum for substituted rings, where the true minimum is a specific (ring-pucker x side-chain-rotamer) combination that a post-enumeration append from a single base would miss. The ring path is gated on ring membership (get_deterministic_sssr): acyclic molecules take an empty-list early return, leaving their behavior unchanged. Aromatic and in-ring-unsaturated rings are skipped; fused/bridged/spiro rings are hard-gated inside ring_pucker_seed_conformers(). Pucker bases receive a reduced combination threshold so the added base count cannot multiply the exhaustive-combination work into a performance cliff. Cross-products of pucker states across multiple independent rings are deferred to a later milestone. Adds ring_pucker_base_conformers() and ring_is_saturated(), plus unit and generate_conformers() integration tests (cyclohexane chair recovery and ethylcyclohexane pucker x rotamer).
…t handle
The Cremer-Pople ring-pucker seeder only handles isolated, saturated,
size-5/6 rings. For every other ring system (fused, bridged, spiro,
unsaturated/aromatic, or size not in {5,6}) add a gated ETKDGv3 embedding
pass as an additive safety net that never does worse than baseline.
mol_has_ring_unsupported_by_cp() is the exact complement of the CP-handled
set (reusing ring_is_saturated() and the SSSR shared-atom test), so the
backstop fires on precisely the rings CP skips. The acyclic path and the
supported-monocyclic path stay byte-identical: the gate returns False and
the default embed_rdkit() branch is unchanged.
The ETKDGv3 conformers are injected into deduce_new_conformers() as extra
ring-conformer bases (deduped and bounded via get_lowest_confs, capped at
ETKDG_MAX_BASES), NOT as members of the pool that torsion-well learning
reads -- so they cannot inflate the sampling-point product and flip the
enumeration to the bounded iterative path. Ring-pucker and ETKDG bases get
independent combination thresholds so neither can degrade the other's
enumeration.
Add TestAcyclicRegressionGuard asserting the ring-pucker and ETKDGv3 machinery is inert on acyclic molecules (heptane, 1-hexanol, iso-octane, glycerol): mol_has_ring_unsupported_by_cp() is False, ring_pucker_base_ conformers() returns nothing, generate_conformers() emits no ETKDGv3- or ring-pucker-sourced conformers across the full pool, and repeated runs are deterministic. Verified out-of-band that generate_conformers() output for these four acyclic species is byte-identical (FF energies + coordinates, same md5) between this branch and the base official/perf_hotspots, and that the acyclic-path wall-clock is unchanged (~1.0x); the CP pucker path adds ~5% on cyclohexane and the ETKDGv3 backstop ~2x on cis-decalin, both expected feature costs on the ring paths only.
For molecules with two or more saturated 5-/6-membered rings that are mutually independent (atom-disjoint across the SSSR), the ring-pucker base generator now enumerates the cross product of per-ring pucker states rather than puckering each ring only against the shared base geometry. Each combination composes per-ring out-of-plane displacements sequentially from the base coordinates and is polished with the union of all cross-producted rings frozen in the constrained stage, so the unconstrained polish cannot relax one ring's pucker back while another is being set. This closes the combined-pucker gap (e.g. reaching (chair, chair) from an unlucky (twist-boat, twist-boat) embedding) that the per-ring path could not. The single displacement implementation is factored into _ring_pucker_plan and _displace_ring_pucker, shared by the single-ring seeder (which stays byte-identical for monocyclic species) and the new cross product. The cross product is gated and bounded: it runs only when every eligible ring is independent and the combination count is within PUCKER_MAX_CROSS_COMBINATIONS; it seeds only the lowest-energy PUCKER_MAX_CROSS_BASE_CONFORMERS bases and caps its output at PUCKER_MAX_BASES. A base whose rings cannot all be planned falls back to per-ring seeding, so the cross product is never a coverage regression against the previous path. Monocyclic, fused/bridged, and acyclic species keep their existing behavior.
alongd
marked this pull request as ready for review
July 28, 2026 19:08
…yclic cap Addresses four defects from the PR #932 review: - ring_conformer_metric raised ValueError from min() when the energy window filtered every conformer out of get_lowest_confs; both get_lowest_confs and the metric now return an empty result instead of crashing on an all-filtered list. - The frozen-ring optimizer's stage-two acceptance test treated any non-continuing Minimize() code as success, so a force field that could not be set up (return code -1) was accepted with an unreliable energy. It now accepts only the converged code (0). - Symmetric boat seeds sit exactly on the boat / twist-boat saddle, where the force field reports zero-gradient convergence and shipped the transition state as a conformer, polluting the global-minimum metric. A small deterministic symmetry-breaking displacement is now applied before the unconstrained polish so a saddle relaxes into a real basin. - The per-ring pucker-base fallback was uncapped, and every monocyclic species takes it, so a flexible ring could emit dozens of bases. The fallback is factored into a shared helper that applies the same base-count and output caps as the cross-product path; get_lowest_confs keeps the lowest-energy bases so the global-minimum base survives.
…sion Addresses three more PR #932 review findings: - The backstop gate fired for every aromatic ring (planar, no pucker freedom), wasting an embedding pass, yet missed saturated 5/6 rings with an exocyclic sp2 atom (cyclohexanones, lactams, lactones) whose true minimum is a half-chair the Cremer-Pople seeder never reaches. The gate is redefined to fire only for rings with pucker freedom CP cannot seed: fused/bridged/spiro, non-aromatic rings outside size 5-6, and 5/6 rings with in-ring unsaturation or an exocyclic double/triple bond; fully aromatic rings and rings smaller than five are excluded. - The backstop ran once per resonance structure; it now runs once on the first molecule, since they share a single 3D skeleton. - The pucker/ETKDG combination threshold was divided by the base count, which silently switched those bases from exhaustive torsion enumeration onto the lossy iterative path and could drop the pucker-times-rotamer minimum. The division is removed; every base now receives the full threshold, with cost bounded by the base-count caps instead. Aromaticity is perceived via Molecule.get_aromatic_rings(save_order=True): the call sorts mol.atoms in place unless save_order is set, which would desync the atom order from the coordinate arrays built from it.
The single-ring pucker seeder sampled only 2 of 6 boat/twist-boat phases on a 6-ring equator and 2 of 10 for a 5-ring pseudorotation wheel (the sign=-1 "pole" was merely phi2+180, and ideal_pucker_geometry hardcoded one phi2 per label), so substituted-ring and 5-ring minima whose global-minimum pucker sits at an unsampled phase (e.g. furanose/proline north-south puckers) were missed. ring_pucker.py: - canonical_pucker_wheel(ring_size): enumerate the full discrete phase wheel -- 2 chair poles + 12 equatorial phase bins (6-ring), 20 pseudorotation bins (5-ring) -- with each label derived from its phase via pucker_label_from_phase (same half-open binning as classify_pucker), so label and phase cannot disagree. - ideal_pucker_geometry gains an optional phase_deg; for an equatorial label it validates the label matches the phase bin (raises otherwise), closing the label='boat',phase_deg=30 -> silently-a-twist-boat footgun. Chair ignores phase_deg and selects its pole via the sign of amplitude (back-compatible: phase_deg=None reproduces the prior single-phase behavior exactly). conformers.py: - The single-ring path (ring_pucker_seed_conformers) enumerates the full wheel; the equatorial sign loop is dropped (amplitude<0 == phi2+180, proven lossless). - The multi-independent-ring cross-product intentionally keeps the reduced canonical-state x pole set (a full wheel there would blow the 36-combination cap: 14**2 = 196); the independent branch now UNIONS the reduced simultaneous cross-product with the per-ring full-wheel seeds so independent rings get full per-ring phase coverage. - The pucker-base cap is now count-only (get_lowest_confs e=None): the default 5 kcal/mol window was a pre-torsion energy filter that could discard a distinct pucker phase whose torsion-packed child is the global minimum. Energy selection is deferred to the final post-torsion selection. - PUCKER_MAX_BASES raised 12 -> 20 so a single ring's full 5-ring wheel is not truncated below its natural size by the count cap. Tests: full-wheel enumeration and round-trip-through-classify (the C6/C7 proof), raw-seed phase-bin coverage for symmetric and substituted 5-rings, phase/label footgun guard, amplitude-sign == phase+180 equivalence, cross-product-stays- reduced + union regression, and structural (not brittle-energy) monocyclic golden.
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.
WIP / draft — do not merge. Opened early for backup + visibility while dev proceeds.
Adds ring-pucker sampling to ARC's conformer search (5-/6-membered monocycles + fused/bridged bicyclics), plus speed/accuracy quantification vs the perf_hotspots baseline.
Stacked on #927 (
perf_hotspots). Base auto-retargets tomainwhen #927 merges; if #927 is squash-merged this branch will need arebase --onto main.Scope was grilled with Alon (2026-07-28); full plan lives in the branch at
docs/conformer-search-plan.md(not committed to this PR).Goal 1 — ring puckering (this PR)
useSmallRingTorsions)Goal 2 — speed/accuracy (this PR)
Follow-up (separate PR, behind an explicit zeus-permission gate)