Add proteinbox workflow - #15
Open
MSiggel wants to merge 26 commits into
Open
Conversation
maxscheurer
requested changes
Jun 1, 2026
| - [76, 94] | ||
| protonation_states: | ||
| HIS15: HIE | ||
| box_padding: 12.0 |
Collaborator
There was a problem hiding this comment.
Consistent naming, as in other system types?
| - [64, 80] | ||
| - [76, 94] | ||
| protonation_states: | ||
| HIS15: HIE |
Collaborator
There was a problem hiding this comment.
Should be a list of key-value pairs, right?
Collaborator
Author
There was a problem hiding this comment.
Kept as a mapping deliberately: a residue can have exactly one protonation state, and a dict (HIS15: HIE) makes duplicate residues structurally impossible, whereas a list of pairs would allow conflicting entries. disulfide_bonds stays a list because a bond is a symmetric pair with no natural key/value — but I've aligned its residue nomenclature to match ([CYS6, CYS127]).
| type: pdb2gmx | ||
| forcefield: charmm36m | ||
| water_model: tip3p | ||
| ignh: true |
Collaborator
There was a problem hiding this comment.
Bad key name, have no idea what that means.
* Add SLURM cluster autodiscovery module with unit tests * Fix partition state: report 'up' if any node is schedulable * Query real SLURM default account via sacctmgr show user * Parse default_time from sinfo %L field separately from max_time * Use tuple for NodeType.features to enforce full immutability * Add _run_command edge case tests (timeout, missing binary, nonzero exit) * Fix PLW2901 lint errors and reformat test file * feat: wire cluster autodiscovery into SlurmConfig and CLI (#12) * Add SlurmConfig.from_cluster() classmethod for autodiscovery * Use autodiscovery fallback when --account not provided in SLURM commands * Add tests for SlurmConfig.from_cluster() autodiscovery * Add tests for mdfactory config cluster CLI command * fix new NodeType signature in tests * Slurm settings taking precendence * Convert dataclasses to Pydantic models for consistency * Refactor subprocess execution into generalized run_command utility * add BaseSlurmConfig/SlurmConfig/normalize_slurm_time to performance package * migrate submit.py: replace @DataClass SlurmConfig with re-export shim from performance * split slurm ini keys to PARTITION_CPU/PARTITION_GPU/DEFAULT_QOS, add settings properties * add test_slurm_config: BaseSlurmConfig 3-tier precedence, SlurmConfig model, from_yaml * update test_submit: Pydantic SlurmConfig, min_cpus partition selection, slurm_partition_cpu * fix from_cluster() return type to Self, path: Path|str on from_yaml, tighten docstring * fix cli.py: partition=None sentinel, forward min_cpus/min_mem_gb to from_cluster() * add no_slurm_settings fixture; apply to isolation-sensitive from_cluster tests * extend no_slurm_settings to positive-path autodiscovery tests * pre-commit fixes * add "error" to is_broken node states * replace lru_cache with sentinel; don't cache transient sinfo failures * use mutable list cell for cluster cache; fix ruff PLW0603 * guard int() in 3-part GRES branch against malformed entries * render typeless GPU as 'unknown' in config cluster display * add tests: memory trailing-plus, node-count tiebreak, cluster-None+no-partition path * add tests: analysis_run/artifacts_run SLURM autodiscovery glue * add develop branch to CI pull_request trigger
* chore: open PR for issue 16 (PR 1 — Parsl build orchestration)
* feat: add Parsl-based parallel build orchestration
- New mdfactory/orchestration/ module (config, apps, build)
- ExecutorConfig/SlurmExecutorConfig with SLURM-native field names
- build_systems() dispatches parallel builds via @python_app
- build_systems_dry_run() previews without loading Parsl
- Rich live progress display with activity log
- Ctrl+C triggers parsl.clear() to cancel SLURM jobs
- Extended CLI: mdfactory build accepts CSV/YAML with --config/--dry-run
- Added parsl to optional-dependencies and pixi dev env
- 19 tests covering config, build dispatch, and error handling
* fix: explicit scancel on shutdown, full error messages in TUI
- _shutdown_parsl() extracts SLURM job IDs from DFK and runs scancel
- Ctrl+C now reliably cancels SLURM jobs instead of relying on parsl.clear()
- Error messages no longer truncated in activity log
- Increased activity log to 12 entries
* feat: show SLURM job status (running/pending) in build TUI
* style: add icon to SLURM status line for consistent indent
* fix: force SOL molecule name for water SMIRNOFF parametrization
When species.resname is not 'SOL' (e.g. 'WAT'), Interchange would
generate atom types with the wrong prefix (WAT_0 instead of SOL_0),
causing a KeyError in OpenMM's GromacsTopFile parser.
Force molecule.name='SOL' before Interchange export so atom types are
always consistent (SOL_0, SOL_1, SOL_2). Also invalidate stale
SOL_params.itp when the ITP is regenerated.
* fix: address review findings — lazy parsl imports, DFK lifecycle, tests
- Move parsl imports inside functions (optional-dependency pattern)
- Wrap build_systems() in try/finally for DFK cleanup on exceptions
- Split _shutdown_parsl() into independent blocks, log at WARNING
- Normalize single-YAML output directory to output/{hash}/
- Add pytest.importorskip guard to orchestration test files
- Add tests: _build_system_impl, SLURM cleanup, KeyboardInterrupt,
dict input path, CLI integration (9 tests), parametrize regression
- Add PLC0415 to ruff ignore (lazy imports intentional throughout)
* fix: address re-review findings — input validation, dry-run guard, tests
* refactor: collapse dry-run into build_systems(), remove build_systems_dry_run
* feat: generate summary YAML when building from CSV (matches prepare-build output)
* feat: integrate PR #11 SLURM infrastructure — from_cluster(), TUI wizard, --slurm flag
* refactor: eliminate duplication — shared resolve_slurm_fields(), CLI helpers, DFK guard
* fix: address review findings — lazy questionary import, run_dir, BMP symbols
- Lazy-import questionary (in [parsl] extra) so importing mdfactory.orchestration
no longer fails for users without the extra (matches parsl/rich pattern)
- Add run_dir field to ExecutorConfig (default ~/.parsl/mdfactory) wired into
parsl.Config so runinfo/ no longer scatters into the working directory;
Path serializer keeps YAML round-trips working
- Clarify cpus_per_node docstring (Parsl cores_per_node != --cpus-per-task)
- Replace print() with Console().print() in the SLURM TUI
- Restrict source symbols to UTF-8 BMP (replace non-BMP emoji in build progress)
- Update TUI tests to patch _import_questionary
* feat: prepare Parsl foundation — reusable session context manager + forward-looking config
- Extract generic Parsl lifecycle into mdfactory/orchestration/session.py:
parsl_session(config) context manager owns the DFK guard, load, and
shutdown (+ scancel) so simulate_systems() / benchmark sweeps reuse it
via 'with parsl_session(config): submit(); wait()'. ParslSession.detach()
covers the wait=False ownership-transfer case. build.py re-exports the
moved helpers for backward compatibility
- Parametrize _wait_with_progress(label=...) so the progress display is
reusable for simulation and benchmark workflows
- Add available_accelerators to ExecutorConfig for GPU worker pinning,
wired into both local and SLURM HighThroughputExecutors
- Add launch_options on SlurmExecutorConfig -> SrunLauncher(overrides=...)
for srun-level task placement / NUMA binding; default launcher untouched
when unset
- Prompt for the constraint field in the SLURM wizard
- Tests: new test_orchestration_session.py plus config/TUI coverage
* refactor: SlurmExecutorConfig inherits BaseSlurmConfig — eliminate field/from_cluster duplication
- SlurmExecutorConfig now inherits (ExecutorConfig, BaseSlurmConfig) instead
of redeclaring account/partition/qos/constraint and reimplementing
from_cluster() with an identical body
- Set model_config = ConfigDict(frozen=False) to resolve the frozen mismatch
(BaseSlurmConfig is frozen; executor configs are mutated by the TUI wizard)
- Single source of truth for SLURM scheduling fields across the submitit
(SlurmConfig) and Parsl (SlurmExecutorConfig) backends — advances issue #20
and prevents a divergent third copy as submitit is phased out
- Add regression tests asserting the inheritance + mutability so the dedup
cannot silently regress
* fix: enrich build failure metadata and guard result completeness
- Add _describe_failure() returning (failure_type, error_detail); failed
build results now carry failure_type/error_detail alongside error, so
future retry logic can distinguish a GROMACS crash from infrastructure
failures. Uses the actual re-raised exception (modern Parsl) and only
falls back to legacy .e_value defensively — not the inaccurate AppFailure
path from the original review note
- Add _collect_results() that raises RuntimeError naming uncaptured hashes
instead of silently returning a shorter list than was submitted
(defensive guard against a future polling-loop bug)
- Tests: unit-cover both helpers (plain + legacy-wrapped exception,
complete + uncaptured-slot) and assert the enriched failure dict
Replace Nextflow pipeline with a native Python orchestration layer built on Parsl, enabling interactive TUI configuration, checkpoint-restart, and adaptive rescue retry — all without leaving the mdfactory CLI. Core architecture ----------------- - orchestration/ package: apps, stages, config, session, simulate, build, progress, environment, errors, rescue, mdp, tui - StageSpec frozen dataclass as the single source of truth for EM → NVT → NPT → Production file names, dependencies, and resource hints - SlurmExecutorConfig (Pydantic) with per-stage overrides → Parsl Config - EnvironmentConfig: structured module-load / pixi / conda / venv setup, auto-detected from the current shell or loaded from YAML Checkpoint and restart ---------------------- - Detect completed/partial stages on disk; skip or resume via -cpi - --restart flag resumes from the last successful checkpoint Rescue retry ------------ - classify_failure() distinguishes physics blowups from infrastructure errors using regex on stderr + GROMACS .log tail - Automatic MDP relaxation (dt halving, nsteps doubling) up to configurable max_rescue_attempts TUI and usability ----------------- - Rich-based interactive wizard (mdfactory simulate --guided) for executor, environment, and stage selection - Real-time Rich Live progress display with per-simulation stage tracking - --dry-run prints resolved gmx commands without submitting Graceful shutdown ----------------- - Single Ctrl+C cleanly exits: daemon worker threads + parsl.clear() + scancel via DFK job-ID tracking with squeue fallback - SIGINT isolated from Parsl interchange processes to suppress child tracebacks Test suite: 257 tests covering orchestration, CLI integration, rescue, progress, TUI, cluster performance, and SLURM config.
Collaborator
Author
|
@maxscheurer I pushed another ProteinBox hardening pass in dc1fa05:
Validation: all 60 ProteinBox tests pass; the 94-test focused prepare/settings/ProteinBox suite passes; touched files are Ruff-clean; and a real GROMACS regression with reversed chain order confirms requested disulfides are applied to the correct chain. The broader short suite reports 2537 passed and 4 skipped, with one unrelated existing macOS multiprocessing pickling failure in test_lock_folder_processes. Please take another look when convenient. |
…Config New models for the proteinbox simulation type: - ProteinSpecies: PDB-path-based species with disulfide/protonation annotations - ProteinBoxComposition: protein + box_padding + ionization config - Pdb2gmxConfig: force field and water model config for pdb2gmx - GromacsProteinParameterSet: output paths from pdb2gmx - BuildInput extended with proteinbox type and pdb2gmx parametrization Closes: relates to #13 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Functions for the proteinbox build pipeline: - check_gmx_available: verify gmx binary is on PATH - clean_pdb: PDBFixer-based PDB standardization - run_pdb2gmx: subprocess wrapper for gmx pdb2gmx - extract_charge_from_topology: parse net charge from .top/.itp - update_topology_molecules: append water/ion entries to [ molecules ] - validate_with_grompp: dry-run topology validation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Brief NPT equilibration with protein heavy atoms position-restrained via CustomExternalForce. Allows water/ions to relax around the fixed protein structure before GROMACS production runs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Build pipeline: clean PDB -> pdb2gmx -> center in cubic box -> solvate (reuses existing solvate()) -> ionize (reuses ionize_solvated_system()) -> update topology -> OpenMM relax with protein restraints -> validate. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
MDP files for protein-in-waterbox equilibration and production: - em.mdp: steepest descent minimization - nvt.mdp: NVT with -DPOSRES and Protein/Non-Protein tc-grps - npt.mdp: NPT with -DPOSRES and Berendsen barostat - md.mdp: production with Parrinello-Rahman, no position restraints All use CHARMM36m-specific nonbonded settings (1.2nm cutoffs, Force-switch VdW modifier, no dispersion correction). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- 13 unit tests covering ProteinSpecies, ProteinBoxComposition, Pdb2gmxConfig, BuildInput integration, and topology parsing - Fix ProteinSpecies to default count=1/fraction=1.0 at field level (avoids parent Species validator ordering issue) - Example YAML for lysozyme with CHARMM36m force field Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addresses 4 P2 review comments: - Add missing `from pathlib import Path` to build.py module scope - Resolve all paths to absolute in run_pdb2gmx before subprocess call (avoids cwd confusion with output_dir) - Add species/total_count/charge properties to ProteinBoxComposition and proteinbox case in BuildInput.metadata for analysis compatibility - Implement _apply_protonation_states: renames residues in PDB before pdb2gmx so protonation overrides (HIS->HIE, GLU->GLH) are applied Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add check_forcefield_available() that verifies the force field directory exists in GROMACS share, GMXLIB, or cwd before invoking pdb2gmx. Lists available force fields on failure. - Clean up mdout.mdp artifact in validate_with_grompp alongside check.tpr. - Fix bare open() file handles in relax_with_protein_restraints (use with-blocks). - Add CHARMM HIS alias translation (HIE→HSE, HID→HSD, HIP→HSP for charmm FFs). - Use re.fullmatch for protonation state key parsing (reject trailing chars). - Validate 3-character residue names for PDB column safety. - Add disulfide bond prompt generation (_build_disulfide_prompt_input) for deterministic pdb2gmx -ss interaction. - Resolve paths before working_directory context switch to avoid cwd breakage. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add FORCEFIELD_REGISTRY mapping friendly names (charmm36m, charmm36m-ljpme) to MacKerell lab download URLs and extracted directory names. - Auto-download missing force fields on first use from the registry. Stored in platformdirs user_data_dir (~/Library/Application Support/mdfactory/forcefields/). - resolve_forcefield() translates friendly names to actual directory stems so users write "charmm36m" in YAML and pdb2gmx receives the correct directory name. - Inject GMXLIB in subprocess env so gmx finds downloaded force fields. - Fix extract_charge_from_topology: skip .ff/ library includes (ions.itp, tip3p.itp) that contain atom type templates, not system charges. Was causing +159 charge for lysozyme with charmm36m instead of +8. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add [gromacs] section to settings.py with GMX_PATH and FORCEFIELD_DIR, following the same pattern as [cgenff] SILCSBIODIR. - Settings.__init__ auto-prepends FORCEFIELD_DIR to GMXLIB env var on startup, matching how SILCSBIODIR is auto-set for CGenFF. - check_gmx_available() checks configured GMX_PATH first, falls back to PATH lookup. - All gmx subprocess calls (pdb2gmx, grompp) use the configured binary and inject GMXLIB for force field resolution. - Add GROMACS setup to config wizard (sync_config.py): prompts for gmx path, forcefield dir, and offers to download CHARMM36m on setup. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- rename Pdb2gmxConfig.ignh to ignore_hydrogens - rename ProteinBoxComposition.box_padding to padding (matches LNP) - accept disulfide bonds as CYS-prefixed residue references (e.g. CYS6) to match the protonation_states residue nomenclature Addresses review comments on examples/proteinbox/lysozyme_charmm36m.yaml.
Address 7 issues flagged in review of the protein-box feature: - [P1] Resolve a relative protein pdb_path against the YAML file's directory in run_build_from_file, not the process cwd (the CLI cd's into the output dir before building), so bundled paths like ./1aki.pdb are found. - [P1] Declare pdbfixer in [tool.pixi.dependencies]; clean_pdb imports it at runtime but it was undeclared. - [P2] Run pdb2gmx with relative output filenames so the generated topology has a portable relative #include "posre.itp" instead of an absolute path. - [P2] Enforce simulation_type <-> parametrization <-> config-type consistency in BuildInput; proteinbox+cgenff and mismatched configs now fail validation with a clear message instead of late in the build. - [P2] Restrict Pdb2gmxConfig to CHARMM force fields and 3-site water (tip3p/spc/spce), matching what solvation and the mdp files support. - [P2] Let grompp validation failure fail the build instead of being swallowed as a warning. - [P2] Center the protein on its bounding-box center (not center of geometry) so every box face keeps the requested padding.
Rewrite the absolute force-field #includes that pdb2gmx writes into a relative path and copy the .ff directory next to the topology, mirroring the CGenFF strategy in generate_gromacs_topology. Both grompp (cwd) and OpenMM's GromacsTopFile (topology dir) then resolve the force field from the build directory, so it is portable to another machine. Reject CHARMM LJ-PME force-field variants (e.g. charmm36m-ljpme): the proteinbox run schedule uses cutoff/Force-switch LJ, which is incompatible with LJ-PME. Move the newly added test imports to module top level.
Multi-chain PDBs now build each subunit as its own Protein_chain_<ID> moleculetype, referenced from topol.top, rather than requiring a merge. The chains list must be declared in the YAML and is validated against the chains pdb2gmx produces; an undeclared multi-chain PDB is an error so subunits are never silently dropped. GromacsProteinParameterSet carries the per-chain include files (topol_Protein_chain_*.itp, posre*.itp), which build_proteinbox copies alongside topology.top in both the build and relaxation directories. Also fixes two review findings: - Protonation overrides match histidine tautomers as a family and raise when an override matches no residue; 4-character CHARMM acid states (GLUP/ASPP) are rejected with a clear message. - bundle_forcefield_into_topology copies each .ff directory only once instead of once per include line.
Protein charge is set from the pdb2gmx topology, and every caller treats species.charge as Optional[int] (guarding with 'is not None'). Raising NotImplementedError broke that contract and would propagate through the hasattr guard in _compute_charge_from_universe if a protein ever reached it. Returning None aligns with the contract and is skipped by the charge sums, leaving proteinbox charge results (from the topology) unchanged.
- Parse delimited protein list cells (chains 'A;B', disulfide pairs 'CYS6-CYS127' joined by ';') and dotted protonation_states columns from CSV rows; drop blank optional protein cells per row and exclude them from the strict NaN guard so heterogeneous proteins share one CSV. - Resolve relative protein pdb_path against the CSV directory at read time. - Reject merge_all combined with declared protein.chains at config validation instead of failing deep inside pdb2gmx. - Make relax_steps config-driven on ProteinBoxComposition. - Remove proteinbox build intermediates on success so the output dir holds only final artifacts, matching small-molecule builds.
Serialize prepared per-system YAML with model_dump(mode="json") so proteinbox Path fields (pdb_path) and tuple fields (disulfide_bonds) become plain str/list that yaml.safe_dump can write. Small-molecule inputs have no Path/tuple fields, so this path never surfaced before. Correct the build_proteinbox cleanup comment: only the three pdb2gmx-specific loose intermediates are removed; solvated.pdb and relaxation/ are left in place, matching how the bilayer build leaves solvated.pdb and bilayer_squeeze/. Add examples/proteinbox/proteins.csv showing the flat dotted-key / delimited-cell CSV encoding for single-chain (lysozyme) and multi-chain (insulin) systems. PDBs are referenced by RCSB id filename and fetched separately, matching the existing YAML example.
MSiggel
force-pushed
the
feature/protein-box
branch
from
September 4, 2026 12:36
dc1fa05 to
2450d80
Compare
Collaborator
Author
Rebased onto
|
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
3 participants
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.
Summary
Tests
Closes #13