Skip to content

Prevent torsions and couplings across filtered residues - #63

Open
choijein0410 wants to merge 1 commit into
srnas:masterfrom
choijein0410:fix/skipped-residue-torsions
Open

choijein0410 wants to merge 1 commit into
srnas:masterfrom
choijein0410:fix/skipped-residue-torsions

Conversation

@choijein0410

Copy link
Copy Markdown

When Nucleic skips an unknown residue or one missing the base atoms required for recognition, its ok_residues list can place nonadjacent topology residues next to each other. The same-chain checks then construct alpha/epsilon/zeta and epsilon-based J-coupling quartets across the skipped residue, returning finite values for an invalid connection.

Require adjacent original topology residue indices as well as the existing chain check in the four inter-residue paths. Invalid quartets retain the existing missing-value sentinel, so the public angle and J-coupling APIs return NaN. PDB residue numbers and insertion codes do not determine adjacency.

The regression tests use the existing UUCG.pdb fixture and cover unknown names, missing base atoms, reordered residue selections, terminal/chain boundaries, nonconsecutive residue numbers, and PDB insertion codes. They also compare all unaffected values and existing NaNs.

I additionally checked two public 20-model GCAA NMR bundles (source data). Their unrecognized URI11 caused G10's epsilon quartet to use G12:P. The patch marks G10 epsilon/zeta, G12 alpha, and the two affected G10 J-couplings as missing; all other outputs remain exactly unchanged. Chemistry-checked URI-to-U copies and a canonical-U MD control preserve their original outputs.

Validation:

  • Eight new regressions passed; the unmodified upstream class fails the five gap-related regressions and passes the three normal controls.
  • Full suite: 37 passed, 4 failed. The same four tests fail on unmodified 48ac4a9 in the same environment: test_dsmotif, test_escore, test_rmsd_1, and test_ssmotif. These are the existing np.float removal and generated-PDB reference-comparison failures.
  • Eight additional real-structure/control cases passed with Python 3.12.14, MDTraj 1.10.3, NumPy 1.26.4.
  • CI's Flake8 error selection (E9,F63,F7,F82) reports zero errors.

Related to #46, but this change specifically handles residues filtered out of an existing topology. It does not infer missing connectivity when a residue is absent from the topology itself, so it does not close that issue.

@choijein0410

Copy link
Copy Markdown
Author

Thanks for rerunning CI. I reproduced the reported failures on the unmodified upstream base (48ac4a9) and compared it with this PR (af4d60a). All 33 existing tests have identical outcomes and failure messages in each environment; the patch adds no failures to the existing suite.

Reproduction run, logs, and artifacts · Comparison script and pinned packages

The run used the same Ubuntu 24.04 x86_64 runner image (20261004.327.1), exact CPython versions, and runtime/test package versions recorded in CI attempt 2. For each Python version, upstream and PR source trees ran in separate directories on the same runner with unchanged dependencies, using PYTHONPATH="$PWD" pytest -v --cov=barnaba plus JUnit output.

Python Unmodified upstream This PR
3.8.18 28 passed, 5 failed 36 passed, 5 failed
3.9.25 30 passed, 3 failed 38 passed, 3 failed
3.10.22 30 passed, 3 failed 38 passed, 3 failed

The failures also match the original CI run:

  • 3.8: MDTraj raises KeyError: '8384' while reading the PDB fixture in the annotation, double-strand motif, e-score, and single-strand motif tests; test_rmsd_1 fails its generated-PDB reference comparison.
  • 3.9/3.10: the double-strand motif and RMSD PDB reference comparisons fail, and e-score uses the removed np.float alias.
  • 3.7: checking out unmodified upstream reproduces the same setup error: Python 3.7 x64 is unavailable for Ubuntu 24.04, before any tests execute.

As a separate control, I ran the eight new regression tests against upstream code: 5 failed and 3 passed in all three runnable environments. With the patch, all 8 passed in each environment.

The artifacts contain full pytest logs, JUnit reports, source hashes, package lists, and machine-readable comparisons. The reproduction workflow's green status means the baseline failures matched; the full Barnaba suite still has the failures listed above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant