diff --git a/tools/battery/README.md b/tools/battery/README.md index 5629ef4b1..1e4783e24 100644 --- a/tools/battery/README.md +++ b/tools/battery/README.md @@ -860,10 +860,9 @@ whole range; the manifest keeps the rule used as `dmin_rule`. reference (`sg`, `sgno`, `cell`, `dmin`) and tags. Set `"pinned": true` when the choice between sweeps was deliberate. Record the source and DOI in `docs/EXTERNAL_TEST_DATA.md`. A set whose frames are not the crystal the deposited data came from (checked independently, - e.g. with XDS on the same frames) gets `"reference_mismatch": ""`: the REFMAC check and the - depositor-data comparison are skipped (`refmac_status` / `dep_status` `skipped`, the reason in - `refmac_reason` / `dep_reason` and in the report), while space group, cell and d_min are still - scored against the deposition. + e.g. with XDS on the same frames) gets `"reference_mismatch": ""`. It is an annotation only: + every check still runs and every number still counts; the report prints the note beside the set's + row and under the REFMAC plot, so an outlier that has an explanation stays visible with it. - **Small molecules** carry `"cod": ""`, the published structure the SHELXL check refines (pick an entry whose Niggli-reduced cell and space group match the row's reference; prefer a modern, ordered, low-R one). diff --git a/tools/battery/battery.py b/tools/battery/battery.py index a7df13e9f..0c1dc3934 100644 --- a/tools/battery/battery.py +++ b/tools/battery/battery.py @@ -477,7 +477,7 @@ def run_one(run_dir, binary, e, opts): if e.get("timing"): row.update(timing.check(e["timing"], row, wd, reference)) elif e["reference"] == "deposition" and row["model"] and os.path.exists(os.path.join(wd, "p.mtz")): - row.update(check_depdata(e, wd, opts["pdb_cache"])) + row.update(depdata_check.check_set(wd, e["id"], opts["pdb_cache"])) if opts["model_check"] and e["arm"] == "open": row.update(check_model(e, wd)) if e.get("cod"): @@ -496,20 +496,6 @@ REFMAC_KEYS = ("refmac_rfree", "refmac_rwork", "refmac_rfree_depflags", "refmac_ "refmac_reason", "refmac_refined_ratio") + tuple("refmac_" + k for k in REFMAC_REFINED) -def mismatch_reason(e): - """Why the deposited data are not a reference for this set's frames (open.json's - `reference_mismatch`), or None: the REFMAC and depositor-data checks are then skipped.""" - return e.get("reference_mismatch") and "reference mismatch: " + e["reference_mismatch"] - - -def check_depdata(e, wd, pdb_cache): - """Our merge against the depositor's data (depdata_check.py), unless the manifest says the - deposition is not a reference for these frames.""" - if mismatch_reason(e): - return dict({k: None for k in depdata_check.KEYS}, dep_status="skipped", dep_reason=mismatch_reason(e)) - return depdata_check.check_set(wd, e["id"], pdb_cache) - - def check_model(e, wd): """R-free of the deposited model against this run's merge, by REFMAC (model_check.py).""" out = {k: None for k in REFMAC_KEYS} @@ -517,8 +503,6 @@ def check_model(e, wd): mtz = os.path.join(wd, "p.mtz") if not PDB_ID.match(pdb): return dict(out, refmac_status="skipped", refmac_reason="not a PDB entry") - if mismatch_reason(e): - return dict(out, refmac_status="skipped", refmac_reason=mismatch_reason(e)) if not os.path.exists(mtz): return dict(out, refmac_status="skipped", refmac_reason="no merged MTZ") try: @@ -1005,7 +989,7 @@ def recheck_one(e, wd, pdb_cache): """The post-processing checks of one open-arm row, on the run's own p.mtz (a worker process).""" out = {} if e["model"] and os.path.exists(os.path.join(wd, "p.mtz")): - out.update(check_depdata(e, wd, pdb_cache)) + out.update(depdata_check.check_set(wd, e["id"], pdb_cache)) out.update(check_model(e, wd)) return out @@ -1047,10 +1031,8 @@ def cmd_recheck(a, site): set_writable([os.path.join(wd, "model_check")]) os.rename(os.path.join(wd, "model_check"), os.path.join(wd, "model_check.pre-recheck-" + stamp)) shutil.copy2(os.path.join(run, "results.json"), os.path.join(run, "results.json.pre-recheck-" + stamp)) - mismatch = {e["id"]: e.get("reference_mismatch") for e in load_sets(site, "open")} with concurrent.futures.ProcessPoolExecutor(a.jobs) as pool: - futures = {pool.submit(recheck_one, {"id": r["set"], "model": r.get("model"), - "reference_mismatch": mismatch.get(r["set"])}, wd, pdb_cache): r + futures = {pool.submit(recheck_one, {"id": r["set"], "model": r.get("model")}, wd, pdb_cache): r for r, wd in zip(rows, wds)} for n, fut in enumerate(concurrent.futures.as_completed(futures), 1): r = futures[fut] diff --git a/tools/battery/report.py b/tools/battery/report.py index c9475c4bd..e31e9841d 100644 --- a/tools/battery/report.py +++ b/tools/battery/report.py @@ -201,6 +201,14 @@ ALIASES = {} # {arm: {set id: manifest row}} of today's manifests (battery.py sets it); rescore() scores with them MANIFESTS = {} + +def reference_note(r): + """The manifest's reference_mismatch note for a row (why its deposition is a poor reference for + these frames), or None. An annotation only: the row is scored and plotted as any other.""" + note = (MANIFESTS.get(r["arm"], {}).get(r["set"]) or {}).get("reference_mismatch") + return note and "reference mismatch: " + note + + # the site's cache of deposited models and structure factors (battery.py sets it) PDB_CACHE = None @@ -523,9 +531,9 @@ def build(run_dir, baseline=None, allow_incomplete=False): "REFMAC check (--model-check): 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") - skipped = [r for r in res if r.get("refmac_status") == "skipped" and r["arm"] == "open"] - if skipped: - doc.p("REFMAC check skipped: " + "; ".join(f"{r['set']} ({r['refmac_reason']})" for r in skipped)) + notes = [r for r in res if reference_note(r)] + if notes: + doc.p("Read with the manifest's note: " + "; ".join(f"{r['set']} - {reference_note(r)}" for r in notes)) dep = [r for r in res if r.get("dep_status") == "ok"] if dep: @@ -547,9 +555,9 @@ def build(run_dir, baseline=None, allow_incomplete=False): doc.table(["set", "dep. data", "our d_min", "dep. d_min", "common refl.", "dCC all", "dCC outer 2", "CC past limit", "to d A", "REFMAC ratio", "ISa", "tags"], rows, num=range(2, 11)) - other = [r for r in res if r.get("dep_status") not in (None, "ok")] - if other: - doc.p("No depositor-data comparison: " + "; ".join(f"{r['set']} ({r['dep_reason']})" for r in other)) + other = [r for r in res if r.get("dep_status") not in (None, "ok")] + if other: + doc.p("No comparison: " + "; ".join(f"{r['set']} ({r['dep_reason']})" for r in other)) xds = [r for r in res if r.get("refres_range")] if xds: @@ -670,7 +678,8 @@ def build(run_dir, baseline=None, allow_incomplete=False): [h for h, _ in model] + (["model fit"] if model else []) + ["time s", "note"] rows = [] for r in sorted(res, key=lambda r: (order.get(r["arm"], 9), r["set"])): - note = "; ".join(x for x in (r.get("reason"), r.get("sg_label") and f"labelled {r['sg_label']} from the model") + note = "; ".join(x for x in (r.get("reason"), r.get("sg_label") and f"labelled {r['sg_label']} from the model", + reference_note(r)) if x) rows.append([r["set"], r["arm"], r["verdict"], r.get("sg") or "-", r.get("sg_ref") or "-", f(r.get("cell_dev_pct")), f(r.get("volume_ratio"), "{:.3f}"),