Skip to content

feat: add cyclic peptide support to relative position encoding - #281

Merged
jandom merged 19 commits into
aqlaboratory:mainfrom
UnnaturalProducts:feature/cyclic-offset
Jul 8, 2026
Merged

feat: add cyclic peptide support to relative position encoding#281
jandom merged 19 commits into
aqlaboratory:mainfrom
UnnaturalProducts:feature/cyclic-offset

Conversation

@ioannisa92

@ioannisa92 ioannisa92 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds cyclic: bool = False field to the Chain inference query model so individual chains can be flagged as cyclic peptides
  • Propagates the cyclic flag through featurization: is_cyclic annotation on atom arrays → cyclic_mask boolean tensor in the feature dict
  • Implements cyclic_offset() in relpos.py to compute a topologically-aware pairwise offset matrix for cyclic chains (wraps distances past the midpoint back toward zero)
  • Integrates cyclic offset into relpos_complex(): for any chain where cyclic_mask is set, the standard linear offset is replaced with the cyclic offset for intra-chain pairs
  • Adds an example cyclic multimer inference input (query_multimer_cyclic.json) with MDM2 + a cyclic peptide binder (chain B)
image

@jandom

jandom commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

@ioannisa92 this is terrific, please let us know if you want to polish this up a bit (it's in draft probably for a reason) or do you want us to jump on this?

@ioannisa92

ioannisa92 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

@jandom i will make it a PR today probably. I just want the tests to run with the cyclic offset.

Who should i request as reviewer?

Additionally, when i did this analysis in the other repo #526 the effect of the cyclic offset was greater. Is there an explanation why OF3 has a different behavior?

@ioannisa92
ioannisa92 marked this pull request as ready for review June 29, 2026 22:42

@jnwei jnwei left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great work @ioannisa92 !

I verified that I can run the provided multimer_cyclic example using the OpenFold3 preview 2 weights.

I have a few nits regarding code style / test example organization. But overall this is really good.

Comment thread openfold3/core/utils/relpos.py
Comment thread openfold3/tests/core/utils/test_relpos.py
Comment thread openfold3/core/utils/relpos.py
Comment thread openfold3/tests/test_relpos.py Outdated
@jandom

jandom commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Who should i request as reviewer?

@ioannisa92 good stuff, you should just tag me and Jennifer for now – we can pull more folks if needed

Additionally, when i did this analysis in the other repo aqlaboratory/openfold#526 the effect of the cyclic offset was greater. Is there an explanation why OF3 has a different behavior?

Ha, there could be any number of explanations :D

@ioannisa92

ioannisa92 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

@jnwei @jandom Apart from moving the test is there anything else blocking merge? This feels like something that is beyond the scope of this PR

@jandom
jandom self-requested a review July 2, 2026 06:41
@jandom

jandom commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Ah, sorry – I tagged myself in for review, I need a bit of time to go through this. By the end of the week, so today tomorrow.

@jandom jandom added the safe-to-test Internal only label used to indicate PRs that are ready for automated CI testing. label Jul 2, 2026
@jandom

jandom commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Okay, immediate comment is we need to make linting and tests pass (easy) and i want to see thumbs up from @jnwei (so just re-request review from her)

@jandom
jandom requested a review from jnwei July 2, 2026 06:45

@jnwei jnwei left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good!

Just one last thing from me - It looks like the linter is catching some errors. Could you use the dev environment and do a ruff check on the codebase? The contributor's guide has some quick instructions for how to install and use ruff

Comment thread openfold3/core/utils/relpos.py
Comment thread openfold3/tests/core/utils/test_relpos.py
@ioannisa92

Copy link
Copy Markdown
Contributor Author

@jandom I ran ruff and all checks pass

@jandom jandom added safe-to-test Internal only label used to indicate PRs that are ready for automated CI testing. and removed safe-to-test Internal only label used to indicate PRs that are ready for automated CI testing. labels Jul 2, 2026
@jandom

jandom commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

@ioannisa92 thanks – i've kicked off tests to see if everything passes

@jandom

jandom commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

@ioannisa92 I think we've got some test failures, if you have a moment to fix those and then I think this is good to go

@ioannisa92

Copy link
Copy Markdown
Contributor Author

@jandom i had to redo the test data under /openfold-3/openfold3/tests/test_data/structure_from_query per your README. The tests pass now locally

@ioannisa92

ioannisa92 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

@jandom The tests that still fail test_triangular_attention.py and test_triangular_multiplicative_update.py dont seem to have anything to do with the cyclic offset. I have used --force-regen on those and they still fail locally.

@jandom

jandom commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

@ioannisa92 brilliant stuff, will re-run now

@jandom jandom added safe-to-test Internal only label used to indicate PRs that are ready for automated CI testing. and removed safe-to-test Internal only label used to indicate PRs that are ready for automated CI testing. labels Jul 7, 2026
@jandom

jandom commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

@ioannisa92 one last thing we can't merge ATM because of conflicts, we also can't resolve the conflicts ourselves (because we can't push to your forked repo). Would you have a moment to look into it?

@jandom

jandom commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

A bunch of test failures

FAILED openfold3/tests/core/data/primitives/structure/test_query.py::test_consistent_structure_from_smiles_and_ccd_code[c1ccc(c(c1)CCCl)NC(=O)Nc2cc(ccc2n3ccnc3)C(F)(F)F-A1ASQ] - AttributeError: 'list' object has no attribute 'array_length'

All these will go away once you update this branch @ioannisa92 (ie merge main into it)

@ioannisa92

Copy link
Copy Markdown
Contributor Author

@jandom ok merged and tests now pass

@jandom

jandom commented Jul 7, 2026 via email

Copy link
Copy Markdown
Collaborator

@jnwei jnwei added safe-to-test Internal only label used to indicate PRs that are ready for automated CI testing. and removed safe-to-test Internal only label used to indicate PRs that are ready for automated CI testing. labels Jul 8, 2026
@jandom
jandom merged commit 85afbf0 into aqlaboratory:main Jul 8, 2026
12 checks passed
@jandom

jandom commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

I'm getting errors running training after the merge, will revert and then we can fix and merge again

  File "/home/jandom/workspace/openfold-3/openfold3/core/data/pipelines/featurization/structure.py", line 239, in featurize_target_gt_structure_of3
    features_target = featurize_structure_of3(
        atom_array=atom_array,
        n_tokens=n_tokens,
        is_gt=False,
    )
  File "/home/jandom/workspace/openfold-3/openfold3/core/data/pipelines/featurization/structure.py", line 195, in featurize_structure_of3
    atom_array.is_cyclic[token_starts], dtype=torch.bool
    ^^^^^^^^^^^^^^^^^^^^
  File "/home/jandom/workspace/openfold-3/.pixi/envs/openfold3-cuda12/lib/python3.14/site-packages/biotite/structure/atoms.py", line 346, in __getattr__
    raise AttributeError(
        f"'{type(self).__name__}' object has no attribute '{attr}'"
    )
AttributeError: 'AtomArray' object has no attribute 'is_cyclic'

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

Labels

safe-to-test Internal only label used to indicate PRs that are ready for automated CI testing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants