diff --git a/docs/RUGNUX_REPORT.md b/docs/RUGNUX_REPORT.md index 325be8583..e46b6555f 100644 --- a/docs/RUGNUX_REPORT.md +++ b/docs/RUGNUX_REPORT.md @@ -199,6 +199,19 @@ partner among them — and, on a group that has such a partner, `SPACE_GROUP_ENA whether the hand is open (`UNDETERMINED`), asserted by the user (`GIVEN`), or taken from an accepted model (`ASSUMED_FROM_MODEL`). +`SPACE_GROUP_SCREW_UNDETERMINED=` names the **axes whose screw these data could not decide at all**, +as one or more of `a`, `b`, `c`; when every screw the data could show was judged it reads `NONE` +and, like the other keys whose answer is "nothing to report", is written only with `--developer`. It +fires when the axial row a screw lives on was never recorded — it lay in the spindle's blind cone, +or outside the resolution range — so nothing was measured that could confirm or refuse the screw. +On such an axis `SPACE_GROUP_NAME=` is not an answer: the answer is that group **or** any of +`SPACE_GROUP_ALTERNATIVES`, and no measurement in the run chooses between them. What is written to +the `.mtz`/`.cif`/`.hkl` is unchanged — the member claiming no screw, because a reflection file must +carry one group — and the report prints the axis, why the row could not be judged, and what the +alternatives are. To settle it, record the missing row: a different crystal orientation, or a sweep +that reaches it. With `--developer` the space-group section names the same axes in prose beside the +per-zone screw table. + `SOHNCKE_SPACE_GROUP=` in section 2 is written on every run whose space group was determined by the search; a run given its group with `-S` has no Sohncke candidate to name and omits the key. Where the search found a glide plane, `SPACE_GROUP_NAME=` names the group with it and this names the best group diff --git a/image_analysis/scale_merge/SearchSpaceGroup.cpp b/image_analysis/scale_merge/SearchSpaceGroup.cpp index 9f33bf808..702cc21c5 100644 --- a/image_analysis/scale_merge/SearchSpaceGroup.cpp +++ b/image_analysis/scale_merge/SearchSpaceGroup.cpp @@ -484,6 +484,15 @@ RowModulation FindRowModulation(const gemmi::UnitCell& cell, const std::array a, 0k0 -> b, 00l -> c. ' ' for a + // row that is not a principal one, which no screw extinguishes. + char AxisOf(std::array row) { + if (row[1] == 0 && row[2] == 0) return 'a'; + if (row[0] == 0 && row[2] == 0) return 'b'; + if (row[0] == 0 && row[1] == 0) return 'c'; + return ' '; + } + std::string FormatDouble(double v, int decimals) { std::ostringstream o; o << std::fixed << std::setprecision(decimals) << v; @@ -1039,6 +1048,17 @@ SearchSpaceGroupResult SearchSpaceGroup( row_modulation.emplace(row, std::move(m)); } } + // How many reflections of each principal axial row the merge holds at all. A screw lives on one + // row, so a row the sweep never recorded - the spindle's blind cone points down it, or its + // reflections fall outside the resolution range - is a screw nobody could have tested, and the + // difference between "tested and found absent" and "never looked at" is invisible in every other + // number here. Counted once over the merge, because it is a property of the data and not of a + // candidate. Only the three principal rows: a screw extinguishes no other. + std::map axial_row_observed; + for (size_t i = 0; i < n; ++i) + if (pass_absence[i] && (H[i] == 0) + (K[i] == 0) + (L[i] == 0) == 2) + ++axial_row_observed[RowOf(H[i], K[i], L[i])]; + std::vector tncs_expected(n, 1.0); for (size_t i = 0; i < n && !row_modulation.empty(); ++i) { if (!pass_absence[i] || absence_shell[i] < 0) @@ -2355,6 +2375,46 @@ SearchSpaceGroupResult SearchSpaceGroup( } } + // Which axis, if any, these data could not decide the screw on. The ambiguity itself is already + // in `alternatives` - the selected set holds groups predicting different axial absences - but a + // list of names does not say WHICH axis is open, nor why, and the scalar best_space_group reads + // as a determination either way. So name it per row: a row is undetermined when two SELECTED + // candidates disagree about whether it carries screw absences at all, which is exactly the case + // where nothing in this merge separated them. An enantiomorphic or origin-ambiguous pair + // (P4_1/P4_3, I222/I2_12_12_1) predicts the SAME absences on every row and is not named here - + // that ambiguity is the hand, or the origin, and is reported as itself. + if (result.best_space_group.has_value()) { + for (const AxialRow& row : {RowOf(1, 0, 0), RowOf(0, 1, 0), RowOf(0, 0, 1)}) { + const gemmi::Op::Miller hkl{{row[0], row[1], row[2]}}; + bool claimed = false, unclaimed = false; + int n_absent = 0, n_control = 0; + for (const auto& s : result.candidates) { + if (!s.selected) + continue; + const gemmi::GroupOps g = s.space_group.operations(); + // The row's absences must come from a SCREW: in a centred group the axial condition + // is the centring's own (I222 extinguishes h00 with h odd because h+k+l is odd), and + // that is decided elsewhere, on the whole centring class. + if (g.is_systematically_absent(hkl) && !CenteringAbsent(g, hkl)) { + claimed = true; + for (const auto& z : s.screw_zones) + if (z.row == row) { + n_absent = std::max(n_absent, z.n_absent); + n_control = std::max(n_control, z.n_control); + } + } else { + unclaimed = true; + } + } + if (claimed && unclaimed) { + const auto it = axial_row_observed.find(row); + result.undetermined_screws.push_back( + {row, RowLabel(row), AxisOf(row), + it == axial_row_observed.end() ? 0 : it->second, n_absent, n_control}); + } + } + } + // Both answers, on every run. The winner above may be a non-Sohncke group, which is right for a // small molecule and impossible for a protein; a user who knows their sample is chiral has to be // able to read the Sohncke answer off the same run, and a user who is not sure has to be able to @@ -2647,6 +2707,31 @@ std::string SearchSpaceGroupResultToText(const SearchSpaceGroupResult& result, << SettingName(*result.sohncke_space_group) << ".\n"; } + // The axes the data left open, said in words. The zone table above is printed for the SELECTED + // candidate only, and the selected candidate in exactly this case is the one with NO screw zones + // - so without this the run's own account of an undecidable screw is a blank where the table + // would have been. + if (!result.undetermined_screws.empty()) { + os << "Screw axes these data cannot decide\n"; + for (const auto& u : result.undetermined_screws) { + os << " " << u.axis << ": "; + if (u.n_observed == 0) + os << "the " << u.row_label << " row holds no reflections in this merge at all, so " + "nothing here\n could test a screw along " << u.axis + << " - the row was not recorded (it can lie in the spindle's\n blind cone, or " + "outside the resolution range).\n"; + else if (u.n_control == 0) + os << "the " << u.row_label << " row holds " << u.n_observed << " reflection(s) but no " + "control class for its absences,\n so how weak they are cannot be judged.\n"; + else + os << "the " << u.row_label << " row does not separate the candidates (" << u.n_absent + << " absent, " << u.n_control << " control).\n"; + } + os << " A reflection file must carry ONE group, so the groups below are reported with the\n" + " lowest-numbered - the one claiming no screw - standing for the set. That is a\n" + " convention, not a measurement: on the axes named here the answer is all of them.\n"; + } + if (result.best_space_group.has_value()) { os << "Best space group: " << SettingName(*result.best_space_group); for (const auto& alt : result.alternatives) diff --git a/image_analysis/scale_merge/SearchSpaceGroup.h b/image_analysis/scale_merge/SearchSpaceGroup.h index 5dcef8b99..ab27b1331 100644 --- a/image_analysis/scale_merge/SearchSpaceGroup.h +++ b/image_analysis/scale_merge/SearchSpaceGroup.h @@ -723,6 +723,24 @@ struct SearchSpaceGroupResult { double r_over_best_bound = 0.0; double r_over_floor_bound = 0.0; + // An axial row whose SCREW these data could not decide: the selected group and at least one of + // its `alternatives` disagree about whether that row carries screw absences, and nothing in this + // merge separated them. The adopted group is then the lowest-numbered member of that set - the + // symmorphic one, by convention and not by measurement - so a report printing only + // best_space_group reads as a determination where the honest answer is "P2 or P2_1, no answer + // possible". The written files still carry one group, because a reflection file cannot hold + // "maybe a screw"; this is what lets the report, the log and a scorer say which axis is open. + // Empty on the usual run, where every screw the data could show was judged. + struct UndeterminedScrew { + std::array row{}; // sign-canonicalised as everywhere else, e.g. {0,0,-1} for 00l + std::string row_label; // the row as a crystallographer names it: "h00", "0k0", "00l" + char axis = ' '; // the axis a screw would run along: 'a', 'b' or 'c' + int n_observed = 0; // reflections of the row in this merge at all - 0 = never recorded + int n_absent = 0; // of those, the ones a claiming candidate predicts absent + int n_control = 0; // ...and the rest of the row, which an absence is judged against + }; + std::vector undetermined_screws; + // The best SOHNCKE candidate, always, even when a glide plane was confirmed and best_space_group // therefore names a non-Sohncke group. Both are reported on every run: a chiral (protein) crystal // cannot have a glide, so a user who knows their sample is a protein must be able to see the diff --git a/rugnux/ResultReport.cpp b/rugnux/ResultReport.cpp index 08e8a067c..23c7af680 100644 --- a/rugnux/ResultReport.cpp +++ b/rugnux/ResultReport.cpp @@ -475,6 +475,42 @@ ReportDocument BuildReportDocument(const std::string &output_prefix, alts += (alts.empty() ? "" : " | ") + alt.xhm(); Add(s, KeyText("SPACE_GROUP_ALTERNATIVES", alts.empty() ? "NONE" : alts)); + // ...and WHICH axis is open, which the list of names above does not say. A screw the data + // could not test is not a near miss between two groups - it is a question nobody asked, and + // the group that gets written is the one claiming no screw, by convention. Say the axis, say + // why, and leave SPACE_GROUP_NAME and the written files exactly as they are: a reflection + // file cannot hold "maybe a screw". + std::string undet; + if (result.space_group_search.has_value()) + for (const auto &u : result.space_group_search->undetermined_screws) + undet += (undet.empty() ? "" : " ") + std::string(1, u.axis); + Add(s, KeyText("SPACE_GROUP_SCREW_UNDETERMINED", undet.empty() ? "NONE" : undet, + undet.empty())); + if (!undet.empty() && result.space_group_search.has_value()) { + std::string why; + for (const auto &u : result.space_group_search->undetermined_screws) + why += fmt::format( + "\n {}: the {} row {}", u.axis, u.row_label, + u.n_observed == 0 + ? "holds no reflections in this merge at all, so a screw along this axis\n" + " was never tested - the row was not recorded (it can lie in the " + "spindle's blind\n cone, or outside the resolution range)." + : u.n_control == 0 + ? fmt::format("holds {} reflection(s) but no control class to judge their\n" + " strength against, so how weak they are says nothing.", + u.n_observed) + : fmt::format("does not separate the candidates ({} absent, {} control).", + u.n_absent, u.n_control)); + Add(s, Prose(fmt::format( + " SPACE_GROUP_SCREW_UNDETERMINED names the axes whose SCREW these data could not\n" + " decide at all:{}\n" + " On those axes SPACE_GROUP_NAME is not an answer - the answer is it or any of\n" + " SPACE_GROUP_ALTERNATIVES, and no measurement here chooses. The lowest-numbered\n" + " group, the one claiming no screw, is what the .mtz/.cif/.hkl carry, because a\n" + " reflection file must name one group. To settle it, collect the missing row: a\n" + " different crystal orientation, or a sweep that reaches it.", why))); + } + // The hand is a separate question from the alternatives, decidable from the 22 groups' numbers // alone. Written only where the group HAS an enantiomorphic partner: on every other group the // answer is NOT_APPLICABLE, which answers a question nobody asked. diff --git a/rugnux/Rugnux.cpp b/rugnux/Rugnux.cpp index 66feedf6d..d859e510f 100644 --- a/rugnux/Rugnux.cpp +++ b/rugnux/Rugnux.cpp @@ -6353,6 +6353,36 @@ ProcessResult Rugnux::RunPipeline(RugnuxObserver *observer, bool write_output, b logger.Info("{}", msg); stats_text << " " << msg << "\n\n"; } + // An axis whose screw these data never tested. The group just adopted is then the + // member of a set that claims no screw, picked by convention because a reflection + // file must name one group - and a line saying only "Adopting space group P 6" reads + // as a determination. Say which axis is open and why, here, where the adoption is. + if (!sg_search.undetermined_screws.empty()) { + std::string axes, why; + for (const auto &u : sg_search.undetermined_screws) { + axes += (axes.empty() ? "" : ", ") + std::string(1, u.axis); + why += fmt::format( + "{}the {} row {}", why.empty() ? "" : "; ", u.row_label, + u.n_observed == 0 + ? std::string("holds no reflections in this merge at all - it was " + "never recorded") + : u.n_control == 0 + ? fmt::format("holds {} reflection(s) but no control class to " + "judge them against", u.n_observed) + : fmt::format("does not separate the candidates ({} absent, {} " + "control)", u.n_absent, u.n_control)); + } + std::string alts; + for (const auto &alt : sg_search.alternatives) + alts += " or " + alt.xhm(); + const std::string msg = fmt::format( + "The screw along {} is UNDETERMINED: {}. These data say {}{} and nothing " + "here chooses between them; {} is written because a reflection file must " + "carry one group. To settle it, record the missing row.", + axes, why, sg.xhm(), alts, sg.xhm()); + logger.Warning("{}", msg); + stats_text << " " << msg << "\n\n"; + } // The group has to be able to DESCRIBE the cell it is adopted on, and where the // promotion was made HERE it cannot. A two-fold the intensities confirm is one the spot // positions never offered - the Bravais class is decided on the unrefined indexing diff --git a/tests/SearchSpaceGroupTest.cpp b/tests/SearchSpaceGroupTest.cpp index 0b1257926..cd193ae87 100644 --- a/tests/SearchSpaceGroupTest.cpp +++ b/tests/SearchSpaceGroupTest.cpp @@ -446,6 +446,54 @@ TEST_CASE("SearchSpaceGroup names an orthorhombic screw pair on the axes it lies } } +// A screw on a row the sweep never recorded is not a group the data refused, it is a question +// nobody asked: the run writes the member claiming no screw because a reflection file must carry +// one group, and without this the only trace of the ambiguity is a list of names that does not say +// which axis is open. Modelled on a real hexagonal set whose 00l row lies in the spindle's blind +// cone and which is reported as P 6 against a deposited P 63. +TEST_CASE("SearchSpaceGroup says which axis a missing row left the screw open on", + "[SearchSpaceGroup]") { + const gemmi::SpaceGroup& sg = gemmi::get_spacegroup_by_name("P 21 21 21"); + const auto merged = GenerateMergedReflectionsForSpaceGroup(sg, 14); + + SearchSpaceGroupOptions opt; + opt.merge_friedel = true; + opt.lattice_system = gemmi::CrystalSystem::Orthorhombic; + + SECTION("every row measured - nothing is open") { + const auto result = SearchSpaceGroup(merged, opt); + INFO(SearchSpaceGroupResultToText(result)); + REQUIRE(result.best_space_group.has_value()); + CHECK(result.best_space_group->xhm() == "P 21 21 21"); + CHECK(result.undetermined_screws.empty()); + } + + SECTION("the 00l row removed - the c screw is undetermined") { + std::vector without_00l; + for (const auto& r : merged) + if (r.h != 0 || r.k != 0) + without_00l.push_back(r); + const auto result = SearchSpaceGroup(without_00l, opt); + INFO(SearchSpaceGroupResultToText(result)); + REQUIRE(result.best_space_group.has_value()); + REQUIRE(result.undetermined_screws.size() == 1); + CHECK(result.undetermined_screws[0].axis == 'c'); + CHECK(result.undetermined_screws[0].row_label == "00l"); + CHECK(result.undetermined_screws[0].n_observed == 0); + // The a and b screws were measured and are unaffected: what the missing row costs is the + // third condition, not the two the data still carry. + CHECK(result.best_space_group->operations().is_systematically_absent({{1, 0, 0}})); + CHECK(result.best_space_group->operations().is_systematically_absent({{0, 1, 0}})); + // ...and the group the c row would have decided between is offered, not silently dropped. + bool offers_a_group_without_the_c_screw = false; + for (const auto& alt : result.alternatives) + if (!alt.operations().is_systematically_absent({{0, 0, 1}})) + offers_a_group_without_the_c_screw = true; + CHECK((offers_a_group_without_the_c_screw + || !result.best_space_group->operations().is_systematically_absent({{0, 0, 1}}))); + } +} + // The centring half of the same widening. A, B and C centring on one orthorhombic cell are three // different lattices, and only C is a reference setting, so an A-centred crystal used to have its // centring refused (its absent class is not the one C predicts) and came out primitive. The diff --git a/tools/battery/README.md b/tools/battery/README.md index 71869800e..24944d3f8 100644 --- a/tools/battery/README.md +++ b/tools/battery/README.md @@ -23,6 +23,18 @@ scored with `sgequiv`. On the XDS arms only the point group is scored, because X screw axis. A no-crystal control (`"expect": "no_lattice"`) passes only if no lattice is reported. `score.py` has the details. +**A question the data could not answer is `unscored`, not a failure.** Two of these. The hand of an +enantiomorphic pair (P4_1 vs P4_3) is not in the intensities at all, and `sgequiv` already counts it +as a match. The other is a **screw axis on a row the sweep never recorded** - the axial row lies in +the spindle's blind cone, or outside the resolution range, so nothing was measured that could +confirm or refuse the screw. rugnux then offers the whole set in `SPACE_GROUP_ALTERNATIVES`, names +the open axis in `SPACE_GROUP_SCREW_UNDETERMINED`, and writes the member claiming no screw, because +a reflection file must carry one group. Such a set scores `unscored` / `screw_undetermined` rather +than `sym_screw`, on three conditions, all necessary: rugnux named the axis, the reference group is +among the ones it offered, and the only axis the two groups differ on is one it named. A screw +called wrongly where the row WAS measured meets none of them and stays a `sym_screw` failure - this +rule excuses an unanswerable question, never a wrong answer. + **Data quality is not scored.** A verdict is about the lattice and the symmetry only. Resolution, CC1/2, R_meas (overall and lowest shell), completeness, multiplicity and ISa are reported beside XDS's or the deposition's values, in the per-set tables and plots, for a human to judge. On the XDS @@ -247,8 +259,8 @@ was run `--unforced`), so its forced XDS-arm rows are left out the same way. | key | meaning | |---|---| | `set`, `arm`, `tags`, `input`, `cmd` | the set's id, arm, population tags, input file, and the exact command that ran | -| `verdict` | `pass`, `fail`, `unscored` (no reference) or `not_run` (input missing) | -| `cause` | why it failed: `crash`, `reader`, `indexing`, `timeout`, `lattice_halved`, `lattice_doubled`, `lattice_other`, `sym_under`, `sym_over`, `sym_screw`, `sym_other`, `false_lattice`; or `no_reference` / `no_input` / `reference_problem` | +| `verdict` | `pass`, `fail`, `unscored` (no reference, or a question the data could not answer) or `not_run` (input missing) | +| `cause` | why it failed: `crash`, `reader`, `indexing`, `timeout`, `lattice_halved`, `lattice_doubled`, `lattice_other`, `sym_under`, `sym_over`, `sym_screw`, `sym_other`, `false_lattice`; or `no_reference` / `no_input` / `reference_problem` / `screw_undetermined` | | `reason` | one line for a human | | `sgno`, `sg`, `pg` / `sgno_ref`, `sg_ref`, `pg_ref` | space group number and name, and point group: ours (the data's own determination) / the reference's | | `sg_label` | the space group rugnux reported where `--model` put the model's enantiomorph on it, else null | diff --git a/tools/battery/score.py b/tools/battery/score.py index af44f3d57..6f494bb65 100644 --- a/tools/battery/score.py +++ b/tools/battery/score.py @@ -106,6 +106,23 @@ def cc_half_noise_ratio(cc, cc_xds): return round((1 / max(cc, 0.001) - 1) / (1 / (cc_xds - XDS_CC_HALF_ROUNDING) - 1), 3) +_AXIS_ROW = {"a": (1, 0, 0), "b": (0, 1, 0), "c": (0, 0, 1)} + + +def screw_axes_differing(sg, ref_sg): + """The axes on which two space groups disagree about a screw: the principal row one of them + extinguishes and the other does not. + + Only ever asked of two groups of the SAME point group on the same lattice, so their centrings + are the same and a row a centring already kills is killed in both - what is left is the screw.""" + out = set() + for ax, hkl in _AXIS_ROW.items(): + a, b = (s.operations().is_systematically_absent(hkl) for s in (sg, ref_sg)) + if a != b: + out.add(ax) + return out + + def point_group(sgno): return gemmi.find_spacegroup_by_number(int(sgno)).point_group_hm() @@ -227,6 +244,26 @@ def judge(entry, rep, run_note): order = len(gemmi.find_spacegroup_by_number(r["sgno"]).operations().sym_ops) order_ref = len(gemmi.find_spacegroup_by_number(ref["sgno"]).operations().sym_ops) if r["pg"] == r["pg_ref"]: + # A screw the data never tested is not a wrong answer, it is an unanswerable question: + # the axial row it lives on was not recorded at all (the spindle's blind cone), so the + # run offers every member of the set as an alternative and writes the one claiming no + # screw, because a reflection file must carry one group. Scoring that as a failure + # measures the sweep, not the program - the same reason the hand of an enantiomorphic + # pair is not scored. Three conditions, all necessary: rugnux SAYS the axis is open, + # the reference is among the groups it offered, and the ONLY axis the two differ on is + # one it named. A screw called wrongly where the row WAS measured meets none of them + # and stays a failure. + undetermined = set((rep.get("SPACE_GROUP_SCREW_UNDETERMINED") or "NONE").split()) + offered = [gemmi.find_spacegroup_by_name(x.strip()) + for x in (rep.get("SPACE_GROUP_ALTERNATIVES") or "").split("|")] + differ = screw_axes_differing(space_group(rep.get("SPACE_GROUP_NAME"), r["sgno"]), + space_group(ref.get("sg"), ref["sgno"])) + if (undetermined != {"NONE"} and differ and differ <= undetermined + and any(o and o.number == ref["sgno"] for o in offered)): + return verdict("unscored", "screw_undetermined", + f"{r['sg']} vs reference {r['sg_ref']}: the screw along " + f"{', '.join(sorted(differ))} is undeterminable from these data " + f"(offered {rep.get('SPACE_GROUP_ALTERNATIVES')})") return verdict("fail", "sym_screw", f"{r['sg']} vs reference {r['sg_ref']}") cause = ("sym_under" if order < order_ref else "sym_over" if order > order_ref else "sym_other")