fix(evals): cif patching regression to allow protenix outputs to run on all evals, and remove hydrogens in RSCC parsing by k-chrispens · Pull Request #336 · diff-use/sampleworks

k-chrispens · 2026-07-22T03:21:12Z

There are a few regressions that needed to be made here to get the evals running completely for the paper. These changes are archived in the zenodo record but should be applied here for consistency until we fix the Protenix JSON parsing issue.

Summary by CodeRabbit

Bug Fixes
- Improved handling of missing atoms during structure analysis.
- Improved reference structure processing and residue/chain mapping for downloaded CIF files.
- Added safer handling for alternate atom locations in reference structures.
Chores
- Expanded synchronized checkout exclusions to include TAR archives, generated output, and repository metadata directories.

coderabbitai · 2026-07-22T03:21:20Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 35207ace-d77e-4377-94c4-a1e6c44f7a56

📥 Commits

Reviewing files that changed from the base of the PR and between 078ea63 and c1a8a23.

📒 Files selected for processing (3)

.actlignore
scripts/eval/rscc_grid_search_script.py
scripts/patch_output_cif_files.py

📝 Walkthrough

Walkthrough

The PR updates CIF reference normalization and chain/residue remapping, disables missing-atom insertion in RSCC parsing, and expands synchronized checkout exclusions.

Changes

CIF patching

Layer / File(s)	Summary
Safe reference resolution and atom remapping `scripts/patch_output_cif_files.py`	Resolves mixed ATOM/HETATM alternate locations before loading references, then strictly remaps both atom chain IDs and residue IDs.

RSCC parsing

Layer / File(s)	Summary
Disable missing atom insertion `scripts/eval/rscc_grid_search_script.py`	Parses reference and refined structures with `add_missing_atoms=False`.

Synchronization exclusions

Layer / File(s)	Summary
Expand synchronization ignore patterns `.actlignore`	Adds exclusions for `*.tar`, `output/`, and `.jj/`.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

diff-use/sampleworks#104: Related CIF residue and chain remapping changes.
diff-use/sampleworks#195: Introduces the mixed ATOM/HETATM alternate-location resolver used here.
diff-use/sampleworks#237: Related chain-mismatch handling in CIF patching.

Suggested reviewers: dorismai, marcuscollins, mag-astera

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is specific and accurately reflects the main CIF patching fix and RSCC parsing change.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

Fix failing CI checks

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch kmc/cif-eval-fixes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

Copilot

Pull request overview

This PR addresses evaluation regressions related to mmCIF altloc/compositional heterogeneity handling, aiming to make reference structures stack cleanly and keep downstream patching/eval scripts consistent with current guidance behavior.

Changes:

Add reference-structure canonicalization for mixed-altloc modified residues (e.g., CYS/CSO) so map_altlocs_to_stack() can succeed when building evaluation reference ensembles.
Add regression tests covering mixed-altloc canonicalization behavior (including a real 6NI6 example).
Update CIF patching + RSCC grid-search scripts to mirror the same CIF/parse handling assumptions used elsewhere.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/eval/test_structure_utils.py	Adds end-to-end + unit regression tests for mixed-altloc residue canonicalization/stacking.
src/sampleworks/eval/structure_utils.py	Implements `_closest_canonical_amino_acid()` and `canonicalize_mixed_altloc_residues()`, and applies it when loading reference stacks.
scripts/patch_output_cif_files.py	Uses `resolve_mixed_hetatm_atom_altlocs()` and adjusts residue/chain remapping logic for current eval needs.
scripts/eval/rscc_grid_search_script.py	Sets `add_missing_atoms=False` in `parse()` calls for consistency with other pipelines.
.actlignore	Ignores additional local artifacts (`*.tar`, `output/`, `.jj/`).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

marcuscollins · 2026-07-22T16:37:21Z

+        # Mirror the fix from guidance_script_utils - strip mixed ATOM/HETATM altlocs
+        # at the same residue position (e.g. 6NI5/6 CYS/CSO) so the reference matches
+        # what is generated by guidance. Returns the original path if nothing to fix.
+        safe_reference_path = resolve_mixed_hetatm_atom_altlocs(reference_path)
+        reference = load_any(safe_reference_path)
+        if safe_reference_path != reference_path:
+            safe_reference_path.unlink()
+
        asym_unit = load_any(cif_file)
        asym_unit = ensure_atom_array_stack(asym_unit)


Will address separately as there are multiple places this comes up. #339

marcuscollins

I have questions (see comments in rscc_grid_search_script.py and patch_output_cif_files.py) but the main thing is that I think this needs to be rebased as it seems to include changes that have to do with canoncalizing amino acids that I think are in a different PR.

marcuscollins · 2026-07-22T16:37:21Z

+        # Mirror the fix from guidance_script_utils - strip mixed ATOM/HETATM altlocs
+        # at the same residue position (e.g. 6NI5/6 CYS/CSO) so the reference matches
+        # what is generated by guidance. Returns the original path if nothing to fix.
+        safe_reference_path = resolve_mixed_hetatm_atom_altlocs(reference_path)
+        reference = load_any(safe_reference_path)
+        if safe_reference_path != reference_path:
+            safe_reference_path.unlink()
+
        asym_unit = load_any(cif_file)
        asym_unit = ensure_atom_array_stack(asym_unit)


Will address separately as there are multiple places this comes up. #339

marcuscollins · 2026-07-22T16:53:55Z

+    # which breaks the commented multi-chain handling below

-        mapping[cif_key] = ref_key[1]
+    # patch the residue numbers to match the original pdb


@k-chrispens did you apply this to all the generated CIF files? If so, I'm fine with it. We're probably going to be able to get rid of this script soon anyway.

Yes, I did!

marcuscollins · 2026-07-22T16:54:51Z

    return atom_array


+def _closest_canonical_amino_acid(res_name: str) -> str | None:


Isn't this in your other PR? Please rebase after merging that one.

marcuscollins · 2026-07-22T16:56:32Z

            )
        # parse() returns only the first altloc.
-        ref_structure = parse(ref_path, ccd_mirror_path=None)
+        ref_structure = parse(ref_path, ccd_mirror_path=None, add_missing_atoms=False)


If you really want to remove hydrogens, add hydrogen_policy="remove"

Strip mixed ATOM/HETATM altlocs and remap chain+residue per atom in patch_output_cif_files.py; pass add_missing_atoms=False in the RSCC grid search parse; extend .actlignore.

Copilot AI review requested due to automatic review settings July 22, 2026 03:21

k-chrispens requested review from a team, Abdelsalam-Abbas, DorisMai, denis-zaitsev, mag-astera and marcuscollins as code owners July 22, 2026 03:21

k-chrispens had a problem deploying to gpu-testing July 22, 2026 03:21 — with GitHub Actions Error

Copilot started reviewing on behalf of k-chrispens July 22, 2026 03:21 View session

k-chrispens changed the title ~~CIF patching fixes~~ fix: cif patching regression to allow protenix outputs to run on all evals, and remove hydrogens in RSCC parsing Jul 22, 2026

k-chrispens force-pushed the kmc/cif-eval-fixes branch from 843e977 to de3d301 Compare July 22, 2026 03:24

k-chrispens had a problem deploying to gpu-testing July 22, 2026 03:24 — with GitHub Actions Error

Copilot AI reviewed Jul 22, 2026

View reviewed changes

k-chrispens changed the title ~~fix: cif patching regression to allow protenix outputs to run on all evals, and remove hydrogens in RSCC parsing~~ fix(evals): cif patching regression to allow protenix outputs to run on all evals, and remove hydrogens in RSCC parsing Jul 22, 2026

marcuscollins requested changes Jul 22, 2026

View reviewed changes

k-chrispens force-pushed the kmc/cif-eval-fixes branch from de3d301 to ac9e3f2 Compare July 23, 2026 02:31

fix(eval): harden CIF output patching and RSCC parsing

c1a8a23

Strip mixed ATOM/HETATM altlocs and remap chain+residue per atom in patch_output_cif_files.py; pass add_missing_atoms=False in the RSCC grid search parse; extend .actlignore.

k-chrispens force-pushed the kmc/cif-eval-fixes branch from ac9e3f2 to c1a8a23 Compare July 29, 2026 21:00

Navigation Menu

Uh oh!

fix(evals): cif patching regression to allow protenix outputs to run on all evals, and remove hydrogens in RSCC parsing - #336

fix(evals): cif patching regression to allow protenix outputs to run on all evals, and remove hydrogens in RSCC parsing#336
k-chrispens wants to merge 1 commit into
mainfrom
kmc/cif-eval-fixes

k-chrispens commented Jul 22, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 22, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

Copilot AI left a comment

Uh oh!

marcuscollins Jul 22, 2026

Uh oh!

marcuscollins left a comment

Uh oh!

marcuscollins Jul 22, 2026

Uh oh!

marcuscollins Jul 22, 2026

Uh oh!

k-chrispens Jul 29, 2026

Uh oh!

marcuscollins Jul 22, 2026

Uh oh!

marcuscollins Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		return atom_array


		def _closest_canonical_amino_acid(res_name: str) -> str \| None:

Uh oh!

Conversation

k-chrispens commented Jul 22, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

marcuscollins Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

marcuscollins left a comment

Choose a reason for hiding this comment

Uh oh!

marcuscollins Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

marcuscollins Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

k-chrispens Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

marcuscollins Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

marcuscollins Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

k-chrispens commented Jul 22, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 22, 2026 •

edited

Loading