Skip to content

Commit ff6e7e4

Browse files
committed
fep-report + run scripts: git_commit provenance always reaches a log file
Root cause: the shell runner scripts (run_freesolv_m5max.sh, run_binding_streptavidin_gpu.sh, run_binding_egfr_gpu.sh) echoed the header block — including `git commit: <HEAD>` — to stdout only. `tee` picked up the python env block afterwards, not the header. Result: `cellsim fep-report` parsed run.log and found no git commit, so every tarball from the field came back with git_commit=None even though the runner DID know the commit hash at launch. Fix is two-sided so both in-flight and future runs are covered: 1. src/fep/report.py: _parse_run_log now falls back to env.log and doctor.log when run.log is missing the `git commit:` line, and also falls back when run.log itself is absent. Run.log wins when both carry the line (most local). 2. Runner scripts: wrap the header `echo` block in `{...} | tee "${OUT_DIR}/env.log"` so the commit lands in env.log from launch onward. The python env block below now appends (`tee -a`) rather than overwriting. Applied to all three runner scripts. 3. 7/7 regression tests covering: run.log present, env.log fallback, no-run.log-at-all, doctor.log fallback, empty case, run.log-wins precedence, direct helper. Wired into smoke.yml. For the M5 Max run in-flight (on an older checkout): git_commit will still be None in its tarball, but the fix means it's recoverable if the runner supplies the commit hash out-of-band — and every future run is covered without operator intervention.
1 parent d9177f6 commit ff6e7e4

6 files changed

Lines changed: 239 additions & 35 deletions

File tree

.github/workflows/smoke.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,9 @@ jobs:
172172
- name: fep-report analyser smoke (tarball → PASS/FAIL verdict)
173173
run: python -u tests/fep/test_report_smoke.py
174174

175+
- name: fep-report provenance fallback (git_commit across sibling logs)
176+
run: python -u tests/fep/test_report_provenance_smoke.py
177+
175178
- name: fep-binding validate dry-run (YAML hygiene, <1s)
176179
run: python -u tests/fep/test_validate_smoke.py
177180

scripts/run_binding_egfr_gpu.sh

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -50,16 +50,20 @@ OUT_DIR="run/fep/egfr_${STAMP}"
5050
mkdir -p "${OUT_DIR}"
5151
CSV="${OUT_DIR}/egfr_results.csv"
5252

53-
echo "============================================================"
54-
echo "CellSim — EGFR kinase binding FEP (Milestone B flagship)"
55-
echo "============================================================"
56-
echo " started: $(date)"
57-
echo " machine: $(uname -a)"
58-
echo " ram: $(sysctl -n hw.memsize 2>/dev/null | awk '{print $1/1024/1024/1024 " GB"}' || echo '?')"
59-
echo " git commit: $(git rev-parse HEAD)"
60-
echo " git ref: $(git describe --tags --always 2>/dev/null || echo '?')"
61-
echo " output: ${OUT_DIR}/"
62-
echo ""
53+
# Header block — mirror to env.log so `cellsim fep-report` extracts
54+
# `git commit:` for provenance. Previously stdout-only.
55+
{
56+
echo "============================================================"
57+
echo "CellSim — EGFR kinase binding FEP (Milestone B flagship)"
58+
echo "============================================================"
59+
echo " started: $(date)"
60+
echo " machine: $(uname -a)"
61+
echo " ram: $(sysctl -n hw.memsize 2>/dev/null | awk '{print $1/1024/1024/1024 " GB"}' || echo '?')"
62+
echo " git commit: $(git rev-parse HEAD)"
63+
echo " git ref: $(git describe --tags --always 2>/dev/null || echo '?')"
64+
echo " output: ${OUT_DIR}/"
65+
echo ""
66+
} | tee "${OUT_DIR}/env.log"
6367

6468
# Env + platform report.
6569
python -c "
@@ -78,7 +82,7 @@ for i in range(Platform.getNumPlatforms()):
7882
print(f' {p.getName()} (speed {p.getSpeed()})')
7983
print()
8084
print('Pipeline will prefer Metal -> CUDA -> OpenCL -> CPU.')
81-
" 2>&1 | tee "${OUT_DIR}/env.log"
85+
" 2>&1 | tee -a "${OUT_DIR}/env.log"
8286
echo ""
8387

8488
echo "=== cellsim doctor ==="

scripts/run_binding_streptavidin_gpu.sh

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,16 +48,21 @@ OUT_DIR="run/fep/streptavidin_${STAMP}"
4848
mkdir -p "${OUT_DIR}"
4949
CSV="${OUT_DIR}/streptavidin_results.csv"
5050

51-
echo "============================================================"
52-
echo "CellSim — Streptavidin binding FEP (Milestone B)"
53-
echo "============================================================"
54-
echo " started: $(date)"
55-
echo " machine: $(uname -a)"
56-
echo " ram: $(sysctl -n hw.memsize 2>/dev/null | awk '{print $1/1024/1024/1024 " GB"}' || echo '?')"
57-
echo " git commit: $(git rev-parse HEAD)"
58-
echo " git ref: $(git describe --tags --always 2>/dev/null || echo '?')"
59-
echo " output: ${OUT_DIR}/"
60-
echo ""
51+
# Header block — mirror to env.log so `cellsim fep-report` extracts
52+
# `git commit:` for provenance. Previously stdout-only, which made
53+
# report.git_commit=None on the tarball.
54+
{
55+
echo "============================================================"
56+
echo "CellSim — Streptavidin binding FEP (Milestone B)"
57+
echo "============================================================"
58+
echo " started: $(date)"
59+
echo " machine: $(uname -a)"
60+
echo " ram: $(sysctl -n hw.memsize 2>/dev/null | awk '{print $1/1024/1024/1024 " GB"}' || echo '?')"
61+
echo " git commit: $(git rev-parse HEAD)"
62+
echo " git ref: $(git describe --tags --always 2>/dev/null || echo '?')"
63+
echo " output: ${OUT_DIR}/"
64+
echo ""
65+
} | tee "${OUT_DIR}/env.log"
6166

6267
# Env + platform report (critical for reproducibility).
6368
python -c "
@@ -76,7 +81,7 @@ for i in range(Platform.getNumPlatforms()):
7681
print(f' {p.getName()} (speed {p.getSpeed()})')
7782
print()
7883
print('Pipeline will prefer Metal -> CUDA -> OpenCL -> CPU.')
79-
" 2>&1 | tee "${OUT_DIR}/env.log"
84+
" 2>&1 | tee -a "${OUT_DIR}/env.log"
8085
echo ""
8186

8287
# Cellsim doctor — fail fast if env is broken.

scripts/run_freesolv_m5max.sh

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,25 @@ STAMP=$(date +%Y%m%d_%H%M%S)
3939
OUT_DIR="run/fep/${STAMP}"
4040
mkdir -p "${OUT_DIR}"
4141

42-
echo "============================================================"
43-
echo "CellSim — FreeSolv FEP gate (Milestone A)"
44-
echo "============================================================"
45-
echo " started: $(date)"
46-
echo " machine: $(uname -a)"
47-
echo " ram: $(sysctl -n hw.memsize 2>/dev/null | awk '{print $1/1024/1024/1024 " GB"}' || echo '?')"
48-
echo " git commit: $(git rev-parse HEAD)"
49-
echo " git ref: $(git describe --tags --always 2>/dev/null || echo '?')"
50-
echo " output: ${OUT_DIR}/"
51-
echo ""
42+
# Header block — mirror to env.log so `cellsim fep-report` can
43+
# extract the `git commit:` line for provenance. Earlier versions
44+
# of this script sent the header to stdout only, which left
45+
# report.env_log metadata empty on the tarball.
46+
{
47+
echo "============================================================"
48+
echo "CellSim — FreeSolv FEP gate (Milestone A)"
49+
echo "============================================================"
50+
echo " started: $(date)"
51+
echo " machine: $(uname -a)"
52+
echo " ram: $(sysctl -n hw.memsize 2>/dev/null | awk '{print $1/1024/1024/1024 " GB"}' || echo '?')"
53+
echo " git commit: $(git rev-parse HEAD)"
54+
echo " git ref: $(git describe --tags --always 2>/dev/null || echo '?')"
55+
echo " output: ${OUT_DIR}/"
56+
echo ""
57+
} | tee "${OUT_DIR}/env.log"
5258

53-
# Env + platform report — critical for reproducibility
59+
# Env + platform report — critical for reproducibility. Appended
60+
# to env.log (not overwriting the header we just wrote).
5461
python -c "
5562
import openmm, openmmtools, openff.toolkit, pymbar, sys
5663
from openmm import Platform
@@ -66,7 +73,7 @@ for i in range(Platform.getNumPlatforms()):
6673
print(f' {p.getName()} (speed {p.getSpeed()})')
6774
print()
6875
print('Pipeline will prefer Metal → OpenCL → CUDA → CPU.')
69-
" 2>&1 | tee "${OUT_DIR}/env.log"
76+
" 2>&1 | tee -a "${OUT_DIR}/env.log"
7077
echo ""
7178

7279
# Cellsim doctor — fail fast if env is broken

src/fep/report.py

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,11 +249,20 @@ def _parse_run_log(log_path: Path, rows: list[CompoundRow]) -> dict:
249249
this section will just be empty and we report "no GHMC data in
250250
log (legacy format)". The parser still works when Phase-2 wires
251251
per-compound acceptance through the CSV.
252+
253+
Provenance fallback: if run.log doesn't carry `git commit:` (the
254+
shell header block tees into env.log on some pre-2026-04-21 runs
255+
and into run.log after), also scan env.log and doctor.log in the
256+
same directory. Biologists care that *some* log captured the
257+
commit — not which one.
252258
"""
253259
meta: dict = {"platform": None, "git_commit": None,
254260
"ghmc_means": [], "ghmc_mins": []}
255261
if not log_path.exists():
256-
return meta
262+
# Fall through to the sibling-log scan below so we at least
263+
# pick up git_commit / platform from env.log when present.
264+
return _scan_sibling_logs_for_provenance(
265+
log_path.parent, meta)
257266

258267
name_to_row = {r.name: r for r in rows}
259268
current: Optional[CompoundRow] = None
@@ -288,6 +297,37 @@ def _parse_run_log(log_path: Path, rows: list[CompoundRow]) -> dict:
288297
current.ghmc_accept_min = mn
289298
meta["ghmc_means"].append(mean)
290299
meta["ghmc_mins"].append(mn)
300+
# If provenance still missing, look in sibling logs.
301+
if not meta["git_commit"] or not meta["platform"]:
302+
meta = _scan_sibling_logs_for_provenance(log_path.parent, meta)
303+
return meta
304+
305+
306+
def _scan_sibling_logs_for_provenance(
307+
dir_path: Path, meta: dict,
308+
) -> dict:
309+
"""Scan env.log and doctor.log next to run.log for git_commit
310+
and platform. These files are written by the shell wrapper
311+
scripts; the header-tee pattern can land the provenance in any
312+
of them depending on the script version."""
313+
for sibling in ("env.log", "doctor.log", "header.log"):
314+
p = dir_path / sibling
315+
if not p.exists():
316+
continue
317+
try:
318+
t = p.read_text(encoding="utf-8", errors="replace")
319+
except OSError:
320+
continue
321+
if not meta["git_commit"]:
322+
m = _GIT_COMMIT_RE.search(t)
323+
if m:
324+
meta["git_commit"] = m.group(1).strip()
325+
if not meta["platform"]:
326+
m = _PLATFORM_RE.search(t)
327+
if m:
328+
meta["platform"] = m.group(1).strip()
329+
if meta["git_commit"] and meta["platform"]:
330+
break
291331
return meta
292332

293333

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
"""Regression tests for fep-report provenance extraction.
2+
3+
The shell wrapper scripts evolved over time in where they tee the
4+
`git commit:` line — earlier versions sent it to stdout only, then
5+
into run.log, then into env.log. `cellsim fep-report` has to find
6+
it wherever it landed, because a biologist handed a tarball from
7+
someone else's machine cannot re-run git commands: the commit hash
8+
IS the provenance.
9+
10+
Pins: _parse_run_log scans run.log, env.log, and doctor.log and
11+
returns the first git_commit / platform it finds.
12+
"""
13+
from __future__ import annotations
14+
15+
import sys
16+
import tempfile
17+
from pathlib import Path
18+
19+
REPO = Path(__file__).resolve().parents[2]
20+
sys.path.insert(0, str(REPO))
21+
22+
from src.fep.report import (
23+
_parse_run_log,
24+
_scan_sibling_logs_for_provenance,
25+
)
26+
27+
28+
def test_git_commit_picked_up_from_run_log():
29+
with tempfile.TemporaryDirectory(prefix="fep_prov_") as tmp:
30+
d = Path(tmp)
31+
(d / "run.log").write_text(
32+
"CellSim\n"
33+
" git commit: abc1234567890abcdef\n"
34+
"FEP sampling platform: CUDA\n"
35+
"[freesolv] methane C\n")
36+
meta = _parse_run_log(d / "run.log", rows=[])
37+
assert meta["git_commit"] == "abc1234567890abcdef", meta
38+
assert meta["platform"] == "CUDA", meta
39+
40+
41+
def test_git_commit_fallback_to_env_log():
42+
"""Old scripts sent the header to stdout and env.log. run.log
43+
contains only bench output. fep-report must still populate
44+
git_commit by falling back to env.log."""
45+
with tempfile.TemporaryDirectory(prefix="fep_prov_") as tmp:
46+
d = Path(tmp)
47+
(d / "env.log").write_text(
48+
"CellSim — FreeSolv FEP gate\n"
49+
" git commit: deadbeefcafe0000111122223333\n"
50+
" openmm 8.2.0\n")
51+
(d / "run.log").write_text(
52+
"[freesolv] methane C\n"
53+
"...bench output without provenance...\n"
54+
"FEP sampling platform: Metal\n")
55+
meta = _parse_run_log(d / "run.log", rows=[])
56+
assert meta["git_commit"] == "deadbeefcafe0000111122223333", meta
57+
assert meta["platform"] == "Metal", meta
58+
59+
60+
def test_git_commit_fallback_when_no_run_log():
61+
"""Malformed tarball (no run.log) — only env.log has provenance.
62+
Parser must still recover git_commit via sibling scan."""
63+
with tempfile.TemporaryDirectory(prefix="fep_prov_") as tmp:
64+
d = Path(tmp)
65+
(d / "env.log").write_text(
66+
" git commit: 1111222233334444aaaabbbbccccdddd\n")
67+
# Intentionally no run.log.
68+
meta = _parse_run_log(d / "run.log", rows=[])
69+
assert meta["git_commit"] == "1111222233334444aaaabbbbccccdddd", meta
70+
71+
72+
def test_doctor_log_as_last_resort():
73+
"""Some runs only landed provenance in doctor.log."""
74+
with tempfile.TemporaryDirectory(prefix="fep_prov_") as tmp:
75+
d = Path(tmp)
76+
(d / "doctor.log").write_text(
77+
"cellsim doctor\n"
78+
"git commit: feedface99887766\n"
79+
"OK\n")
80+
meta = _parse_run_log(d / "run.log", rows=[])
81+
assert meta["git_commit"] == "feedface99887766", meta
82+
83+
84+
def test_no_logs_at_all_returns_none():
85+
"""Nothing to find — git_commit stays None, doesn't crash."""
86+
with tempfile.TemporaryDirectory(prefix="fep_prov_") as tmp:
87+
d = Path(tmp)
88+
meta = _parse_run_log(d / "run.log", rows=[])
89+
assert meta["git_commit"] is None
90+
assert meta["platform"] is None
91+
92+
93+
def test_run_log_wins_over_sibling():
94+
"""When both run.log and env.log have git commit, run.log wins
95+
(most local to the bench process)."""
96+
with tempfile.TemporaryDirectory(prefix="fep_prov_") as tmp:
97+
d = Path(tmp)
98+
(d / "run.log").write_text(
99+
"git commit: 0000000aaaaaaaabbbbbbbbbbcccccc\n")
100+
(d / "env.log").write_text(
101+
"git commit: ffffffffffffffffffffffffffffffff\n")
102+
meta = _parse_run_log(d / "run.log", rows=[])
103+
assert meta["git_commit"] == "0000000aaaaaaaabbbbbbbbbbcccccc", meta
104+
105+
106+
def test_sibling_scan_direct_helper():
107+
"""_scan_sibling_logs_for_provenance is a public-ish helper;
108+
cover it directly so the fallback policy (env > doctor > header)
109+
is pinned."""
110+
with tempfile.TemporaryDirectory(prefix="fep_prov_") as tmp:
111+
d = Path(tmp)
112+
(d / "env.log").write_text("git commit: aaaaaaaaaaaaaaaa\n")
113+
(d / "doctor.log").write_text("git commit: bbbbbbbbbbbbbbbb\n")
114+
meta = _scan_sibling_logs_for_provenance(
115+
d, {"git_commit": None, "platform": None,
116+
"ghmc_means": [], "ghmc_mins": []})
117+
# env.log scanned first.
118+
assert meta["git_commit"] == "aaaaaaaaaaaaaaaa", meta
119+
120+
121+
if __name__ == "__main__":
122+
funcs = [
123+
test_git_commit_picked_up_from_run_log,
124+
test_git_commit_fallback_to_env_log,
125+
test_git_commit_fallback_when_no_run_log,
126+
test_doctor_log_as_last_resort,
127+
test_no_logs_at_all_returns_none,
128+
test_run_log_wins_over_sibling,
129+
test_sibling_scan_direct_helper,
130+
]
131+
fails = []
132+
for f in funcs:
133+
try:
134+
f()
135+
print(f"[PASS] {f.__name__}")
136+
except AssertionError as e:
137+
print(f"[FAIL] {f.__name__}: {e}")
138+
fails.append(f.__name__)
139+
except Exception as e:
140+
import traceback
141+
traceback.print_exc()
142+
print(f"[ERROR] {f.__name__}: {e}")
143+
fails.append(f.__name__)
144+
print(f"{len(funcs) - len(fails)}/{len(funcs)} PASS")
145+
sys.exit(0 if not fails else 1)

0 commit comments

Comments
 (0)