diff --git a/common/ResolutionShells.cpp b/common/ResolutionShells.cpp index 828a64c7..cac11f04 100644 --- a/common/ResolutionShells.cpp +++ b/common/ResolutionShells.cpp @@ -23,7 +23,10 @@ ResolutionShells::ResolutionShells(float d_min, float d_max, int32_t nshells) } std::optional ResolutionShells::GetShell(float d) const { - if (d <= d_min || d > d_max) + // NaN fails every comparison, so without the explicit test it would fall through to the + // arithmetic below, where static_cast(NaN) is INT_MIN and the clamp turns it into + // shell 0 - silently binning "no resolution" as the lowest-resolution shell. + if (!std::isfinite(d) || d <= d_min || d > d_max) return {}; if (d == d_max) diff --git a/image_analysis/scale_merge/Merge.cpp b/image_analysis/scale_merge/Merge.cpp index 4a5c834c..dd7c2147 100644 --- a/image_analysis/scale_merge/Merge.cpp +++ b/image_analysis/scale_merge/Merge.cpp @@ -633,7 +633,11 @@ MergeStatistics MergeOnTheFly::MergeStats(const std::vector &m continue; const int s = *shell; if (s >= 0 && s < n_shells) { - acc[s].total_obs++; + // Multiplicity counts what the merge kept. This walk deliberately applies a wider + // filter than the merge so R_meas is computed on the same reflections either way, + // but a masked-ring reflection is not in `unique` and must not be counted against it. + if (!IsMaskedRing(r)) + acc[s].total_obs++; const auto key = generator(r).pack(); const auto mit = merged_I.find(key); if (mit != merged_I.end()) { diff --git a/image_analysis/scale_merge/RotationScaleMerge.cpp b/image_analysis/scale_merge/RotationScaleMerge.cpp index f3dd6840..34df9a66 100644 --- a/image_analysis/scale_merge/RotationScaleMerge.cpp +++ b/image_analysis/scale_merge/RotationScaleMerge.cpp @@ -2017,7 +2017,13 @@ RotationScaleMerge::Result RotationScaleMerge::MergeAndStats(int n_groups, bool if (rnusable[g] == 0) continue; const auto shell = shells.GetShell(acc[g].d); if (!shell || *shell < 0 || *shell >= n_shells) continue; - sa[*shell].total_obs += rnusable[g]; + // Count the MERGED population, not the R_meas one. The R_meas re-walk deliberately + // ignores the ring mask (and, on a search pass, the ice flag), so its count includes + // observations that never entered `unique` - which inflates the reported multiplicity + // of whatever shell they land in. acc[g].nh is what actually went into this group's + // mean, and it is zero for a masked group. (acc[g].d is NaN for such a group, so + // GetShell above already declines it; this is the same statement made where it counts.) + sa[*shell].total_obs += static_cast(acc[g].nh[0] + acc[g].nh[1]); if (std::isfinite(merged_I[g]) && rn[g] > 0) { auto &r = rmeas[g]; r.sum_abs_dev = rabsdev[g]; r.sum_I = rsumI[g]; r.n = rn[g]; r.shell = *shell; @@ -2037,7 +2043,10 @@ RotationScaleMerge::Result RotationScaleMerge::MergeAndStats(int n_groups, bool if (!std::isfinite(I_corr) || !std::isfinite(sigma_corr) || sigma_corr <= 0.0f) continue; const auto shell = shells.GetShell(o.d); if (!shell || *shell < 0 || *shell >= n_shells) continue; - sa[*shell].total_obs++; + // Only what the merge kept counts towards multiplicity - see the GPU branch above. The + // R_meas accumulation below keeps its own, wider filter. + if (usable_merge(o)) + sa[*shell].total_obs++; if (std::isfinite(merged_I[o.group])) { auto &r = rmeas[o.group]; r.sum_abs_dev += std::fabs(static_cast(I_corr) - merged_I[o.group]); diff --git a/image_analysis/scale_merge/RotationScaleMergeGPU.cu b/image_analysis/scale_merge/RotationScaleMergeGPU.cu index 676f3243..f02400fb 100644 --- a/image_analysis/scale_merge/RotationScaleMergeGPU.cu +++ b/image_analysis/scale_merge/RotationScaleMergeGPU.cu @@ -558,8 +558,10 @@ namespace { } } - // One thread per group: R_meas accumulators (sum|I_corr - merged_I|, sum_I, n) + usable count for the - // per-shell total_observations. Mirrors MergeAndStats' R_meas re-walk (looser, cell-only filter). + // One thread per group: R_meas accumulators (sum|I_corr - merged_I|, sum_I, n) + the count of + // observations this looser walk accepted. Mirrors MergeAndStats' R_meas re-walk (cell-only filter). + // That count is NOT the per-shell total_observations - it is wider than the merge, so it would + // over-report multiplicity on a masked ring; the host uses it only to skip empty groups. __global__ void MergeRmeasKernel(MergeParams p) { for (int g = blockIdx.x * blockDim.x + threadIdx.x; g < p.n_groups; g += gridDim.x * blockDim.x) { const int lo = p.gstart[g], hi = lo + p.gcount[g]; diff --git a/image_analysis/scale_merge/RotationScaleMergeGPU.h b/image_analysis/scale_merge/RotationScaleMergeGPU.h index b5c3d37a..714c44df 100644 --- a/image_analysis/scale_merge/RotationScaleMergeGPU.h +++ b/image_analysis/scale_merge/RotationScaleMergeGPU.h @@ -82,7 +82,8 @@ public: double *swh0, double *swh1, int32_t *nh0, int32_t *nh1, double *d_out, int32_t *rejected, uint8_t *rejected_obs); - // Per-group R_meas accumulators (sum|I_corr-merged_I|, sum_I, n, and the usable count for per-shell + // Per-group R_meas accumulators (sum|I_corr-merged_I|, sum_I, n, and the count this looser walk + // accepted - which the host uses only to skip empty groups, NOT as the per-shell // total_observations); merged_I is uploaded. All arrays length n_groups. void MergeRmeas(const double *merged_I, double *absdev, double *sumI, int32_t *n, int32_t *nusable);