From e9da892790c192a7deff4229106da0db6f2a14bc Mon Sep 17 00:00:00 2001 From: Filip Leonarski Date: Sat, 26 Sep 2026 13:48:53 +0200 Subject: [PATCH] Rugnux report: low warning thresholds, worded as prompts to check; ice apart from powder Owner decision: a warning is a prompt to check and must catch the real cases (9min) at the cost of some spurious ones. The physically motivated corrections stay (<|L|> outside its physical range is not twinning; a single sweep's indexing choice; NO_LATTICE on rotation; the no-crystal report); thresholds raised only to cut noise come back down: - SUPERCELL_POSSIBLE warns wherever the class measures and rocks (as before rc173's audit fix), worded as "check the cell", naming weak ordered intensity of a correct cell and spots of further lattice domains as the other readings. 9min (rock 4.2%) warns again. - LATTICE_TRANSLATION warns on every admitted vector (>=75% of the origin); below 90% the wording names a very strong pseudo-translation as the other reading. - PSEUDO_TRANSLATION warns on every detection; below a 20% peak it is worded as weak, check. - SWEEP_GAPS warns where the degraded ranges cover at least 1% of the sweep (the 4 sets of 77 below that had 1-2 frames, 0.4-0.6% of the sweep). - Powder rings are split between hexagonal-ice positions and the rest (MeasurePowderRings, report-only fields); ICE_RINGS and POWDER_RINGS warn separately from 5% of the spots, and ICE_RINGS also where the merge's ice gate found ice. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01D1G8gJVAy6gp1K5Dz3NE5C --- docs/CHANGELOG.md | 7 +- docs/RUGNUX_REPORT.md | 50 ++++--- image_analysis/spot_finding/SpotUtils.cpp | 8 +- image_analysis/spot_finding/SpotUtils.h | 5 + rugnux/ReportDocument.h | 3 +- rugnux/ResultReport.cpp | 162 ++++++++++++---------- tests/ResultReportTest.cpp | 53 ++++--- tests/SpotUtilsTest.cpp | 21 +++ 8 files changed, 182 insertions(+), 127 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index e91ec3229..6699b98c0 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -33,13 +33,12 @@ * Rugnux reports further lattices found in the spots the main lattice leaves over on rotation data - split-crystal and twin domains, a crystal that changes along the sweep, a related or an unrelated cell (`EXTRA_LATTICE_*` keys, a `Multiple lattices` summary line) - and warns (`MULTIPLE_LATTICES`) where domains of the crystal hold at least 10 % of the spot intensity; processing is unchanged. * Rugnux reports the mosaicity the merge used (`MOSAICITY_DEG=` with `MOSAICITY_DEG_P10=`/`_P90=` over the frames, and a summary line), as XDS's sigma_M (`REFLECTING_RANGE_E.S.D.`). * `rugnux --polarization` is documented as the polarization degree (XDS `FRACTION_OF_POLARIZATION` = (1 + p)/2); the default 0.99 suits undulators. -* Rugnux warns about a possible supercell (`SUPERCELL_POSSIBLE`) only where the half-integer class rocks at 20 % or more; weaker cases are stated on the summary's `Supercell` line. -* Rugnux warns about an undeclared lattice translation only where the Patterson reaches 90 % of the origin there (`UNDECLARED_LATTICE_TRANSLATION_PCT=`), and about a pseudo-translation only from a 20 % peak. +* Rugnux words the supercell, lattice-translation and pseudo-translation warnings as prompts to check, naming the other possible reading, and reports the Patterson height at an undeclared translation (`UNDECLARED_LATTICE_TRANSLATION_PCT=`). * Rugnux reports `TWINNING_VERDICT= NOT_READABLE`, without a warning, where <|L|> lies outside its physical range (below 0.375 or above 0.55). * Rugnux no longer reports `NO_LATTICE` on a rotation run that indexed the sweep although no single frame indexed on its own; `INDEXING_RATE=` is a `--developer` key on rotation runs. * Rugnux writes a report with `VERDICT= FAILED` and `NO_LATTICE` when a rotation run finds no crystal lattice, instead of stopping with an input-parameter error and no report. -* Rugnux adds `Powder` and `Ice` summary lines and `ICE_*` keys, and warns (`POWDER_RINGS`) where powder rings hold at least half of the spots. -* Rugnux warns about degraded sweep ranges (`SWEEP_GAPS`) only where together they cover at least 10 % of the sweep. +* Rugnux separates ice rings from other powder rings (`POWDER_ICE_*`, `POWDER_NON_ICE_SPOT_FRACTION`, `ICE_*` keys, `Powder` and `Ice` summary lines) and warns about each (`ICE_RINGS`, `POWDER_RINGS`). +* Rugnux no longer warns about degraded sweep ranges (`SWEEP_GAPS`) that together cover less than 1 % of the sweep. * Rugnux no longer warns about an indexing ambiguity on a rotation sweep, which is indexed consistently; it reports the operators (`INDEXING_AMBIGUITY_OPERATORS=`, an `Indexing choice` summary line). * Rugnux calls a further lattice more than 10° from the main one a second crystal rather than a split crystal. diff --git a/docs/RUGNUX_REPORT.md b/docs/RUGNUX_REPORT.md index 72cb0f2cd..1ef530cad 100644 --- a/docs/RUGNUX_REPORT.md +++ b/docs/RUGNUX_REPORT.md @@ -150,8 +150,10 @@ and a run that produced a textbook data set read identically for their first thr `GONIO_SCALE`, `SPINDLE_CAP`, `ANISOTROPY`, `TWINNING`, `PSEUDO_TRANSLATION`, `LATTICE_TRANSLATION`, `MODEL_HAND`, `MODEL_NOT_VALIDATED`, `REFERENCE_MISMATCH`, `CANCELLED`, `RESOLUTION_FIT`, `FLIGHT_PATH`, `GEOMETRY_NOT_CONVERGED`, `SCALING_NOT_CONVERGED`, `HARMONIC_CONTAMINATION`, - `SUPERCELL_POSSIBLE`, `MULTIPLE_LATTICES`, `POWDER_RINGS`. `NONE` when nothing fired. A code appears if and only if its - warning fired, so the flags and the `WARNING:` lines are two renderings of one list — the closed + `SUPERCELL_POSSIBLE`, `MULTIPLE_LATTICES`, `ICE_RINGS`, `POWDER_RINGS`. `NONE` when nothing fired. A code appears if and only if its + warning fired, so the flags and the `WARNING:` lines are two renderings of one list. Thresholds are + deliberately low: a warning is a prompt to check something, not a verdict, and some of them fire on + data that turn out fine — the closed type for machinery, the open sentence for a person. - Below them, one line each, the facts a reader needs before reading further: space group, cell, mosaicity, the powder and ice rings, multiple lattices, resolution, completeness, signal, anomalous @@ -295,9 +297,9 @@ experiment was useless". It is reported beside `ROTATION_REJECTED_DEG` on purpos *frames* moves when the same experiment is re-sliced, and a percentage of the *rotation* does not. The prose headline above the table states both. -A `SWEEP_GAPS` warning is given, one per range, only where the degraded ranges together cover at least -10 % of the sweep's rotation; below that they are in the table and the summary's `Sweep` line, and the -warning lines in `--developer`. One or two frames rejected on a long sweep do not change what the data are. +A `SWEEP_GAPS` warning is given, one per range, where the degraded ranges together cover at least 1 % +of the sweep's rotation; below that - a frame or two - they are in the table and the summary's `Sweep` +line, and the warning lines in `--developer`. `SWEEP_QUALITY_STATUS` distinguishes **`COMPUTED`** (the diagnostic ran; a count of 0 means the sweep was clean throughout) from **`NOT_COMPUTED`** (it did not run — no scaling and merging, or stills @@ -435,9 +437,11 @@ can be asked to trust on such a pattern; it is absent where they stay separable which is the ordinary case. `POWDER_EXCLUDED_FROM_INDEXING` says whether the run needed them left out to index at all (with `POWDER_INDEXING_D_MIN` the resolution the retried first pass used). Rings are detected far more often than they are excluded: exclusion happens only where a first pass -found no usable lattice. The summary's `Powder` line states the rings on every run that found a lattice; -a warning under the `POWDER_RINGS` flag is given only where they hold at least half the spots -(`POWDER_SPOT_FRACTION=` 0.5 or more) - where the contaminant diffracts more than the crystal. +found no usable lattice. The rings are split between those on a hexagonal-ice position +(**`POWDER_ICE_RING_COUNT=`**, **`POWDER_ICE_SPOT_FRACTION=`**) and the rest - a salt, microcrystals, another +phase (**`POWDER_NON_ICE_SPOT_FRACTION=`**); the two fractions add up to `POWDER_SPOT_FRACTION=`. The +summary's `Powder` line gives both. A warning under the `ICE_RINGS` flag is given where the ice rings hold +at least 5 % of the spots, and one under `POWDER_RINGS` where the other rings do. Hexagonal ice is also measured by the merge itself, which leaves reflections on the ice rings out of scaling (and keeps them in the merge) where it finds ice: @@ -454,8 +458,8 @@ that makes a smooth ring), **`ICE_SPOT_RATIO=`** the pile-up of found spots on t against the ice-free flanks beside them (ice in large crystallites); 1 is no ice on either. **`ICE_RINGS_DETECTED=`** is `TRUE` where either passes its gate (1.5 and 2.0), and **`ICE_REFLECTIONS_ON_RINGS_PCT=`** is then the share of the integrated reflections set aside from -scaling. The summary has an `Ice` line. Ice handling is automatic and costs the merge nothing, so it is -not a warning. The measurement is described in +scaling. The summary has an `Ice` line, and where the merge found ice an `ICE_RINGS` warning prompts a +look at the ice-ring shells (one warning line, whichever of the two measurements raised it). The measurement is described in [CPU/GPU data analysis ▸ Resolution and ice-ring handling](CPU_DATA_ANALYSIS_IMAGE.md#33-resolution-and-ice-ring-handling). ## Index-2 superstructure @@ -483,11 +487,11 @@ errors clear of zero). It is advice to check, not a finding: the same numbers co cell and from a correct cell with weak ordered intensity between its reflections - or with further lattice domains whose spots land on the half-integer positions - and which of the two a structure is decided by refinement. Process both settings - the run's cell, and `SUPERCELL_DOUBLED_CELL=` given with -`-C` - and compare them there. The summary at the top of the report carries that advice on its -`Supercell` line, and says so where further lattice domains were found. A warning under the -`SUPERCELL_POSSIBLE` flag is given only where the rocking part reaches 20 %: on a battery of rotation -data sets, correct sub-cells rocked at 2-16 %, and a real doubled cell can read anywhere in that range -too, so below 20 % nothing measured here tells the two apart. +`-C` - and compare them there. A `TRUE` is also a warning under the `SUPERCELL_POSSIBLE` flag, worded as a +prompt to check: on a battery of rotation data sets most of the crystals it named refine normally in the +sub-cell (rocking parts of 2-16 %), but a real doubled cell can read inside that range too. The summary's +`Supercell` line carries the same advice, and says where further lattice domains were found that may put +spots on the half-integer positions. ## Further lattices @@ -560,10 +564,10 @@ all, `INCONCLUSIVE` means the Patterson was measured but too few acentric reflec whether the vector it names modulates the intensities. Neither is a statement that the crystal has no pseudo-symmetry. -A warning under the `PSEUDO_TRANSLATION` flag is given where `TNCS_DETECTED= TRUE` and the Patterson -peak is at least 20 % of the origin - the height at which phenix.xtriage, too, calls a -pseudo-translation one molecular replacement must be told about. A detection below that is significant -for these data but its modulation is small; the summary calls it weak. +A warning under the `PSEUDO_TRANSLATION` flag is given wherever `TNCS_DETECTED= TRUE`. Below a Patterson +peak of 20 % of the origin (the height phenix.xtriage flags) the detection is significant but its +modulation is small; the warning and the summary call it weak and ask to check whether molecular +replacement needs it. `TRUE` requires **both** of two tests, because either alone over-calls by about a factor of two: @@ -591,10 +595,10 @@ almost gone — says the reported cell may be a supercell. pseudo-symmetry rather than as one: a translation at which the Patterson reaches at least 75 % of the origin, **`UNDECLARED_LATTICE_TRANSLATION_PCT=`**. Near 100 % the merged data are invariant under it, and a translation the data are invariant under is a lattice vector by definition — so the centring or the -cell is wrong, not the packing; an undeclared centring reads 83-102 %. A warning under the -`LATTICE_TRANSLATION` flag is given from 90 %. Between 75 and 90 % it is a pseudo-translation so strong it -is nearly a lattice translation: the warning is then under `PSEUDO_TRANSLATION`, since molecular -replacement needs it declared unless refinement shows the cell to be a supercell. It is what a centred lattice merged in P1 looks like, which +cell is wrong, not the packing; an undeclared centring reads 83-102 %. Every one is a warning under the +`LATTICE_TRANSLATION` flag, asking to check the centring and the cell; below 90 % the warning also names +the other reading, a very strong pseudo-translation that molecular replacement needs declared, which +refinement in the cell tells apart. It is what a centred lattice merged in P1 looks like, which `--mode scale` on a file with no space group produces by design. The pseudo-symmetry search continues underneath it, so a real pseudo-translation sitting under an undeclared centring is still found. diff --git a/image_analysis/spot_finding/SpotUtils.cpp b/image_analysis/spot_finding/SpotUtils.cpp index 7d345a804..a844d7033 100644 --- a/image_analysis/spot_finding/SpotUtils.cpp +++ b/image_analysis/spot_finding/SpotUtils.cpp @@ -183,7 +183,7 @@ PowderRings MeasurePowderRings(const std::vector &spot_q_recipA, float ha } // Contiguous runs of ring bins are one ring, placed at their count-weighted centre. - double excess_total = 0.0; + double excess_total = 0.0, ice_excess = 0.0; size_t run_start = nbins; const auto close_run = [&](size_t run_end) { double num = 0.0, den = 0.0; @@ -195,6 +195,10 @@ PowderRings MeasurePowderRings(const std::vector &spot_q_recipA, float ha if (den > 0.0) { out.rings_q_recipA.push_back(static_cast(num / den)); excess_total += den; + if (IsOnIceRing(static_cast(2.0 * PI * den / num), half_width_q_recipA)) { + ice_excess += den; + out.ice_ring_count++; + } } run_start = nbins; }; @@ -208,6 +212,8 @@ PowderRings MeasurePowderRings(const std::vector &spot_q_recipA, float ha close_run(nbins); out.spot_fraction = static_cast(excess_total / static_cast(total)); + out.ice_spot_fraction = static_cast(ice_excess / static_cast(total)); + out.non_ice_spot_fraction = static_cast((excess_total - ice_excess) / static_cast(total)); if (out.spot_fraction < RING_MIN_SPOT_FRACTION) return {}; // measured, and it is not a phase return out; diff --git a/image_analysis/spot_finding/SpotUtils.h b/image_analysis/spot_finding/SpotUtils.h index 090fba330..a5fd9b8f2 100644 --- a/image_analysis/spot_finding/SpotUtils.h +++ b/image_analysis/spot_finding/SpotUtils.h @@ -43,6 +43,11 @@ struct PowderRings { // The share of the pooled spots that the rings hold OVER the baseline - what the contaminant // contributes, not what happens to lie in a ring band. 0 where there is nothing. float spot_fraction = 0.0f; + // The same, split between the rings on a hexagonal-ice position (ICE_RING_RES_A, within the + // measurement's half-width) and the rest - another phase, a salt or microcrystals. Report only. + float ice_spot_fraction = 0.0f; + float non_ice_spot_fraction = 0.0f; + size_t ice_ring_count = 0; // How far the rings can be told apart, in A. A powder's rings crowd together as q grows, and past // the point where ring bins are the majority of bins the baseline is itself made of rings and a // ring no longer measures as one. Nothing finer than this can be separated from the crystal, so diff --git a/rugnux/ReportDocument.h b/rugnux/ReportDocument.h index b3d5daf61..4031ac619 100644 --- a/rugnux/ReportDocument.h +++ b/rugnux/ReportDocument.h @@ -48,7 +48,8 @@ namespace PathologyCode { constexpr const char *MULTIPLE_LATTICES = "MULTIPLE_LATTICES"; // a second lattice carries much of the spot intensity constexpr const char *GEOMETRY_NOT_CONVERGED = "GEOMETRY_NOT_CONVERGED"; // the geometry walk ran out of rounds constexpr const char *SCALING_NOT_CONVERGED = "SCALING_NOT_CONVERGED"; // the per-frame scales hit their cap still moving - constexpr const char *POWDER_RINGS = "POWDER_RINGS"; // most of the spots are on powder rings + constexpr const char *POWDER_RINGS = "POWDER_RINGS"; // non-ice powder rings hold many spots + constexpr const char *ICE_RINGS = "ICE_RINGS"; // hexagonal-ice rings hold many spots } // A closed type plus an open description. The type is what machinery switches on; the text is what a diff --git a/rugnux/ResultReport.cpp b/rugnux/ResultReport.cpp index 0f8a4d002..189295c7b 100644 --- a/rugnux/ResultReport.cpp +++ b/rugnux/ResultReport.cpp @@ -39,7 +39,8 @@ namespace { // R_WORK and R_FREE are unchanged and still describe the model the maps were made from. // 15: TWINNING_VERDICT gains NOT_READABLE (<|L|> outside its physical range); ICE_* keys, // UNDECLARED_LATTICE_TRANSLATION_PCT and INDEXING_AMBIGUITY_OPERATORS; INDEXING_RATE is a - // --developer key on rotation runs; the POWDER_RINGS flag. + // --developer key on rotation runs; POWDER_ICE_* / POWDER_NON_ICE_SPOT_FRACTION; + // the ICE_RINGS and POWDER_RINGS flags. // 8: every line that is not `KEY= value` and not blank starts with `#`, warnings included // (`# WARNING:`); the header gains RUGNUX_DOWNLOAD and BUILD_CXX_FLAGS, and RUGNUX_GIT is // the full build-time hash. @@ -62,26 +63,21 @@ namespace { return sc.i_over_sigma >= 0.5 && sc.rock_pct >= 2.0 && sc.rock_pct > 3.0 * sc.rock_se_pct; } - // ...and a warning only where the rocking part is larger than any correct sub-cell showed. On the - // rc173 battery SUPERCELL_POSSIBLE named 15 rotation sets: 13 refine normally in the sub-cell and - // rock at 2.1-16.2%, where twin or split domains often put spots on the half-integer nodes; the - // two accepted doubled cells rock at 4.2% and 29.1%. Nothing measured here separates the first of - // those from the sub-cells, so it stays a summary line; only a rocking part no sub-cell reached - // is a condition. - constexpr double SUPERCELL_WARNING_ROCK_PCT = 20.0; + // A warning is a prompt to check, so it fires wherever SupercellPossible does - including on correct + // sub-cells with weak ordered intensity, or with further lattice domains whose spots land on the + // half-integer nodes (on the rc173 battery 13 of the 15 it names refine normally in the sub-cell, at + // rocking parts of 2.1-16.2%; the two accepted doubled cells read 4.2% and 29.1%, and nothing measured + // here separates the first from the sub-cells). The wording says which reading it may be. + // Every translation the analysis admits (from 75% of the origin) is warned about. An undeclared + // centring reads 83-102% (TranslationalNCS.cpp); on the rc173 battery the only two admitted vectors + // read 79% and 85%, on crystals that refine normally in the cell as it stands - so below this the + // wording names a very strong pseudo-translation as the other reading, and refinement decides. + constexpr double LATTICE_TRANSLATION_WORDING_PCT = 90.0; - // A translation the Patterson puts at this share of the origin is a lattice translation. The - // analysis admits a vector from 75%, and an undeclared centring reads 83-102% (TranslationalNCS.cpp); - // on the rc173 battery the only two admitted vectors read 79% and 85%, on crystals that refine - // normally in the cell as it stands. Below this it is reported as a pseudo-translation so strong it - // is nearly a lattice translation, and refinement is what tells the two apart. - constexpr double LATTICE_TRANSLATION_WARNING_PCT = 90.0; - - // A pseudo-translation below this Patterson peak is real where TNCS_DETECTED says so, but not what - // a molecular-replacement program has to be told about: its intensity modulation is small. The - // convention phenix.xtriage uses (a peak above 20% of the origin). On the rc173 battery the - // detected peaks are 8.1% on one set and 24.8-63.2% on the other 21. - constexpr double PSEUDO_TRANSLATION_WARNING_PCT = 20.0; + // Every detection is warned about; below this Patterson peak the wording says the modulation is small + // and may not matter to molecular replacement (phenix.xtriage flags peaks above 20% of the origin). + // On the rc173 battery the detected peaks are 8.1% on one set and 24.8-63.2% on the other 21. + constexpr double PSEUDO_TRANSLATION_WORDING_PCT = 20.0; // The physical range of <|L|>: 0.5 untwinned, 0.375 a perfect twin, whatever the twin law. A // reading outside it is not a twin fraction - tNCS, anisotropy, overlapping reflections of a long @@ -91,15 +87,15 @@ namespace { return mean_abs_l < 0.375 || mean_abs_l > 0.55; } - // Powder rings holding at least this share of the pre-scan's spots are a condition; fewer are - // stated in the summary only. POWDER_RINGS_DETECTED is TRUE on 63 of the rc173 battery's 235 sets, - // 35 of them at 0.2 or more and 13 at 0.5 or more - where the rings hold more spots than the crystal. - constexpr double POWDER_WARNING_SPOT_FRACTION = 0.5; + // Rings holding at least this share of the pre-scan's spots are warned about - hexagonal-ice rings + // (ICE_RINGS) and other rings (POWDER_RINGS) separately. Low on purpose: a warning is a prompt to + // look at the pattern. The detection itself already needs 5% of the spots in rings. + constexpr double RING_WARNING_SPOT_FRACTION = 0.05; - // Degraded ranges that together cover less than this share of the sweep are listed in the table and - // the summary but not warned about: one or two frames rejected, or a short weak stretch, does not - // change what the data are. On the rc173 battery the warning fired on 77 of 235 sets; this keeps 48. - constexpr double SWEEP_GAPS_WARNING_FRACTION = 0.10; + // Degraded ranges that together cover less than this share of the sweep - a frame or two - are listed + // in the table and the summary but not warned about. On the rc173 battery 4 of the 77 sets the warning + // fired on were below it (0.4-0.6% of the sweep). + constexpr double SWEEP_GAPS_WARNING_FRACTION = 0.01; // Whether <|L|> - on the merge reported, or on the one the space-group search was given - reads // outside its physical range. Then no twin verdict is read from it, in either direction. @@ -1228,13 +1224,15 @@ ReportDocument BuildReportDocument(const std::string &output_prefix, Add(s, KeyText("SUPERCELL_DOUBLED_CELL", CellString(sc.doubled_cell))); const bool possible = SupercellPossible(sc); Add(s, KeyBool("SUPERCELL_POSSIBLE", possible)); - if (possible && sc.rock_pct >= SUPERCELL_WARNING_ROCK_PCT) + const bool domains = result.leftover_lattices && DomainIntensityPct(*result.leftover_lattices) >= 5.0; + if (possible) Warn(doc, PathologyCode::SUPERCELL_POSSIBLE, - fmt::format("Half-integer reflections of parity {} {} {} carry Bragg-like intensity ({:.0f}% " - "of the lattice's, rocking part {:.1f}%): the true cell may be twice as large. " - "Check carefully, preferably by processing both settings - this cell and " - "-C \"{:.2f},{:.2f},{:.2f},{:.2f},{:.2f},{:.2f}\" - and comparing them in refinement", + fmt::format("Check the cell: half-integer reflections of parity {} {} {} carry Bragg-like intensity " + "({:.0f}% of the lattice's, rocking part {:.1f}%), so the true cell may be twice as " + "large - or this is weak ordered intensity of a correct cell{}. Process both settings - " + "this cell and -C \"{:.2f},{:.2f},{:.2f},{:.2f},{:.2f},{:.2f}\" - and compare them in refinement", sc.h, sc.k, sc.l, sc.occupancy_pct, sc.rock_pct, + domains ? ", or spots of the further lattice domains found landing on those positions" : "", sc.doubled_cell.a, sc.doubled_cell.b, sc.doubled_cell.c, sc.doubled_cell.alpha, sc.doubled_cell.beta, sc.doubled_cell.gamma)); Add(s, Blank()); @@ -1246,10 +1244,10 @@ ReportDocument BuildReportDocument(const std::string &output_prefix, " empty. One that is occupied and rocks is an index-2 superstructure the lattice does not\n" " index: its reflections are not integrated. To process on the doubled lattice, give\n" " SUPERCELL_DOUBLED_CELL with -C. An occupied class that does not rock is diffuse or\n" - " disordered intensity rather than Bragg reflections. SUPERCELL_POSSIBLE is TRUE where\n" - " the class is measured and rocks; it is a warning only where the rocking part reaches\n" - " 20%, since correct cells with weak ordered intensity, or with further lattice domains\n" - " whose spots land on the half-integer positions, read up to about 16%.")); + " disordered intensity rather than Bragg reflections. SUPERCELL_POSSIBLE is TRUE, and a\n" + " warning to check, where the class is measured and rocks; correct cells with weak ordered\n" + " intensity, or with further lattice domains whose spots land on the half-integer\n" + " positions, read the same, so only refinement of both cells decides.")); Add(s, Blank()); } @@ -1322,6 +1320,9 @@ ReportDocument BuildReportDocument(const std::string &output_prefix, if (!pw.rings_q_recipA.empty()) { Add(s, KeyInt("POWDER_RING_COUNT", static_cast(pw.rings_q_recipA.size()))); Add(s, KeyReal("POWDER_SPOT_FRACTION", pw.spot_fraction, "{:.3f}")); + Add(s, KeyInt("POWDER_ICE_RING_COUNT", static_cast(pw.ice_ring_count))); + Add(s, KeyReal("POWDER_ICE_SPOT_FRACTION", pw.ice_spot_fraction, "{:.3f}")); + Add(s, KeyReal("POWDER_NON_ICE_SPOT_FRACTION", pw.non_ice_spot_fraction, "{:.3f}")); if (pw.resolved_to_d_A) Add(s, KeyReal("POWDER_RINGS_SEPARABLE_TO", *pw.resolved_to_d_A, "{:.2f}")); std::string ds; @@ -1333,12 +1334,17 @@ ReportDocument BuildReportDocument(const std::string &output_prefix, {"TRUE", "FALSE"})); if (result.powder_indexing_d_min_A) Add(s, KeyReal("POWDER_INDEXING_D_MIN", *result.powder_indexing_d_min_A, "{:.2f}")); - if (pw.spot_fraction >= POWDER_WARNING_SPOT_FRACTION) + if (pw.ice_spot_fraction >= RING_WARNING_SPOT_FRACTION) + Warn(doc, PathologyCode::ICE_RINGS, fmt::format( + "Ice rings: {} rings on hexagonal-ice positions hold {:.0f}% of the spots found - check " + "the ice-ring shells; reflections of the crystal that fall on a ring carry its intensity too", + pw.ice_ring_count, 100.0f * pw.ice_spot_fraction)); + if (pw.non_ice_spot_fraction >= RING_WARNING_SPOT_FRACTION) Warn(doc, PathologyCode::POWDER_RINGS, fmt::format( - "Powder rings hold {:.0f}% of the spots found ({} rings): a crystalline phase other " - "than the crystal diffracts more than the crystal does. Reflections of the crystal " - "that fall on a ring carry its intensity too", - 100.0f * pw.spot_fraction, pw.rings_q_recipA.size())); + "Powder rings that are not ice: {} rings hold {:.0f}% of the spots found - a crystalline " + "phase other than the crystal and ice (a salt, microcrystals); check the pattern. " + "Reflections of the crystal that fall on a ring carry its intensity too", + pw.rings_q_recipA.size() - pw.ice_ring_count, 100.0f * pw.non_ice_spot_fraction)); Add(s, Blank()); Add(s, Prose(fmt::format( " Powder contamination: {} rings, holding {:.0f}% of the spots the pre-scan found\n" @@ -1369,6 +1375,14 @@ ReportDocument BuildReportDocument(const std::string &output_prefix, " hexagonal-ice rings were left out of scaling and kept in the merge.", 100.0 * result.ice_flagged_fraction) : std::string(" Ice rings: none detected, so no reflection was set aside for them."))); + // The merge's own ice gate is a second route to the same prompt; one warning line is enough. + const bool ice_warned = std::any_of(doc.warnings.begin(), doc.warnings.end(), + [](const ReportWarning &w) { return w.code == std::string(PathologyCode::ICE_RINGS); }); + if (result.ice_rings_detected && !ice_warned) + Warn(doc, PathologyCode::ICE_RINGS, fmt::format( + "Ice rings: the merge found hexagonal ice, and the {:.1f}% of the reflections on its rings were " + "left out of scaling and kept in the merge - check the ice-ring shells", + 100.0 * result.ice_flagged_fraction)); Add(s, Prose(" ICE_RING_SCORE is the strongest ice ring over the smooth radial background (smooth\n" " ice), ICE_SPOT_RATIO the pile-up of found spots on the ring positions against the\n" " flanks beside them (ice in large crystallites); 1 is no ice on either.", true)); @@ -1414,7 +1428,7 @@ ReportDocument BuildReportDocument(const std::string &output_prefix, const bool undeclared = !t.undeclared_lattice_translations.empty(); const double undeclared_pct = t.undeclared_lattice_translation_pct.empty() ? 100.0 : t.undeclared_lattice_translation_pct.front(); - const bool lattice_translation = undeclared && undeclared_pct >= LATTICE_TRANSLATION_WARNING_PCT; + const bool lattice_translation = undeclared && undeclared_pct >= LATTICE_TRANSLATION_WORDING_PCT; Add(s, KeyReal("TNCS_PATTERSON_PEAK_PCT", t.peak_percent, "{:.1f}")); Add(s, KeyReal("TNCS_PATTERSON_PEAK_Z", t.peak_z, "{:.1f}")); Add(s, KeyText("TNCS_PATTERSON_NULL_PCT", @@ -1488,11 +1502,11 @@ ReportDocument BuildReportDocument(const std::string &output_prefix, " The pseudo-symmetry numbers above were measured underneath it.", v[0], v[1], v[2], undeclared_pct))); } - if (t.detected && t.peak_percent < PSEUDO_TRANSLATION_WARNING_PCT) + if (t.detected && t.peak_percent < PSEUDO_TRANSLATION_WORDING_PCT) Add(s, ProseDefaultOnly(fmt::format( "\n The peak is significant but weak: below the {:.0f}% of the origin at which a\n" " pseudo-translation is conventionally declared to molecular replacement.", - PSEUDO_TRANSLATION_WARNING_PCT))); + PSEUDO_TRANSLATION_WORDING_PCT))); Add(s, Prose("\n" + TranslationalNCSToText(t), true)); if (t.modulation_measured && !t.detected) @@ -1504,29 +1518,27 @@ ReportDocument BuildReportDocument(const std::string &output_prefix, " intensities. Either test alone over-calls by about a factor of two on a corpus of merged\n" " datasets.", true)); - if (t.detected && t.peak_percent >= PSEUDO_TRANSLATION_WARNING_PCT) + if (t.detected) Warn(doc, PathologyCode::PSEUDO_TRANSLATION, fmt::format( "Translational pseudo-symmetry: Patterson off-origin peak {:.1f}% of the origin at " - "({:.3f}, {:.3f}, {:.3f}), {:.1f} A - the molecular-replacement program needs the " - "vector declared, or it will be searching against a modulation it does not model", + "({:.3f}, {:.3f}, {:.3f}), {:.1f} A - {}", t.peak_percent, t.vector_frac[0], t.vector_frac[1], t.vector_frac[2], - t.vector_length_A)); - if (lattice_translation) + t.vector_length_A, + t.peak_percent >= PSEUDO_TRANSLATION_WORDING_PCT + ? "the molecular-replacement program needs the vector declared, or it will be " + "searching against a modulation it does not model" + : "significant but weak; check whether molecular replacement needs it declared")); + if (undeclared) Warn(doc, PathologyCode::LATTICE_TRANSLATION, fmt::format( - "The merged data are invariant under the translation ({:.3f}, {:.3f}, {:.3f}) - the " - "Patterson there is {:.0f}% of the origin - which the space group in use does not " - "declare: the lattice centring or the unit cell is wrong", + "Check the centring and the cell: the Patterson at ({:.3f}, {:.3f}, {:.3f}) is {:.0f}% of " + "the origin, a translation the space group in use does not declare - {}", t.undeclared_lattice_translations[0][0], t.undeclared_lattice_translations[0][1], - t.undeclared_lattice_translations[0][2], undeclared_pct)); - else if (undeclared) - Warn(doc, PathologyCode::PSEUDO_TRANSLATION, fmt::format( - "Very strong translational pseudo-symmetry: the Patterson at ({:.3f}, {:.3f}, {:.3f}) is " - "{:.0f}% of the origin - the molecular-replacement program needs the vector declared, " - "unless refinement shows the cell to be a supercell with this vector as a lattice translation", - t.undeclared_lattice_translations[0][0], - t.undeclared_lattice_translations[0][1], - t.undeclared_lattice_translations[0][2], undeclared_pct)); + t.undeclared_lattice_translations[0][2], undeclared_pct, + lattice_translation + ? "the data are invariant under it, so the lattice centring or the unit cell is wrong" + : "a lattice translation (centring or cell wrong), or a very strong pseudo-translation " + "that molecular replacement needs declared; refinement in this cell tells them apart")); if (t.detected && t.near_extinct_class) Warn(doc, PathologyCode::LATTICE_TRANSLATION, fmt::format( "The pseudo-translation at {:.1f} A suppresses its phase class almost completely, so the " @@ -2270,8 +2282,10 @@ ReportDocument BuildReportDocument(const std::string &output_prefix, if (!result.powder.rings_q_recipA.empty() || result.consensus_cell) { const auto &pw = result.powder; row("Powder", pw.rings_q_recipA.empty() ? std::string("no rings detected") - : fmt::format("{} rings holding {:.0f}% of the spots found{}", pw.rings_q_recipA.size(), - 100.0f * pw.spot_fraction, + : fmt::format("{} rings holding {:.0f}% of the spots found - {} ice ({:.0f}%), {} other " + "({:.0f}%){}", pw.rings_q_recipA.size(), 100.0f * pw.spot_fraction, + pw.ice_ring_count, 100.0f * pw.ice_spot_fraction, + pw.rings_q_recipA.size() - pw.ice_ring_count, 100.0f * pw.non_ice_spot_fraction, result.powder_excluded_from_indexing ? "; left out of indexing" : "")); } if (result.ice_ring_score || result.ice_spot_ratio) @@ -2351,16 +2365,16 @@ ReportDocument BuildReportDocument(const std::string &output_prefix, const double pct = t.undeclared_lattice_translation_pct.empty() ? 100.0 : t.undeclared_lattice_translation_pct.front(); ps = fmt::format("{} ({:.3f} {:.3f} {:.3f}, Patterson {:.0f}% of origin)", - pct >= LATTICE_TRANSLATION_WARNING_PCT - ? "undeclared LATTICE translation" - : "VERY STRONG, nearly a lattice translation", + pct >= LATTICE_TRANSLATION_WORDING_PCT + ? "undeclared LATTICE translation - check centring and cell" + : "check: nearly a lattice translation, or a very strong pseudo-translation", v[0], v[1], v[2], pct); } if (t.detected) ps += (ps.empty() ? "" : "; ") + fmt::format( "{} (Patterson peak {:.1f}% of origin, {:.1f} A)", - t.peak_percent >= PSEUDO_TRANSLATION_WARNING_PCT - ? "INDICATED" : "weak - below the 20% that matters for molecular replacement", + t.peak_percent >= PSEUDO_TRANSLATION_WORDING_PCT + ? "INDICATED" : "INDICATED, weak - check whether molecular replacement needs it", t.peak_percent, t.vector_length_A); row("Pseudo-symmetry", ps.empty() ? std::string("no indication") : ps); } @@ -2376,14 +2390,10 @@ ReportDocument BuildReportDocument(const std::string &output_prefix, const bool domains = result.leftover_lattices && DomainIntensityPct(*result.leftover_lattices) >= 5.0; if (!SupercellPossible(sc)) row("Supercell", "no indication"); - else if (sc.rock_pct >= SUPERCELL_WARNING_ROCK_PCT) - row("Supercell", fmt::format("POSSIBLE - class {} {} {} rocks like Bragg reflections " - "({:.1f}%); {}", sc.h, sc.k, sc.l, sc.rock_pct, doubled)); else - row("Supercell", fmt::format("weak Bragg-like intensity in class {} {} {} (rocking part " - "{:.1f}%) - as correct cells with weak ordered intensity " - "show{}; to test a doubled cell, {}", sc.h, sc.k, sc.l, - sc.rock_pct, + row("Supercell", fmt::format("POSSIBLE, check - class {} {} {} rocks like Bragg reflections " + "(rocking part {:.1f}%); correct cells with weak ordered intensity " + "read the same{}; {}", sc.h, sc.k, sc.l, sc.rock_pct, domains ? ", and the further lattice domains found may put " "spots there" : "", doubled)); diff --git a/tests/ResultReportTest.cpp b/tests/ResultReportTest.cpp index 9fd4b3475..b5f26dd21 100644 --- a/tests/ResultReportTest.cpp +++ b/tests/ResultReportTest.cpp @@ -824,23 +824,22 @@ TEST_CASE("ResultReport_TranslationalNCS", "[Diagnostics]") { CHECK(lat.find("\nPATHOLOGY_FLAGS= LATTICE_TRANSLATION\n") != std::string::npos); CHECK(lat.find("\nTNCS_SUBLATTICE=") == std::string::npos); - // Admitted at 75% but short of a lattice translation's height: the claim is a very strong - // pseudo-translation, which is what molecular replacement has to be told. + // Admitted at 75% but short of a lattice translation's height: still a prompt to check, and the + // wording names the very strong pseudo-translation as the other reading. result.tncs.undeclared_lattice_translation_pct = {80.0}; const auto strong = RenderResultReport("prefix", "in.h5", x, result); - CHECK(strong.find("\nPATHOLOGY_FLAGS= PSEUDO_TRANSLATION\n") != std::string::npos); - CHECK(strong.find("VERY STRONG, nearly a lattice translation") != std::string::npos); - CHECK(strong.find("invariant under the translation") == std::string::npos); + CHECK(strong.find("\nPATHOLOGY_FLAGS= LATTICE_TRANSLATION\n") != std::string::npos); + CHECK(strong.find("or a very strong pseudo-translation") != std::string::npos); - // Significant, but a peak this weak is not one molecular replacement needs declared. + // Significant but weak: warned about, and worded as a check. result.tncs.undeclared_lattice_translations.clear(); result.tncs.undeclared_lattice_translation_pct.clear(); result.tncs.detected = true; result.tncs.peak_percent = 8.1; const auto weak = RenderResultReport("prefix", "in.h5", x, result); CHECK(weak.find("\nTNCS_DETECTED= TRUE\n") != std::string::npos); - CHECK(weak.find("PSEUDO_TRANSLATION") == std::string::npos); - CHECK(weak.find("weak - below the 20% that matters for molecular replacement") != std::string::npos); + CHECK(weak.find("\nPATHOLOGY_FLAGS= PSEUDO_TRANSLATION\n") != std::string::npos); + CHECK(weak.find("significant but weak; check whether molecular replacement needs it declared") != std::string::npos); } TEST_CASE("ResultReport_GeometryNotConverged", "[Diagnostics]") { @@ -982,7 +981,7 @@ TEST_CASE("ResultReport_Mosaicity", "[Diagnostics]") { CHECK(text.find("Mosaicity 0.091 deg") != std::string::npos); } -TEST_CASE("ResultReport_SupercellDemoted", "[Diagnostics]") { +TEST_CASE("ResultReport_SupercellCheck", "[Diagnostics]") { DiffractionExperiment x(DetJF(1)); ProcessResult result; result.consensus_cell = UnitCell{.a = 50.0f, .b = 60.0f, .c = 70.0f, @@ -997,11 +996,12 @@ TEST_CASE("ResultReport_SupercellDemoted", "[Diagnostics]") { sc.doubled_cell = UnitCell{.a = 50.0f, .b = 60.0f, .c = 140.0f, .alpha = 90.0f, .beta = 90.0f, .gamma = 90.0f}; result.supercell = sc; - // Possible, as correct sub-cells also read: stated, not warned about. + // A weak rocking part, as a real doubled cell can show: a prompt to check, worded as one. auto text = RenderResultReport("p", "in.h5", x, result); CHECK(text.find("\nSUPERCELL_POSSIBLE= TRUE\n") != std::string::npos); - CHECK(text.find("\nPATHOLOGY_FLAGS= NONE\n") != std::string::npos); - CHECK(text.find("weak Bragg-like intensity in class 0 0 1") != std::string::npos); + CHECK(text.find("\nPATHOLOGY_FLAGS= SUPERCELL_POSSIBLE\n") != std::string::npos); + CHECK(text.find("POSSIBLE, check - class 0 0 1") != std::string::npos); + CHECK(text.find("WARNING: Check the cell") != std::string::npos); CHECK(text.find("may put spots there") == std::string::npos); // Further domains of the crystal can put spots on the half-integer nodes, and the line says so. @@ -1013,12 +1013,13 @@ TEST_CASE("ResultReport_SupercellDemoted", "[Diagnostics]") { result.leftover_lattices = census; text = RenderResultReport("p", "in.h5", x, result); CHECK(text.find("the further lattice domains found may put spots there") != std::string::npos); + CHECK(text.find("spots of the further lattice domains found landing on those positions") != std::string::npos); - // A rocking part no correct sub-cell reached is a condition. - result.supercell->rock_pct = 29.0; + // Not measured to rock: no indication. + result.supercell->rock_pct = 1.0; text = RenderResultReport("p", "in.h5", x, result); - CHECK(text.find("\nPATHOLOGY_FLAGS= SUPERCELL_POSSIBLE\n") != std::string::npos); - CHECK(text.find("POSSIBLE - class 0 0 1 rocks like Bragg reflections (29.0%)") != std::string::npos); + CHECK(text.find("Supercell no indication") != std::string::npos); + CHECK(text.find("\nPATHOLOGY_FLAGS= NONE\n") != std::string::npos); } TEST_CASE("ResultReport_TwinningOutsidePhysicalRange", "[Diagnostics]") { @@ -1099,23 +1100,31 @@ TEST_CASE("ResultReport_PowderAndIce", "[Diagnostics]") { CHECK(text.find("ICE_RINGS_DETECTED") == std::string::npos); result.powder.rings_q_recipA = {1.61f, 1.71f, 1.82f}; - result.powder.spot_fraction = 0.20f; + result.powder.spot_fraction = 0.04f; + result.powder.ice_ring_count = 2; + result.powder.ice_spot_fraction = 0.03f; + result.powder.non_ice_spot_fraction = 0.01f; result.ice_ring_score = 2.69f; result.ice_spot_ratio = 1.08f; result.ice_rings_detected = true; result.ice_flagged_fraction = 0.257; text = RenderResultReport("p", "in.h5", x, result); - CHECK(text.find("Powder 3 rings holding 20% of the spots found") != std::string::npos); + CHECK(text.find("Powder 3 rings holding 4% of the spots found - 2 ice (3%), 1 other (1%)") != std::string::npos); CHECK(text.find("\nICE_RINGS_DETECTED= TRUE\n") != std::string::npos); CHECK(text.find("\nICE_REFLECTIONS_ON_RINGS_PCT= 25.7\n") != std::string::npos); CHECK(text.find("Ice rings DETECTED: the 25.7% of the reflections") != std::string::npos); - CHECK(text.find("POWDER_RINGS ") == std::string::npos); - CHECK(text.find("\nPATHOLOGY_FLAGS= NONE\n") != std::string::npos); + // The merge found ice: a prompt to check the ice shells, and not a powder warning. + CHECK(text.find("\nPATHOLOGY_FLAGS= ICE_RINGS\n") != std::string::npos); - // Most of the spots on rings is a condition. - result.powder.spot_fraction = 0.74f; + // Rings that are not ice are a separate prompt, in their own words. + result.ice_rings_detected = false; + result.powder.non_ice_spot_fraction = 0.20f; text = RenderResultReport("p", "in.h5", x, result); CHECK(text.find("\nPATHOLOGY_FLAGS= POWDER_RINGS\n") != std::string::npos); + CHECK(text.find("Powder rings that are not ice: 1 rings hold 20%") != std::string::npos); + result.powder.ice_spot_fraction = 0.30f; + text = RenderResultReport("p", "in.h5", x, result); + CHECK(text.find("\nPATHOLOGY_FLAGS= ICE_RINGS POWDER_RINGS\n") != std::string::npos); } TEST_CASE("ResultReport_SweepGapsMaterial", "[Diagnostics]") { diff --git a/tests/SpotUtilsTest.cpp b/tests/SpotUtilsTest.cpp index db3fb59d0..f408e35ae 100644 --- a/tests/SpotUtilsTest.cpp +++ b/tests/SpotUtilsTest.cpp @@ -144,3 +144,24 @@ TEST_CASE("MaxSpotCount_InputFloor") { REQUIRE_NOTHROW(s.MaxSpotCount(MIN_SPOT_COUNT)); REQUIRE(s.GetMaxSpotCount() == MIN_SPOT_COUNT); } + +// The ring share is split between rings on hexagonal-ice positions and the rest, so a report can say +// which it is. +TEST_CASE("MeasurePowderRings_SplitsIceFromOtherRings", "[SpotUtils]") { + const float half_width = 0.01f; + std::vector q; + // A smooth spot density over 0.5-3.0 1/A... + for (int i = 0; i < 5000; ++i) + q.push_back(0.5f + 2.5f * static_cast(i) / 5000.0f); + // ...one ring at the 3.661 A ice line, and one at 5.000 A, which is not ice. + for (int i = 0; i < 400; ++i) { + q.push_back(6.283185307f / 3.661f); + q.push_back(6.283185307f / 5.000f); + } + const auto pw = MeasurePowderRings(q, half_width); + REQUIRE(pw.rings_q_recipA.size() == 2); + CHECK(pw.ice_ring_count == 1); + CHECK(pw.ice_spot_fraction > 0.05f); + CHECK(pw.non_ice_spot_fraction > 0.05f); + CHECK(pw.ice_spot_fraction + pw.non_ice_spot_fraction == Catch::Approx(pw.spot_fraction)); +}