From ec7a8261365448f428828e68b76b537e0729f1a0 Mon Sep 17 00:00:00 2001 From: Filip Leonarski Date: Mon, 27 Jul 2026 23:08:50 +0200 Subject: [PATCH] Rotation scaling: guard the fulls refit against a collapsed per-frame scale too 704098712 guarded the per-frame scale on the partials, but it put the check inside ComputeSmoothGWindow - which only the partials path calls. Step 4 refits the scale from scratch on the COMBINED FULLS (Unity model, so corr is exactly 1/G), with no smoothing and no floor, and that refit was still free to collapse toward zero. It is the same failure and it is worse here, because there is no window average to dilute it: the collapsed frame's own fulls are multiplied directly. Measured on a dataset where the previous commit had already fixed the partials stage, the fulls refit put 1/G = 559x and 175x on two frames carrying 517 observations, and the merged CC1/2 read 26.2% where the intensities ENTERING that stage were fine - better, in fact, than the comparison run's in all ten resolution shells (R_meas 34.5% vs 39.5%, CC1/2 89.1% vs 83.0%). Reject a collapsed scale here as well, on the same measured criterion, and let those fulls merge unscaled - the state the combine left them in, and the same fallback the fit already uses for a frame with too few reflections. The check reads the host fulls after both the CPU loop and the GPU ScaleFulls, so one implementation covers both paths; the corrected corr is pushed back to the device exactly as the correction surfaces already do. THIS IS NOT A DETECTION-MODE PROBLEM. Over 8 configurations (both spot finders x 4 frame ranges) the separation is exact: every run with a collapsed scale had CC1/2 <= 58.5%, every run without had CC1/2 >= 74.1%, and nothing else predicted it. On one frame range it is the DEFAULT finder that collapses (CC1/2 58.5%) while the other is clean at 93.8%. The instability was never specific to the finder; it was latent in the scaling stage and either finder could trip it. Same 8 configurations, with this commit: finder A full 93.7 -> 93.7 (untouched) finder A -s 1 58.5 -> 91.3 (recovered) finder A -e 899 94.0 -> 94.0 (untouched) finder A -e 898 87.2 -> 87.2 (untouched) finder B full 26.2 -> 91.1 (recovered) finder B -s 1 93.8 -> 93.8 (untouched) finder B -e 899 8.1 -> 91.7 (recovered) finder B -e 898 74.1 -> 74.1 (untouched) Every collapse recovers; every healthy run is unchanged. The CC1/2 spread over the four frame ranges falls from 35.5 to 6.8 points for one finder and from 85.7 to 19.7 for the other - this dataset was not sampling a deep instability when its CC1/2 swung between 17 and 94 across frame ranges, it was sampling whether this bug happened to fire. Co-Authored-By: Claude Opus 5 (1M context) --- .../scale_merge/RotationScaleMerge.cpp | 40 ++++++++++++++++++- .../scale_merge/RotationScaleMerge.h | 8 ++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/image_analysis/scale_merge/RotationScaleMerge.cpp b/image_analysis/scale_merge/RotationScaleMerge.cpp index c08cbf3b..b37f3237 100644 --- a/image_analysis/scale_merge/RotationScaleMerge.cpp +++ b/image_analysis/scale_merge/RotationScaleMerge.cpp @@ -1099,6 +1099,43 @@ void RotationScaleMerge::SmoothG(std::vector &obs, std::vector &g, g[f] = g_smooth[f]; } +bool RotationScaleMerge::RejectCollapsedFullScales() { + // The Unity model leaves corr = 1/G, and every full of a frame carries that frame's G. + std::vector g_frame(n_frames, NAN); + for (const auto &o : fulls) + if (o.frame >= 0 && o.frame < n_frames && std::isfinite(o.corr) && o.corr > 0.0f) + g_frame[o.frame] = 1.0 / static_cast(o.corr); + + std::vector fitted; + fitted.reserve(g_frame.size()); + for (const double gf : g_frame) + if (std::isfinite(gf) && gf > 0.0) + fitted.push_back(gf); + if (fitted.empty()) + return false; + const size_t mid = fitted.size() / 2; + std::nth_element(fitted.begin(), fitted.begin() + mid, fitted.end()); + const double g_floor = fitted[mid] * MIN_CREDIBLE_SCALE_RATIO; + + int n_rejected = 0; + double worst = 1.0; + for (auto &o : fulls) { + if (o.frame < 0 || o.frame >= n_frames) + continue; + const double gf = g_frame[o.frame]; + if (!std::isfinite(gf) || gf >= g_floor) + continue; + worst = std::max(worst, 1.0 / gf); + o.corr = 1.0f; // as if the frame's scale had never been fitted + ++n_rejected; + } + if (n_rejected > 0) + logger.Warning("Rejected the fitted scale of {} full(s) whose frame scaled to less than {:.0f}x " + "below the run median (worst {:.0f}x amplification); those frames merge unscaled", + n_rejected, 1.0 / MIN_CREDIBLE_SCALE_RATIO, worst / fitted[mid]); + return n_rejected > 0; +} + void RotationScaleMerge::Combine() { fulls.clear(); g_full.assign(n_frames, 1.0); @@ -2028,6 +2065,7 @@ RotationScaleMerge::Result RotationScaleMerge::Run(bool for_search, } logger.Info("Scaled fulls (XDS order, Unity model)"); } + const bool rejected_full_scales = scale_fulls && RejectCollapsedFullScales(); // --- 4b. Optional correction surfaces (decay = resolution x time; absorption = goniometer-frame // diffracted-beam direction), each an alternating multiplicative fit of the fulls' corr against @@ -2051,7 +2089,7 @@ RotationScaleMerge::Result RotationScaleMerge::Run(bool for_search, #ifdef JFJOCH_USE_CUDA // The corrections mutate the host fulls' corr; when the merge runs on the resident (GPU) fulls, push // the corrected corr back to the device so the merge reads it. - if (corrections && combined_on_gpu && scaled_fulls_on_gpu) { + if ((corrections || rejected_full_scales) && combined_on_gpu && scaled_fulls_on_gpu) { std::vector fcorr(fulls.size()); for (size_t i = 0; i < fulls.size(); ++i) fcorr[i] = fulls[i].corr; gpu_->SetFullsCorr(fcorr.data()); diff --git a/image_analysis/scale_merge/RotationScaleMerge.h b/image_analysis/scale_merge/RotationScaleMerge.h index 1ca5c5a5..2dd3be38 100644 --- a/image_analysis/scale_merge/RotationScaleMerge.h +++ b/image_analysis/scale_merge/RotationScaleMerge.h @@ -198,6 +198,14 @@ private: void Combine(); // partials -> fulls (CPU) + // Undo the scale on fulls whose frame's scale collapsed toward zero. The fulls are scaled with the + // Unity model, so their corr IS 1/G and a collapsed G multiplies every intensity on that frame + // without bound. This stage refits G from scratch with no smoothing to fall back on, so a rejected + // frame simply keeps the unscaled corr the combine gave it. Reads the host fulls, so it covers the + // CPU and GPU scaling paths alike. Returns true if anything was rejected (the caller then has to + // push the corrected corr back to the device). + bool RejectCollapsedFullScales(); + // Post-scale-fulls correction surfaces, each an alternating multiplicative fit of the host fulls' corr // against the merged reference (cheap host loops; the corrected corr is re-uploaded to the resident // fulls afterwards). Each is cross-validated (fit even frames, keep only if held-out odd equivalents