Skip to content

Commit 2666615

Browse files
refactor: address whole-repo code review findings
- split _rmsd_matrix.py: extract numba kernel to _rmsd_matrix_numba.py and dedupe 3 copies of QCP Newton-Raphson into a backend-agnostic _rmsd_qcp_block(xp, ...) shared by torch / jax / cupy backends - replace assert-as-guard in pinned-D2H pipeline with a _PinnedChunk NamedTuple so prev state is atomic and survives python -O - promote _average_rmsf_with_sem to public average_rmsf_with_sem and re-export from mdpp.analysis (was a cross-module private import) - annotate GPU helpers with ModuleType + explicit return types - harden mdrun{,_mpi_plumed}.sbatch with set -euo pipefail - fix dtn_download.sh -e quoting bug; validate remote subdirectory names; make LOGIN_HOST / DTN_HOST / OPENFE_CONTAINER env-overridable - replace Unicode A-ring / superscript-2 in plot labels with LaTeX equivalents (rendered output unchanged) - silence 18 RDKit-stub mypy errors with targeted type-ignore comments - replace __import__('multiprocessing') with a normal top-level import - remove dead torch_mod parameter from _rmsd_torch_row_chunk 543 CPU + 69 GPU tests pass; mypy + ruff + shellcheck green.
1 parent d55c4ca commit 2666615

16 files changed

Lines changed: 430 additions & 495 deletions

File tree

scripts/gromacs/data_transfer/dtn_download.sh

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,14 @@ EOF
1717
exit 1
1818
}
1919

20-
LOGIN_HOST="sherlock-plain"
21-
DTN_HOST="sherlock-dtn"
20+
# Override LOGIN_HOST / DTN_HOST in the environment for non-Sherlock deployments.
21+
# Defaults match the Sherlock cluster SSH aliases used by this group.
22+
LOGIN_HOST="${LOGIN_HOST:-sherlock-plain}"
23+
DTN_HOST="${DTN_HOST:-sherlock-dtn}"
2224
JOBS=0
2325
DRY_RUN=""
2426

25-
SSH_OPTS="ssh -T -c aes128-gcm@openssh.com -o Compression=no -o ServerAliveInterval=60"
27+
SSH_OPTS=(ssh -T -c aes128-gcm@openssh.com -o Compression=no -o ServerAliveInterval=60)
2628

2729
while [[ $# -gt 0 ]]; do
2830
case "$1" in
@@ -71,6 +73,16 @@ mapfile -t SUBDIRS < <(
7173
exit 1
7274
}
7375

76+
# Validate remote-supplied subdirectory names: a compromised or malformed remote
77+
# could return entries with shell metacharacters or leading dashes that would be
78+
# misread as rsync flags by GNU parallel's command construction.
79+
for subdir in "${SUBDIRS[@]}"; do
80+
if [[ ! "$subdir" =~ ^[A-Za-z0-9_.][A-Za-z0-9_./-]*$ ]]; then
81+
echo "Error: refusing unsafe subdirectory name from remote: $subdir" >&2
82+
exit 1
83+
fi
84+
done
85+
7486
# Default to one job per subdirectory if -j was not specified.
7587
[[ "$JOBS" -eq 0 ]] && JOBS=${#SUBDIRS[@]}
7688

@@ -84,8 +96,8 @@ RSYNC_OPTS=(-ahP --append-verify)
8496

8597
parallel -j "$JOBS" --bar --joblog "$LOGDIR/joblog.tsv" \
8698
rsync "${RSYNC_OPTS[@]}" \
87-
-e "'$SSH_OPTS'" \
88-
--log-file="'$LOGDIR/{}.log'" \
99+
-e "${SSH_OPTS[*]}" \
100+
--log-file="$LOGDIR/{}.log" \
89101
"$DTN_HOST:$REMOTE_DIR/{}/" \
90102
"$LOCAL_DIR/{}/" \
91103
::: "${SUBDIRS[@]}"

scripts/gromacs/mdrun/mdrun.sbatch

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
#SBATCH --constraint="GPU_SKU:A100_PCIE|GPU_SKU:A100_SXM4|GPU_SKU:A40|GPU_SKU:H100_SXM5|GPU_SKU:H200_SXM5|GPU_SKU:L40S|GPU_SKU:V100_PCIE|GPU_SKU:V100S_PCIE|GPU_SKU:V100_SXM2"
1414
#SBATCH --time=2-00:00:00
1515

16+
set -euo pipefail
17+
1618
ml gcc/12.4.0
1719
ml cmake/3.31.4
1820
ml make/4.4

scripts/gromacs/mdrun/mdrun_mpi_plumed.sbatch

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
#SBATCH --mem-per-gpu=8G
1414
#SBATCH --time=2-00:00:00
1515

16+
set -euo pipefail
17+
1618
ml gcc/12.4.0
1719
ml cmake/3.31.4
1820
ml make/4.4

scripts/openfe/quickrun/quickrun.sbatch

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919

2020
set -euo pipefail
2121

22-
OPENFE_CONTAINER=/scratch/groups/ayting/containers/openfe/openfe_latest-apptainer.sif
22+
# Override OPENFE_CONTAINER (apptainer .sif) via the environment for non-Sherlock
23+
# or non-ayting-group deployments; the default below is the Sherlock ayting path.
24+
OPENFE_CONTAINER="${OPENFE_CONTAINER:-/scratch/groups/ayting/containers/openfe/openfe_latest-apptainer.sif}"
2325

2426
usage() {
2527
cat >&2 <<EOF

src/mdpp/analysis/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
from mdpp.analysis.fes import compute_fes_2d, compute_fes_from_projection
2828
from mdpp.analysis.hbond import compute_hbonds, format_hbond_triplets
2929
from mdpp.analysis.metrics import (
30+
average_rmsf_with_sem,
3031
compute_dccm,
3132
compute_delta_rmsf,
3233
compute_radius_of_gyration,
@@ -43,6 +44,7 @@
4344
"KMeans",
4445
"MiniBatchKMeans",
4546
"RegularSpace",
47+
"average_rmsf_with_sem",
4648
"compute_contact_frequency",
4749
"compute_contacts",
4850
"compute_dccm",

0 commit comments

Comments
 (0)