rugnux: keep the P1 merge, so a wrong space group is recoverable

A de-novo rotation run merges in P1 to search for the symmetry, adopts a
group, and then overwrites that merge with the in-symmetry one. If the
adopted group is wrong the user has no route back: every file the run
wrote, and every statistic in them, is computed in the group that was
assumed, so nothing in the output says the choice was wrong and the only
way to a different answer is to process the images again.

Merge the same integration once more in P1 after the run's own files are
written, and put it beside them as <prefix>_P1.mtz. From it the space
group can be re-determined and the data re-merged, re-solved or
re-refined in any subgroup - measured end to end on three crystals:
POINTLESS reads the file, recovers the group, and AIMLESS re-merges it,
reproducing rugnux's own merged intensities at CC 0.9965 where the two
agree. On one of the three it recovered the deposited/XDS group where
this run had under-called the screw axis.

The merge is the full one - correction surfaces fitted, ice rings and
near-tangential observations kept, whole resolution range - not the
deliberately degraded merge the space-group search itself runs on, and
it is what `rugnux --mode scale -S P1` produces from a _process.h5. That
route already existed but needs a _process.h5, which a merging run does
not write, so it only helped a user who had foreseen the problem.

Every de-novo rotation run writes the file, including one whose search
concluded P1 and where it therefore repeats the merged output byte for
byte. Whether a file exists must not depend on what the pipeline
decided: a script harvesting results would otherwise have to reproduce
the search's decision to know whether to expect it, and a missing file
would not separate "the run chose P1" from "the run failed".

A user-fixed -S writes nothing, and that condition is not a pipeline
decision. With a group fixed, prediction rejects that group's centring
absences (IndexAndRefine.cpp:499-506), so those reflections are never
integrated; a P1 merge built from such a run would be missing whole
centring classes and would mislead rather than merely be smaller.

Cost, median of five paired runs read off the log timestamps (the box is
shared, so end-to-end wall time is noise): +0.32 s of 4.2 s, +2.18 s of
31.4 s, +0.19 s of 17.9 s, +0.66 s of 15.9 s - 1 to 8% of a run. The
file is 1.6x the merged MTZ on a monoclinic crystal and 30x on a cubic
one, where the merged MTZ is tiny; it is well under the unmerged export
in every case measured, and 0.02 to 0.6% of the raw dataset.

Rotation only for now: the stills merge re-fits per-image scales and
per-reflection partialities onto the integration outcomes, and
_unmerged.mtz is written from those afterwards, so on stills this extra
merge alters a file that is the run's own output. Fixing that means
running the cross-check below the unmerged export, which needs the merge
lambda hoisted out of its block; deferred, since rotation is what the
online pipeline processes. On rotation the merged .mtz, .cif, .hkl,
_image.dat, _unmerged.mtz and _unmerged_partials.mtz are all
byte-identical with and without this change, including on a run whose
search returns P1.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T3yNBXk4wKdMZy1ak2NY7f
This commit is contained in:
2026-08-30 12:36:49 +02:00
co-authored by Claude Opus 5
parent 8e9ca1f6d2
commit dc71cb7299
5 changed files with 86 additions and 0 deletions
+1
View File
@@ -1,6 +1,7 @@
# Changelog
## 1.0.0
### 1.0.0-rc.166
* Every rotation run that determines its own space group also writes `<prefix>_P1.mtz`, the same observations merged in P1, so a wrong space group can be re-merged, re-solved or re-refined without processing the images again; `--no-p1-crosscheck` declines it, and a run given `-S` writes nothing because its space group's centring absences were never integrated.
* `jfjoch_viewer` opens PILATUS miniCBF sweeps - naming any frame opens the whole sweep - and can run a processing job on one.
* A detector whose stored image is mirrored in Y or mounted at a multiple of 90 degrees can be described as such, in the detector configuration or with `--detector-mirror-y` / `--detector-quarter-turns`, rather than having to be expressed as a detector rotation.
* The rotation first pass refines twelve candidate lattices rather than four, so a correct cell that the pre-refinement ranking put fifth is still reached.
+10
View File
@@ -405,6 +405,16 @@ reusing them would hide the spot-finding settings from the lattice search.
asks for it, alongside the merged files and with `--no-merge` too. See
[The unmerged export](#the-unmerged-export) below. `--export-unmerged-partials` writes
`<prefix>_unmerged_partials.mtz`, one row per image, instead of summing.
- `<prefix>_P1.mtz` — the **P1 cross-check dataset**: the same observations merged in P1 instead of the
space group the run determined, so a wrong call can be re-merged, re-solved or re-refined without
processing the images again. It is a full merge, not the degraded one the search itself runs on, and
it is what `rugnux --mode scale -S P1` would make from a `_process.h5`. **Every rotation run that
determines its own space group writes it** — including one that determined P1, where it simply
repeats the merged output — so a script harvesting results can always expect the file rather than
having to reproduce the search's decision to know whether it exists. It is **not** written when `-S`
fixed the space group: prediction then rejects that group's centring absences, so those reflections
were never integrated and a P1 merge of them would be missing whole classes of reflections.
`--no-p1-crosscheck` declines it. Stills are not covered yet.
- `<prefix>_report.txt` — the **results report**: what the run determined, in a form both a person and
a beamline script can read. Always written, next to the files above. See
[The results report](#the-results-report) below.
+61
View File
@@ -3517,6 +3517,67 @@ ProcessResult Rugnux::RunPipeline(RugnuxObserver *observer, bool write_output, b
// per-image arrays are only filled on the online per-image path). Sourced from the
// partials, which carry the first-pass per-image scale.
ScalingResult(indexer->GetIntegrationOutcome()).SaveToFile(config_.output_prefix);
// P1 cross-check dataset. The group the files above are written in was chosen by the
// search, and if that choice is wrong nothing in them says so - every statistic was
// computed in the group that was assumed, and the only way back is to process the images
// again. Merging the same integration once more in P1 removes that: the user can re-merge
// it in any subgroup, run MR or refinement in it, or hand it to POINTLESS. It is a proper
// merge - correction surfaces fitted, ice rings and near-tangential observations kept -
// not the deliberately degraded one the search itself ran on, and it is the same file
// `rugnux --mode scale -S P1` makes from a _process.h5. Written after the merged output,
// so the answer of the run is already on disk in the group that was determined.
//
// Written on EVERY de-novo run, including one whose search concluded P1 and where the
// file therefore repeats the merged output. Whether a file exists must not depend on
// what the pipeline decided: a script harvesting results would otherwise have to
// reproduce the search's decision to know whether to expect it, and a missing file
// would not distinguish "the run chose P1" from "the run failed".
//
// A user-fixed space group (-S) is the one case that writes nothing, and the reason is
// not policy: with a group fixed, prediction rejects that group's centring absences and
// those reflections are never integrated (IndexAndRefine.cpp, `.centering = ...`). A P1
// merge built from such a run would be missing whole centring classes - a misleading P1
// dataset, not merely a smaller one. Do not "fix" this gap by writing the file anyway.
//
// Rotation only, for now. The stills merge re-fits each image's scale and each
// reflection's partiality onto the integration outcomes, and _unmerged.mtz is written
// from those afterwards - so on stills this merge changes a file that is the run's own
// output (measured: same length, different bytes). The fix is to run this below the
// unmerged export rather than here, which needs the merge lambda hoisted out of this
// block; deferred, as rotation is what the online pipeline processes.
if (config_.write_p1_crosscheck && search_space_group && is_rotation) {
const auto determined_sg = experiment_.GetSpaceGroupNumber();
const gemmi::SpaceGroup *determined = gemmi::find_spacegroup_by_number(
static_cast<int>(determined_sg.value_or(1)));
// scale_and_merge reports the error model of whatever it merged; this merge is not
// the run's answer, so put the determined group's model back afterwards.
const double em_isa = result.error_model_isa;
const double em_isa_asymptotic = result.error_model_isa_asymptotic;
const double em_a = result.error_model_a;
const double em_b = result.error_model_b;
// Both the merge and the MTZ read the group from the experiment, so it is set for
// the whole of it and restored after.
experiment_.SpaceGroupNumber(1);
const auto p1 = scale_and_merge("P1 cross-check", false);
const std::string path = config_.output_prefix + "_P1.mtz";
WriteMtzReflections(p1.merged, *result.consensus_cell, experiment_, path);
experiment_.SpaceGroupNumber(determined_sg);
result.error_model_isa = em_isa;
result.error_model_isa_asymptotic = em_isa_asymptotic;
result.error_model_a = em_a;
result.error_model_b = em_b;
if (determined_sg.value_or(1) > 1)
logger.Info("P1 cross-check dataset written to {} ({} unique reflections): the "
"same observations merged in P1 instead of {}, so a wrong space group "
"can be recovered from without reprocessing. It is not the result of "
"this run.", path, p1.merged.size(),
determined ? determined->short_name() : "?");
else
logger.Info("P1 cross-check dataset written to {} ({} unique reflections). This "
"run determined P1, so it repeats the merged output; it is written "
"anyway, so the file is there on every run.", path, p1.merged.size());
}
}
}
+6
View File
@@ -151,6 +151,12 @@ struct ProcessConfig {
// mmCIF, gzipped or not): compute R-free with an optimized bulk solvent and write 2Fo-Fc /
// Fo-Fc maps (--model).
std::string model_path;
// Merge the same integration a second time in P1 and write it as <prefix>_P1.mtz, so a
// space group the search got wrong stays recoverable without reprocessing (--no-p1-crosscheck
// declines it). Every de-novo rotation run writes it, including one that determined P1 itself;
// a user-fixed -S writes nothing, because its centring absences were never integrated.
bool write_p1_crosscheck = true;
};
struct ProcessResult {
+8
View File
@@ -170,6 +170,7 @@ void print_usage() {
std::cout << " --model <file> After merging, validate vs this atomic model: R-free, 2Fo-Fc/Fo-Fc maps, and - where the merge kept the Bijvoet split - an anomalous difference map with the strongest anomalous scatterers named by the atom they sit on. PDB or mmCIF, gzipped or not, recognised by content rather than by extension" << std::endl;
std::cout << " --export-unmerged Write <prefix>_unmerged.mtz, an unmerged MTZ (POINTLESS column layout) of the integrated observations, for aimless / pointless / careless. Off by default - the file is large, and a run that does not need it should not pay for it. On a rotation run the partials of each reflection are summed into one full, written at the batch its rocking curve is centred on, with the summed rocking-curve fraction in FRACTIONCALC; an event that caught less of its rocking curve than --min-partiality is not written, as in the merge. Intensities carry the Lorentz-polarization factor and nothing else: the partiality is not divided out and the per-image scale is not applied at all, since those programs scale the data themselves. Written in --mode mx and --mode scale, and with --no-merge" << std::endl;
std::cout << " --export-unmerged-partials Write <prefix>_unmerged_partials.mtz, the same observations with each partial as its own row (one batch per image) flagged for the reading program to sum, instead of summed here. Independent of --export-unmerged; both can be given" << std::endl;
std::cout << " --no-p1-crosscheck Do not write <prefix>_P1.mtz. That file is the same observations merged in P1 instead of the space group the run determined, so a wrong space group can be re-merged, re-solved or re-refined without processing the images again. Every rotation run that determines its own space group writes it - including one that determined P1, where it repeats the merged output - so it is there to be harvested whatever the run decided. Not written when -S fixed the space group: that run never integrated the group's centring absences, so its P1 merge would be missing whole classes of reflections. It is a full merge, not the degraded one the search runs on, and it costs one extra merge" << std::endl;
std::cout << std::endl;
std::cout << " Integration" << std::endl;
@@ -279,6 +280,7 @@ enum {
OPT_CALIBRANT,
OPT_CALIBRATION,
OPT_NO_MERGE,
OPT_NO_P1_CROSSCHECK,
OPT_POLARIZATION_CORRECTION,
OPT_SOLID_ANGLE_CORRECTION,
OPT_BEAM_X,
@@ -317,6 +319,7 @@ static option long_options[] = {
{"calibrant", required_argument, nullptr, OPT_CALIBRANT},
{"calibration", required_argument, nullptr, OPT_CALIBRATION},
{"no-merge", no_argument, nullptr, OPT_NO_MERGE},
{"no-p1-crosscheck", no_argument, nullptr, OPT_NO_P1_CROSSCHECK},
{"scale-fulls", no_argument, nullptr, OPT_SCALE_FULLS},
{"no-scale-fulls", no_argument, nullptr, OPT_NO_SCALE_FULLS},
{"write-process-h5", no_argument, nullptr, OPT_WRITE_PROCESS_H5},
@@ -663,6 +666,7 @@ static int RunRugnux(int argc, char **argv) {
std::optional<float> rotation_indexing_range;
std::optional<int64_t> min_indexed_spots; // --min-indexed-spots: spots on the lattice for a frame to count as indexed
bool run_scaling = true; // merge is on by default; --no-merge turns it off
bool write_p1_crosscheck = true; // <prefix>_P1.mtz is written by default; --no-p1-crosscheck declines it
std::optional<bool> scale_fulls_arg; // --scale-fulls / --no-scale-fulls; default on for rot3d
bool write_process_h5_flag = false; // --write-process-h5; also write _process.h5 when merging
std::optional<bool> detect_ice_rings; // --detect-ice-rings[=on|off]; unset => use the dataset (file) value
@@ -1043,6 +1047,9 @@ static int RunRugnux(int argc, char **argv) {
case OPT_NO_MERGE:
run_scaling = false;
break;
case OPT_NO_P1_CROSSCHECK:
write_p1_crosscheck = false;
break;
case OPT_SCALE_FULLS:
scale_fulls_arg = true;
break;
@@ -2387,6 +2394,7 @@ static int RunRugnux(int argc, char **argv) {
config.observation_dump_path = dump_observations;
config.export_unmerged = export_unmerged;
config.export_unmerged_partials = export_unmerged_partials;
config.write_p1_crosscheck = write_p1_crosscheck;
config.model_path = model_pdb;
// When merging, the merged reflections (.mtz/.cif) are the wanted output; skip the large
// _process.h5 unless explicitly requested. Without merging, the _process.h5 is the only output.