Skip to content

docs[installation]: Add CCD sync to test setup instructions - #101

Open
Emrys-Merlin wants to merge 1 commit into
RosettaCommons:stagingfrom
Emrys-Merlin:docs/installation_instructions
Open

Emrys-Merlin wants to merge 1 commit into
RosettaCommons:stagingfrom
Emrys-Merlin:docs/installation_instructions

Conversation

@Emrys-Merlin

Copy link
Copy Markdown

📋 PR Checklist

  • This PR is tagged as a draft if it is still under development and not ready for review.

    This avoids auto-triggering the slower tests in the CI and needlessly wasting resources.

  • I have ensured that all my commits follow angular commit message conventions.

    Format: <type>[optional scope]: <subject>
    Example: fix(af3): add missing crop transform to the af3 pipeline

    This affects semantic versioning as follows:

    • fix: patch version increment (0.0.1 → 0.0.2)
    • feat: minor version increment (0.0.1 → 0.1.0)
    • BREAKING CHANGE: major version increment (0.0.1 → 1.0.0)
    • All other types do not affect versioning

    The format ensures readable changelogs through auto-generation from commit messages.

  • I have run make format on the codebase before submitting the PR (this autoformats the code and lints it).

  • I have named the PR in angular PR message format as well (c.f. above), with a sensible tag line that summarizes all the changes in the PR.

    This is useful as the name of the PR is the default name of the commit that will be used if you merge with a squash & merge.
    Format: <type>[optional scope]: <subject>
    Example: fix(af3): add missing crop transform to the af3 pipeline


ℹ️ PR Description

Hi :)

I could not follow the dev setup instructions exactly, because I ran into two issues:

  1. aw setup tests failed if I did not setup the PDB_MIRROR_PATH variable beforehand.
  2. After running aw setup tests successfully, ca. half the tests still failed due to missing CCD codes. In fact, no tests/data/ccd directory was created contrary to my expectation. The artifact test_pack_latest.tar.gz did not contain any CCD data and the setup code does not seem to try to download the CCDs (in contrast to the missing PDBs).

I am not sure if the issue should be addressed on the documentation level or if aw setup tests should be adapted (or something else entirely). I thought updating the docs is very low effort. So, I just went ahead. If you prefer to address the issue another way or if I simply made a mistake during setup, I'm happy to close the PR.

What changes were made and why?

  1. I switched point 1 and 2 in the instructions, which instructs users to set up the ENV variables before running aw setup tests.
  2. I added aw ccd sync tests/data/ccd to the (new) second point to make sure that CCD data is available.

How were the changes tested?

I ran the test suite. Without the changes ca. half the tests failed. After these fixes, all but one1 test passed and the failing test seems unrelated to the downloaded data.

Additional Notes

Footnotes

  1. The failing test is a regression test, namely: tests/io/components/test_regression.py::test_regression_against_stored_result[1twr]. There was a mismatch in charges for 4 atoms.

I needed to adapt the dev setup instructions when I tried to run the tests locally:
1.) I needed to specify PDB_MIRROR_PATH before running aw setup tests
2.) The downloaded test_pack_latest.tar.gz, did not contain any CCD codes.

I fixed the first by setting up the ENV variable. For the second,
I ended up running `aw ccd sync test/data/ccd` in the root directory.
@Emrys-Merlin Emrys-Merlin changed the title docs(installation): Add CCD sync to test setup instructions docs[installation]: Add CCD sync to test setup instructions Sep 10, 2026
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