From b2e6ec7d36d0bf30a33a31289c979b3c6ccbe444 Mon Sep 17 00:00:00 2001 From: zfried Date: Fri, 10 Apr 2026 10:05:21 -0400 Subject: [PATCH 1/2] fix filter_constrained_rotors to exclude axis atom from movability check Bond-axis atoms (atom_i and atom_j) sit on the rotation axis and never physically move during SetDihedralDeg, so they should not disqualify a rotor. This allows boundary-attachment bonds at the edge of a pinned scaffold to be kept when only the distal fragment is free. --- openconf/perceive.py | 31 ++++++++++++------- tests/test_constrained.py | 64 ++++++++++++++++++++++++++++++--------- 2 files changed, 70 insertions(+), 25 deletions(-) diff --git a/openconf/perceive.py b/openconf/perceive.py index 37559e1..44c24d9 100644 --- a/openconf/perceive.py +++ b/openconf/perceive.py @@ -6,6 +6,7 @@ from rdkit import Chem + @dataclass class Rotor: """Represents a rotatable bond. @@ -353,13 +354,19 @@ def _atoms_on_side(mol: Chem.Mol, start_atom: int, excluded_atom: int) -> frozen def filter_constrained_rotors(rotor_model: "RotorModel", constrained_atoms: frozenset[int]) -> "RotorModel": - """Return a new RotorModel containing only rotors that move no constrained atoms. + """Return a new RotorModel containing only rotors whose moving fragment is constraint-free. + + A rotor around bond (i, j) is kept when the *distal* atoms on one side — + those beyond the bond-axis atom that actually translate during + ``SetDihedralDeg`` — contain no constrained atoms. The bond-axis atoms + (atom_i and atom_j) lie on the rotation axis and never physically move, so + they are excluded from the movability check. This allows boundary-attachment + bonds at the edge of a pinned scaffold to be kept even when both bond atoms + are in the constrained set, provided the substituent beyond the axis atom is + entirely free. - A rotor around bond (i, j) is included if the fragment on one side of the - bond contains no constrained atoms (meaning we can rotate that fragment - freely). When the free fragment happens to be on the i-side rather than the - j-side as stored in dihedral_atoms, the rotor is flipped so the moving - fragment is always the free one. + When the free distal fragment is on the i-side rather than the j-side, the + rotor is flipped so the moving fragment is always the free one. Ring flips are kept only when the entire ring is free of constrained atoms. @@ -376,14 +383,16 @@ def filter_constrained_rotors(rotor_model: "RotorModel", constrained_atoms: froz for rotor in rotor_model.rotors: atom_i, atom_j = rotor.atom_idxs moving_j = _atoms_on_side(mol, atom_j, atom_i) + distal_j = moving_j - {atom_j} - if not constrained_atoms & moving_j: - # j-side is free — use rotor as-is (SetDihedralDeg moves j-side) + if not constrained_atoms & distal_j: + # j-side distal atoms are free — use rotor as-is free_rotors.append(rotor) else: moving_i = _atoms_on_side(mol, atom_i, atom_j) - if not constrained_atoms & moving_i: - # i-side is free — flip so the free side is the moving side + distal_i = moving_i - {atom_i} + if not constrained_atoms & distal_i: + # i-side distal atoms are free — flip so the free side is the moving side a, i, j, b = rotor.dihedral_atoms free_rotors.append( Rotor( @@ -393,7 +402,7 @@ def filter_constrained_rotors(rotor_model: "RotorModel", constrained_atoms: froz rotor_type=rotor.rotor_type, ) ) - # else: constrained atoms on both sides — exclude this rotor + # else: constrained atoms in distal fragments on both sides — exclude this rotor # Ring flips: keep only rings with no constrained atoms free_ring_flips = [rf for rf in rotor_model.ring_flips if not constrained_atoms & frozenset(rf.ring_atoms)] diff --git a/tests/test_constrained.py b/tests/test_constrained.py index f4e7b1f..773d5c8 100644 --- a/tests/test_constrained.py +++ b/tests/test_constrained.py @@ -166,25 +166,58 @@ def test_config_and_preset_both_raises(): def test_filter_constrained_rotors_eliminates_double_sided(): - """A rotor whose both fragments contain constrained atoms is excluded. + """A rotor whose distal fragments on both sides contain constrained heavy atoms is excluded. - In butane (CCCC) with heavy atoms {1, 2} constrained, the central C1–C2 - bond has constrained atoms on both sides and must be eliminated. - The terminal bonds C0–C1 and C2–C3 each have one free side and are kept. + Biphenyl with all 12 carbons constrained: the single biaryl C–C bond has + constrained ring carbons beyond the axis atom on both sides, so it cannot + be rotated without displacing a constrained atom and must be eliminated. """ from openconf import build_rotor_model, filter_constrained_rotors, prepare_molecule - mol = prepare_molecule(Chem.MolFromSmiles("CCCC")) + mol = prepare_molecule(Chem.MolFromSmiles("c1ccccc1-c1ccccc1")) rm = build_rotor_model(mol) - full_count = rm.n_rotatable # 3 bonds: C0-C1, C1-C2, C2-C3 + full_count = rm.n_rotatable # 1 bond: the biaryl C–C - # Constrain the two middle carbons; the central bond has constrained atoms - # on both sides and should be removed. - constrained = frozenset([1, 2]) - filtered = filter_constrained_rotors(rm, constrained) + # Constrain every carbon; the biaryl bond has constrained distal heavy + # atoms on both sides and should be removed. + all_carbons = frozenset(i for i, a in enumerate(mol.GetAtoms()) if a.GetAtomicNum() == 6) + filtered = filter_constrained_rotors(rm, all_carbons) assert filtered.n_rotatable < full_count - assert filtered.n_rotatable > 0 # terminal bonds should remain + assert filtered.n_rotatable == 0 + + +def test_filter_constrained_rotors_boundary_attachment_kept(): + """Scaffold-edge bond is kept when both axis atoms are constrained but distal side is free. + + Butylbenzene with the benzene ring AND the adjacent chain carbon (C3, index 3) + all constrained: the C3–ring bond has two constrained axis atoms, but the free + butyl chain (C0–C2) lies entirely beyond C3. The boundary-attachment rule keeps + the bond so the chain can still be explored. + """ + from openconf import build_rotor_model, filter_constrained_rotors, prepare_molecule + from openconf.perceive import _atoms_on_side + + mol = prepare_molecule(Chem.MolFromSmiles("CCCCc1ccccc1")) + rm = build_rotor_model(mol) + + # Ring atoms (4–9) plus the first chain carbon attached to the ring (3). + # Both atoms of the ring–chain bond are now constrained. + constrained = frozenset([3, 4, 5, 6, 7, 8, 9]) + filtered = filter_constrained_rotors(rm, constrained) + + # The C3–ring bond must survive: free chain atoms are the distal fragment. + assert filtered.n_rotatable > 0 + + # Every surviving rotor's distal moving fragment must be constraint-free. + for rotor in filtered.rotors: + atom_i, atom_j = rotor.atom_idxs + moving = _atoms_on_side(mol, atom_j, atom_i) + distal = moving - {atom_j} + assert not constrained & distal, ( + f"Rotor {rotor.atom_idxs} has constrained atoms in distal fragment: " + f"{constrained & distal}" + ) def test_filter_constrained_rotors_free_side_reoriented(): @@ -205,12 +238,15 @@ def test_filter_constrained_rotors_free_side_reoriented(): # All 4 chain rotors should be preserved (just possibly reoriented). assert filtered.n_rotatable == rm.n_rotatable - # For every remaining rotor the moving fragment must be entirely free. + # For every remaining rotor the distal moving fragment must be entirely free. + # atom_j sits on the rotation axis and never physically translates, so it is + # excluded from the check — only the atoms beyond it (distal) must be free. for rotor in filtered.rotors: atom_i, atom_j = rotor.atom_idxs moving = _atoms_on_side(mol, atom_j, atom_i) - assert not constrained & moving, ( - f"Rotor {rotor.atom_idxs} has constrained atoms in moving fragment: {constrained & moving}" + distal = moving - {atom_j} + assert not constrained & distal, ( + f"Rotor {rotor.atom_idxs} has constrained atoms in distal fragment: {constrained & distal}" ) From cb6f975d78c0f225966e996729a961074f8dcd27 Mon Sep 17 00:00:00 2001 From: zfried Date: Fri, 10 Apr 2026 10:11:14 -0400 Subject: [PATCH 2/2] fix EN dash lint errors in comments --- tests/test_constrained.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_constrained.py b/tests/test_constrained.py index 773d5c8..52ed713 100644 --- a/tests/test_constrained.py +++ b/tests/test_constrained.py @@ -176,7 +176,7 @@ def test_filter_constrained_rotors_eliminates_double_sided(): mol = prepare_molecule(Chem.MolFromSmiles("c1ccccc1-c1ccccc1")) rm = build_rotor_model(mol) - full_count = rm.n_rotatable # 1 bond: the biaryl C–C + full_count = rm.n_rotatable # 1 bond: the biaryl C-C # Constrain every carbon; the biaryl bond has constrained distal heavy # atoms on both sides and should be removed. @@ -191,7 +191,7 @@ def test_filter_constrained_rotors_boundary_attachment_kept(): """Scaffold-edge bond is kept when both axis atoms are constrained but distal side is free. Butylbenzene with the benzene ring AND the adjacent chain carbon (C3, index 3) - all constrained: the C3–ring bond has two constrained axis atoms, but the free + all constrained: the C3-ring bond has two constrained axis atoms, but the free butyl chain (C0–C2) lies entirely beyond C3. The boundary-attachment rule keeps the bond so the chain can still be explored. """ @@ -201,12 +201,12 @@ def test_filter_constrained_rotors_boundary_attachment_kept(): mol = prepare_molecule(Chem.MolFromSmiles("CCCCc1ccccc1")) rm = build_rotor_model(mol) - # Ring atoms (4–9) plus the first chain carbon attached to the ring (3). - # Both atoms of the ring–chain bond are now constrained. + # Ring atoms (4-9) plus the first chain carbon attached to the ring (3). + # Both atoms of the ring-chain bond are now constrained. constrained = frozenset([3, 4, 5, 6, 7, 8, 9]) filtered = filter_constrained_rotors(rm, constrained) - # The C3–ring bond must survive: free chain atoms are the distal fragment. + # The C3-ring bond must survive: free chain atoms are the distal fragment. assert filtered.n_rotatable > 0 # Every surviving rotor's distal moving fragment must be constraint-free.