From 1bc2ca912505dec3da165a8b6a51dfbf742e9173 Mon Sep 17 00:00:00 2001 From: Filip Leonarski Date: Thu, 6 Aug 2026 20:18:17 +0200 Subject: [PATCH] rugnux: do not re-find the rotation spots just because --detect-ice-rings was named First-pass rotation indexing reuses the spots the acquisition wrote, and does NOT re-mark them - their ice flags come from the file. So --detect-ice-rings can only invalidate those spots when it asks for something the file did not do. It was flagged as a spot-finding option, which forced reuse_rotation_spots off whenever it appeared, so naming it swapped the acquisition's spots for this program's own and moved the first-pass lattice by itself. Measured over the 37-crystal rotation battery, passing the SEMANTICALLY NULL --detect-ice-rings=on to files that already carry detect_ice_rings=1 changed the merged data on every crystal, sent one crystal's ISa from 1.66 to 0.38, and lost MyoB_13 to indexing failure outright. Both arms of any A/B on the flag therefore moved for a reason that had nothing to do with ice, which made the flag impossible to test. Re-find only when the requested value differs from the file's, and say so when it happens. A file with no key at all counts as "did not mark", which is what its stored spots show - such a dataset carries no per-spot ice flags to reuse. Verified over the battery, with the merge mask and the radial background pinned off so this is the only variable. --detect-ice-rings=on on the 36 keyed crystals: all 36 log "using the spots stored in the file", the re-finding line appears nowhere in the arm, unique counts are identical on every crystal, the largest mean |dI|/sigma is 3.4e-5 against a repeat-run floor of 9.3e-5, and not one of R_meas, CC1/2, CC1/2_hi, ISa, completeness, SigAno, d_min or space group differs anywhere. MyoB_13 indexes again. The one file carrying no key reuses under =off and re-finds under =on, as it should. --detect-ice-rings=off still re-finds, since it does differ from those files, and two crystals still fail to index there. That is not this change: a control that re-finds with ice marking ON indexes both. With the marking off, ice spots are no longer ordered last, so they consume the --max-spots budget and the first pass collapses. With the confound removed the flag can finally be measured, and on a comparison whose spot source is identical on both arms it is clearly worth having - though the win is at INDEXING rather than at the scale fit. Three crystals are saved outright (one would otherwise collapse to P1 at CC1/2 19%, one loses half its completeness and its screw axis, one loses its F-centred cubic lattice), two more only index with it on, and the remaining eight gate-fired crystals differ by well under 1% in R_meas and CC1/2 in both directions. Co-Authored-By: Claude Opus 5 (1M context) --- rugnux/rugnux_cli.cpp | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/rugnux/rugnux_cli.cpp b/rugnux/rugnux_cli.cpp index 5abd5ab3..93cc6914 100644 --- a/rugnux/rugnux_cli.cpp +++ b/rugnux/rugnux_cli.cpp @@ -849,7 +849,8 @@ static int RunRugnux(int argc, char **argv) { scale_fulls_arg = false; break; case OPT_DETECT_ICE_RINGS: - spot_finding_options_given = true; + // Deliberately NOT a spot-finding option for the purpose of re-finding the first-pass + // rotation spots - see where reuse_rotation_spots is decided below. if (optarg == nullptr || strcmp(optarg, "on") == 0) detect_ice_rings = true; else if (strcmp(optarg, "off") == 0) @@ -1591,6 +1592,27 @@ static int RunRugnux(int argc, char **argv) { else if (!dataset->file_detect_ice_rings.has_value()) experiment.DetectIceRings(rotation_indexing); + // First-pass rotation indexing reuses the spots stored in the file, and does NOT re-mark them - + // their ice flags are the ones the acquisition wrote. So --detect-ice-rings can only invalidate + // them when it asks for something the file did not do; matching the file is a no-op for that pass. + // Re-finding whenever the flag is merely present would swap the acquisition's spots for this + // program's own, which moves the first-pass lattice by itself: measured over the rotation battery, + // passing the semantically null --detect-ice-rings=on to files that already carry it changed the + // merged data on every crystal and lost one to indexing failure. That also made the flag impossible + // to test, since both arms of an A/B moved for a reason unrelated to ice. + // A file with no key at all is treated as "did not mark", which is what its stored spots show: + // such a dataset carries no per-spot ice flags to reuse. That is a statement about the SPOTS, not + // about the setting - which is why it stays value_or(false) even though a keyless rotation file + // now defaults to detecting ice. + if (detect_ice_rings.has_value() + && detect_ice_rings.value() != dataset->file_detect_ice_rings.value_or(false)) { + if (reuse_rotation_spots) + logger.Info("--detect-ice-rings={} differs from the spots stored in the file: re-finding " + "them for first-pass rotation indexing too", + detect_ice_rings.value() ? "on" : "off"); + reuse_rotation_spots = false; + } + // Scale-fulls refits the per-frame scale on the rotation combined fulls; on by default for rotation // data (where it lifts ISa substantially) and off for stills. --no-scale-fulls overrides. const bool scale_fulls = scale_fulls_arg.value_or(rotation_indexing);