fix dihedral_backbone() returning garbage angles across chain breaks - #933
Open
steps-re wants to merge 1 commit into
Open
fix dihedral_backbone() returning garbage angles across chain breaks#933steps-re wants to merge 1 commit into
steps-re wants to merge 1 commit into
Conversation
dihedral_backbone() and nucleotide_dihedral_backbone() built their phi/psi/omega (and alpha/beta/gamma/delta/epsilon/zeta) coordinate arrays purely from positional adjacency between residues, with no check that consecutive residues were actually bonded. When two residues were adjacent in the AtomArray but not chemically connected (missing loop, concatenated chains), the dihedrals spanning that gap came out as finite garbage instead of NaN. Add a bond-distance check (C-N for the peptide backbone, O3'-P for the phosphate backbone) using the same 1.2-1.8 A tolerance already used by check_backbone_continuity()/filter_linear_bond_continuity(), and NaN out the angles that span a junction where the distance falls outside that range. Applied to both dihedral_backbone() (fixes biotite-dev#744) and the identical defect in nucleotide_dihedral_backbone(), which was not filed but has the same root cause. Fixes biotite-dev#744. Co-Authored-By: Claude <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
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.
fixes #744
dihedral_backbone()builds its phi/psi/omega coordinate arrays purely by positional adjacency between residues in the AtomArray. it never checks whether two consecutive residues are actually bonded. so if a structure has a missing loop, or you concatenate two unrelated fragments into one AtomArray, the dihedrals spanning that gap come out as finite garbage instead of NaN, exactly like padix-key called out in the issue thread.went with the approach discussed in the thread: check the C-N distance between consecutive residues directly on the coordinates
dihedral_backbone()already extracted, rather than callingcheck_backbone_continuity()separately (which would mean re-filtering the whole array again). used the same 1.2-1.8 A tolerance thatcheck_backbone_continuity()/filter_linear_bond_continuity()already use elsewhere in the codebase, so the threshold is consistent across the library. where the C(i)-N(i+1) distance falls outside that range,psi[i],omega[i], andphi[i+1]get set to NaN since those are the three angles that straddle the junction between residue i and i+1.while testing this i found
nucleotide_dihedral_backbone()has the exact same bug for the O3'-P bond between residues, so I fixed that too with the same approach (epsilon[i],zeta[i],alpha[i+1]get NaN'd on a broken O3'(i)-P(i+1) junction). that one isn't filed anywhere, just the same root cause turning up twice.verification:
test_dihedral_backbone_chain_breakandtest_nucleotide_dihedral_backbone_chain_breaktotests/structure/test_geometry.py. both build a synthetic structure from two real fragments of 1l2y / 4p5j, renumber the second fragment's res_id to be perfectly continuous with the first (so the test can't accidentally pass just because of a res_id jump), then translate it 1000 A away so the two fragments are not bonded. confirmed both tests fail on current main (angles come back as finite numbers) and pass with this fix.tests/structure/suite (4176 passed, 13 skipped, no failures) to check for regressions, plus the existing multi-model, MDTraj-consistency, and barnaba-consistency dihedral tests specifically.disclosure: this fix (code + tests) was put together with AI assistance (Claude), then reviewed and verified by me before opening the PR.