diff --git a/tools/battery/README.md b/tools/battery/README.md index 18bec5d32..6b31188fa 100644 --- a/tools/battery/README.md +++ b/tools/battery/README.md @@ -130,13 +130,17 @@ everything else on the machine competes with it. ### The model check (open arm) -If `tools/battery/model_check.py` is present, each open-arm merge (`p.mtz`) is also scored -against the deposited model: `model_check.check(mtz, pdb_id, workdir)` returns R-free and R-work -of the deposited model against our data, plus the deposited R-free. The runner records these as -`rfree`, `rwork`, `rfree_deposited`, `rfree_ratio`, `model_status` and `model_reason`, and the -report plots their ratio. The check is on by default when the module exists. Without the module -the fields stay null. The PDB entry is the set id up to the first `_`, so `6h2p_native` is checked -as `6h2p`. Small-molecule sets are skipped. +`tools/battery/model_check.py` scores each open-arm merge (`p.mtz`) against the deposited model, +unmodified (ligands, waters and hydrogens kept): REFMAC's rigid-body mode, R-factors taken before +any shift, the data reindexed into the model's setting and reduced to the deposited space group. +Our own free set was mostly work reflections for the depositor, so `rfree` reads low. The fair +comparison is `rfree_depflags` (our data on the depositor's free set) against `rfree_depdata` (the +depositor's structure factors, same free set, same protocol); `rfree_ratio` is that ratio, and the +report plots it. It is null when the entry deposited no structure factors with a free set. +`rfree_deposited` is the value in the entry, often from twin or full refinement. The runner records +`rfree`, `rwork`, `rfree_deposited`, `rfree_depflags`, `rfree_depdata`, `rfree_ratio`, +`model_status` and `model_reason`. The check is on by default when the module exists. The PDB entry +is the set id up to the first `_`, so `6h2p_native` is checked as `6h2p`. Small-molecule sets are skipped. `model_sweep.py` is a different tool: it runs `rugnux --model` over the open arm (same inputs, same site config) and compares the R-free that rugnux itself reports with the published value. It needs @@ -179,7 +183,7 @@ This is a flat list with one object per set, and every arm uses the same keys. M | `d_min_ref_rule`, `d_min_xds` | XDS arms: how `d_min_ref` was derived (below), and the range XDS merged | | `r_meas`, `cc_half`, `isa`, `completeness`, `multiplicity`, `i_over_sigma`, `indexing_rate`, `images` | from rugnux's `_report.txt` | | `r_meas_ref`, `cc_half_ref`, `isa_ref`, `completeness_ref` | XDS's overall values, over XDS's own range | -| `rfree`, `rwork`, `rfree_deposited`, `rfree_ratio`, `model_status`, `model_reason` | the model check (open arm) | +| `rfree`, `rwork`, `rfree_deposited`, `rfree_depflags`, `rfree_depdata`, `rfree_ratio`, `model_status`, `model_reason` | the model check (open arm) | | `wall_s`, `rugnux_wall_s`, `elapsed_s`, `exit_code`, `gpu_others`, `rugnux_verdict` | timing (`wall_s` is rugnux's own WALL_TIME, not the time spent waiting for the GPU queue), the exit code, other GPU processes seen before the set, and rugnux's own verdict line | ## The report diff --git a/tools/battery/battery.py b/tools/battery/battery.py index ceed6ed20..b369d949e 100644 --- a/tools/battery/battery.py +++ b/tools/battery/battery.py @@ -191,7 +191,7 @@ def run_one(run_dir, binary, e, opts): os.makedirs(wd) row = {"set": e["id"], "arm": e["arm"], "tags": e.get("tags", []), "input": e["input_path"], "gpu_others": gpu_others(), "rfree": None, "rwork": None, "rfree_deposited": None, - "rfree_ratio": None, "model_status": None, "model_reason": None} + "rfree_depflags": None, "rfree_depdata": None, "rfree_ratio": None, "model_status": None, "model_reason": None} if not os.path.exists(e["input_path"]): row.update(score.judge(e, {}, "no input"), exit_code=None, elapsed_s=None, wall_s=None, cmd=None) return row @@ -221,7 +221,8 @@ def run_one(run_dir, binary, e, opts): def check_model(e, wd): """R-free of the deposited model against this run's merge (model_check.py). A set id may carry a suffix (6h2p_native); the PDB entry is the part before it.""" - out = {"rfree": None, "rwork": None, "rfree_deposited": None, "rfree_ratio": None} + out = {"rfree": None, "rwork": None, "rfree_deposited": None, "rfree_depflags": None, + "rfree_depdata": None, "rfree_ratio": None} pdb = e["id"].split("_")[0] mtz = os.path.join(wd, "p.mtz") if not PDB_ID.match(pdb): @@ -232,10 +233,14 @@ def check_model(e, wd): res = model_check.check(mtz, pdb, os.path.join(wd, "model_check")) except Exception as ex: # a failing check must not lose the set's processing result return dict(out, model_status="error", model_reason=f"{type(ex).__name__}: {ex}"[:300]) - out.update({k: res.get(k) for k in ("rfree", "rwork", "rfree_deposited")}, + out.update({k: res.get(k) for k in ("rfree", "rwork", "rfree_deposited", "rfree_depflags", + "rfree_depdata")}, model_status=res.get("status"), model_reason=res.get("reason")) - if out["rfree"] and out["rfree_deposited"]: - out["rfree_ratio"] = round(out["rfree"] / out["rfree_deposited"], 4) + # The fair ratio: our merge and the depositor's structure factors, both scored on the + # depositor's free set by the same protocol. Our own free set was mostly work reflections + # for the depositor, so R_free on it reads low. + if out["rfree_depflags"] and out["rfree_depdata"]: + out["rfree_ratio"] = round(out["rfree_depflags"] / out["rfree_depdata"], 4) return out diff --git a/tools/battery/report.py b/tools/battery/report.py index fb4f6eef6..0acad8427 100644 --- a/tools/battery/report.py +++ b/tools/battery/report.py @@ -268,12 +268,13 @@ def build(run_dir, baseline=None, allow_incomplete=False): f"{arm}: d_min(rugnux) / d_min(reference), one point per set, sorted; below 1 = " "finer than the reference") pts = [(r["set"], r["rfree_ratio"], r["verdict"], - f"R_free {r['rfree']:.3f} vs deposited {r['rfree_deposited']:.3f}") + f"R_free {r['rfree_depflags']:.3f} vs {r['rfree_depdata']:.3f} on the deposited data") for r in res if r.get("rfree_ratio")] if pts: doc.svg(ratio_dots(pts, "R_free ratio"), - "open: R_free of the deposited model against our merge / the deposited R_free, one " - "point per set, sorted; below 1 = our data fit the model better") + "open: R_free of the deposited model against our merge / against the deposited structure " + "factors, both on the depositor's free set with the same protocol, one point per set, " + "sorted; below 1 = our data fit the model better") derived = [r for r in res if (r.get("d_min_ref_rule") or "xds_range") != "xds_range"] if derived: