Repository navigation
Conversation
make_crystal shelled out to ChimeraX (changechains/unitcell/combine) for the asymmetric-unit -> unit-cell expansion and to AmberTools PropPDB for the supercell. Neither is available in every container, and PropPDB numbers atoms continuously, which silently wraps past the 99999-atom / 9999-residue PDB fields that a 5x5x5 crystal easily exceeds. Add core/crystal.py, a pair of gemmi primitives alongside core/gmx.py: - expand_to_unit_cell() applies every symmetry operation of the CRYST1 space group and packs each copy by the integer lattice shift that brings its atom centroid into [0,1)^3 -- the rule ChimeraX's `unitcell` uses -- labels the copies A, B, C, ... and writes CRYST1 as P 1, so the separate spacegroup rewrite is gone. crystal.op_order reorders the operations: gemmi lists them in Hall order, which for P 21 21 21 swaps the third and fourth relative to the International Tables order ChimeraX used, i.e. the contents of chains C and D. - propagate_cell() replicates the P 1 cell in PropPDB's order (z fastest) and adds a per-cell numbering scheme that restarts the counters in every unit cell and tags the cell index in the segID field, so large supercells stay inside the fixed-column fields. crystal.numbering=auto picks it only when continuous numbering would not fit. Both rewrite the fixed-column ATOM/HETATM text rather than round-tripping a gemmi.Structure, so atom order, residue naming and record types survive byte-for-byte for the atom-order-dependent downstream topologies, and serials stay plain decimal because gmx solvate cannot read hybrid-36. The step API is unchanged; crystal.chimerax_exec is kept (unused) so existing configs and --chimerax-exec still validate. pdb4amber is now the only external tool make_crystal needs, and Dockerfile.base's ChimeraX install is no longer required by the pipeline (left in place). Validated against the 7FPV DHFR reference data: expand reproduces the ChimeraX prot_dry_cell.pdb for all 10472 atoms with a maximum coordinate deviation of 0.0000 A, same order and chains; propagate reproduces the PropPDB 2x2x2 xtal.pdb (83776 atoms, 5248 residues) to 0.0000 A with identical serial and resSeq fields. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
make_waterbox shelled out to AmberTools PropPDB to tile the solvated sub-cell back up nc_scale^3. That was the last PropPDB call left after core/crystal.py replaced the make_crystal one, and it has the same silent-wrap problem: the reservoir is one residue per water, so nc_scale=12 on the 7FPV cell already produces 10368 waters and overruns the 9999-residue PDB field. Use core.crystal.propagate_cell() instead, with a new waterbox.numbering knob defaulting to auto -- continuous while the totals fit, per-cell beyond that. gmx insert-molecules is now the only external tool this step needs. Two existing helpers needed no change: _restore_cryst1 already strips HEADER as well as CRYST1, which is what propagate_cell emits, and count_waters counts water oxygens rather than residue keys, so the per-cell counter restart cannot skew the water count written to the topology. The knob is config-only, with no CLI flag to match make-crystal's --numbering, because the water box is a regenerated intermediate rather than a file anyone inspects. Validated against the 7FPV reference data: tiling box_solv.pdb (18 atoms, 6 waters) 10x10x10 reproduces the PropPDB box_solv_expand.pdb exactly -- 18000 atoms with zero differing lines in columns 1-54, identical coordinate and atom-name sets, and count_waters 6000 on both. nc_scale 12 and 20 cross into per-cell numbering at 10368 and 48000 residues with no malformed serial or resSeq fields, and count_waters agrees with the reported residue count in every case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing in the pipeline launches ChimeraX any more: make_crystal expands the asymmetric unit with core/crystal.py (gemmi) since the unit-cell commit, so the licence-gated UCSF download in Dockerfile.base only added build time, image size and a fragile network step. Remove the ChimeraX RUN block (token fetch, .deb install and the ~/.config/ChimeraX directory; /home/dev/.config is still created by Dockerfile.actl) and the references to it in the Dockerfile.base header, the README stage table and the ExternalToolError docstring. crystal.chimerax_exec and --chimerax-exec are kept, still unused, because CrystalParams forbids unknown keys and existing configs set them. The build-images.yml comment that places base/gromacs in the proprietary Harbor project because they bake the non-redistributable ChimeraX is left for a separate decision on registry placement. Not build-tested: no container builder is available on the development machine. The ChimeraX .deb also pulled in Qt/X11/GL system libraries; nothing in the repo references them, but an image build plus a make-crystal / make-waterbox run is needed to confirm nothing depended on them implicitly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The only way to build the images on diffuse-sh-builder was the publishing chain, which pushes every stage to Harbor and runs only from `astera` (the `harbor` environment is restricted to that branch). That left no way to try Dockerfile changes -- such as dropping ChimeraX from the base image -- on the production build host before publishing them. Add a `publish` input to workflow_dispatch, off by default. With it off, the publishing jobs are skipped and a new `test-build` job builds base, gromacs and actl in one job on diffuse-sh-builder, chained through local tags as in the README. The publishing chain cannot be reused for this because each stage pulls its parent from Harbor by digest. `--builder default` pins the daemon's own BuildKit, which resolves locally built images in FROM, regardless of any buildx builder left selected on the runner. A GPU-free smoke test then checks that md-workflows runs, that gemmi and md_workflows.core.crystal import, that gmx and pdb4amber are on PATH and that ChimeraX is absent, and the test tags are removed. test-build pushes nothing and uses no Harbor credentials, so it has no `environment` and can be dispatched from any branch. That lets any branch's code run on the self-hosted runner, which previously only `astera` code reached in practice; write access was already the effective boundary, since a branch can edit this workflow, and the Harbor credentials stay behind the environment. Pushes to `astera` still publish. A manual dispatch now publishes only with publish=true; before, every dispatch published. Not run: actionlint and shellcheck report nothing new, but the job has not executed on the runner. It uses no registry cache, so the first run is a full build including the CUDA GROMACS compile. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
core/config.py imported tomllib unconditionally, but tomllib is only in the standard library from Python 3.11. pyproject.toml declares requires-python >= 3.10 and the image's lunus env pins python =3.10, so the installed md-workflows CLI died on import with "ModuleNotFoundError: No module named 'tomllib'" before doing anything. The actl build step only checks that md-workflows is on PATH, so image builds kept passing; the new test-build smoke test caught it. CI missed it because it tests on Python 3.12 only. Import tomllib on 3.11+ and tomli -- the package tomllib was adopted from, with the same API and TOMLDecodeError -- as tomllib on 3.10, and add tomli>=1.1 as a dependency for python_version < '3.11'. The unconditional import dates from 366f2d2 and is on main and astera as well, so images published from astera since then very likely have the same broken CLI. Checked on Python 3.10.21: md-workflows --help and all nine subcommands' --help run, all 23 modules import, a TOML config loads and malformed TOML is still rejected with the repo's ValueError, and pytest passes (53). Ruff is clean and pytest passes on 3.12. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CI ran ruff and pytest on Python 3.12 only, while the image's lunus env pins Python 3.10. That gap let an unconditional `import tomllib` (3.11+) pass CI and break the image's md-workflows CLI on import; it surfaced only in the build-images smoke test. Add a test-py310 job that installs the package with its dev extras on Python 3.10, runs pytest, and checks that `md-workflows --help` starts. It is a separate job rather than a python-version matrix on `lint`, so the existing "Lint Python code" check keeps its name for any branch-protection rule that requires it. Checked by running the job's steps in fresh Python 3.10 environments: on the tree before the tomli fix (d857afb) pytest errors on collection and the CLI fails with "No module named 'tomllib'"; on the current tree pytest passes (53) and the CLI starts. actionlint reports nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…f the repo base and gromacs went to a private project on Astera's internal registry only because they baked in ChimeraX, which could not be redistributed. With ChimeraX gone, nothing in those stages restricts redistribution, so publish them publicly to ghcr.io/prism-science/md-workflows with the same base-*/ gromacs-* tags, pushed with the workflow's own GITHUB_TOKEN (packages: write). The build cache for those stages moves to GHCR too. actl, the Astera ACTL overlay, still goes to library/md-workflows on the Astera registry, so the pods that pull it need no change. The registry host now comes from an ASTERA_REGISTRY secret in the harbor environment -- the name sampleworks uses for the same split -- instead of being written in this public repo, which the org's internal-reference scan flagged. GitHub masks the host in logs, including inside the image refs built from it, and the actl job fails early if the secret is unset. The example tag in Dockerfile.actl no longer names the host, and the README says where each stage is published and how to pull the gromacs image. base and gromacs keep environment: harbor although they no longer use its secrets: the environment's restriction to the astera branch is what limits publishing, now including the public images, to astera. actl now also needs base, so its digest report prints the base digest instead of an empty string (a pre-existing actionlint finding). Needed before this reaches astera: add the ASTERA_REGISTRY secret (bare host) to the harbor environment, and after the first publish make the GHCR package public if it was created private. Not run: the publishing jobs only run on astera. actionlint reports only the existing SC2129 style note in the version job. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The org secret scan reports 13 findings of its astera-internal-hostname rule, all in older commits on main, astera and dev: references to Astera's internal registry host in the build workflows, Dockerfiles and README. The repo is public, so they are already disclosed and git history keeps them; the previous commit removed the host from the working tree, which the scan states cannot clear history findings. List their fingerprints in .gitleaksignore with the reason, as the scan's guidance asks for accepted findings; the org workflow adds an in-repo .gitleaksignore to its ignore list. The file names commits, paths, lines and the rule id only, not the host. None of the findings come from this PR's commits: the scan's PR-range pass reported no leaks. Checked with gitleaks 8.30.1, the version the org workflow pins, and a stand-in rule with the same id: without the file the full-history scan reproduces the same 13 fingerprints as the CI log; with it, none remain. The file itself passes gitleaks' default credential rules. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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
1 participant
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.
Removes the ChimeraX dependency, allowing this repository to become public.
New public container location at ghcr.io/prism-science
There were preexisting gitleaks issues, added an ignore file here to prevent test failure.