diff --git a/image_analysis/scale_merge/Merge.cpp b/image_analysis/scale_merge/Merge.cpp index 0db26709..7ad95166 100644 --- a/image_analysis/scale_merge/Merge.cpp +++ b/image_analysis/scale_merge/Merge.cpp @@ -358,8 +358,7 @@ std::vector MergeOnTheFly::ExportReflections() { .k = accum.k, .l = accum.l, .I = static_cast(accum.sum_wI / accum.sum_w), - .sigma = SigmaWithSystematicFloor(1.0 / std::sqrt(accum.sum_w), - static_cast(accum.sum_wI / accum.sum_w), error_model_b), + .sigma = static_cast(1.0 / std::sqrt(accum.sum_w)), .I_half = {NAN, NAN}, .sigma_half = {NAN, NAN}, .d = accum.d @@ -368,8 +367,7 @@ std::vector MergeOnTheFly::ExportReflections() { if (accum.n_half[0] + accum.n_half[1] > 0 && accum.sum_w_half[0] > 0.0 && accum.sum_w_half[1] > 0.0) { for (int i = 0; i < 2; ++i) { mr.I_half[i] = static_cast(accum.sum_wI_half[i] / accum.sum_w_half[i]); - mr.sigma_half[i] = SigmaWithSystematicFloor(1.0 / std::sqrt(accum.sum_w_half[i]), - mr.I_half[i], error_model_b); + mr.sigma_half[i] = static_cast(1.0 / std::sqrt(accum.sum_w_half[i])); } } diff --git a/image_analysis/scale_merge/Merge.h b/image_analysis/scale_merge/Merge.h index 464ac152..dc250a15 100644 --- a/image_analysis/scale_merge/Merge.h +++ b/image_analysis/scale_merge/Merge.h @@ -16,18 +16,6 @@ #include "HKLKey.h" -// The error model splits a reflection's variance into a statistical part (a*sigma^2, which averages -// down with multiplicity) and a systematic part ((b*I)^2 - absorption, beam flicker, partiality, -// detector non-uniformity - correlated across a reflection's repeats). Inverse-variance merging -// (sigma = 1/sqrt(sum_w)) wrongly divides BOTH by the multiplicity, so high-multiplicity reflections -// get an unphysically small merged sigma (merged I/sigma far above ISa). Floor the merged sigma at -// b*|I| so the systematic term survives the merge and ISa = 1/b stays the asymptotic I/sigma ceiling. -// error_model_b <= 0 (no active error model) leaves the sigma unchanged. -inline float SigmaWithSystematicFloor(double inv_variance_sigma, float merged_I, double error_model_b) { - const auto floor = static_cast(error_model_b * std::abs(static_cast(merged_I))); - return std::max(static_cast(inv_variance_sigma), floor); -} - struct MergeStatisticsShell { float d_min = 0.0f; float d_max = 0.0f; diff --git a/image_analysis/scale_merge/RotationScaleMerge.cpp b/image_analysis/scale_merge/RotationScaleMerge.cpp index d73ace1f..9e5d055d 100644 --- a/image_analysis/scale_merge/RotationScaleMerge.cpp +++ b/image_analysis/scale_merge/RotationScaleMerge.cpp @@ -1801,10 +1801,8 @@ RotationScaleMerge::Result RotationScaleMerge::MergeAndStats(int n_groups, bool // intensity raises its systematic term `b`, so 1/b understates that limit. Read it instead directly // from the strong equivalents: for each well-measured reflection group the counting-subtracted // fractional scatter of its symmetry mates estimates the systematic term, and the robust median over - // strong groups is the asymptote. The I/sigma threshold that selects "strong" is relaxed on weak / - // radiation-damaged data that has few strong reflections (rather than fall back to the higher - // whole-range b); it falls back only when even the relaxed set is too small. Host-side over the - // merged fulls, so CPU and GPU agree. + // strong groups is the asymptote. Report-only: it is the reported ISa, nothing downstream uses it. + // Host-side over the merged fulls, so CPU and GPU agree. double error_model_b_asymptotic = error_model_b; if (error_model_active) { struct GroupScatter { double sum = 0, sum_sq = 0, sum_var = 0; int n = 0; }; @@ -1816,47 +1814,60 @@ RotationScaleMerge::Result RotationScaleMerge::MergeAndStats(int n_groups, bool auto &g = gs[o.group]; g.sum += I_corr; g.sum_sq += I_corr * I_corr; g.sum_var += sigma_corr * sigma_corr; ++g.n; } - // The counting term to subtract. The estimator below has always used the RAW reported - // sigma^2, but the (a, b) fit immediately above concluded the counting variance is - // a*sigma^2. Where a < 1 the raw subtraction overshoots: the per-group systematic is driven - // to zero, the median lands on the boundary, and 1/b_asy reports an impossible I/sigma - // (measured: one dataset reporting 64.6 where every other statistic supports ~16, and - // flipping between 10.9 and 64.6 on consecutive merges of the same data). Opt-in for now - // (env JFJOCH_ISA_ASY_A) so the default path stays bit-identical while the two are - // batteried against each other -- an earlier attempt at this subtraction was rejected for - // over-claiming on a > 1 data, which this change would reintroduce. - const double asy_counting_scale = - std::getenv("JFJOCH_ISA_ASY_A") != nullptr ? error_model_a : 1.0; - // Per-group counting-subtracted fractional systematic variance, paired with the group's I/sigma. + // Two conventions decide whether this measures anything: + // * the counting term is the error model's OWN, a*sigma^2. Subtracting the raw sigma^2 while the + // fit has just concluded the counting variance is a*sigma^2 leaves a bias of (1-a)*sigma^2, + // which at the I/sigma admitted here is the same size as the systematic being measured - so at + // a < 1 the per-group value collapses onto zero and 1/b_asy reports an impossible I/sigma. + // The I/sigma that selects "strong" uses the same counting term, so the gate and the + // subtraction agree about what a strong reflection is. + // * the sample variance of n observations is chi^2_(n-1)-distributed, and its MEDIAN lies below + // its mean (16% at n = 5, 5% at n = 13). Taking a median across groups of a variance minus an + // unbiased counting term therefore subtracts more than it should, again by an amount + // comparable to the systematic. Rescale each group's variance to be median-unbiased first + // (Wilson-Hilferty median(chi^2_k) = k*(1-2/9k)^3, exact to 0.4% for k >= 4) - the same + // median-to-mean conversion the (a, b) fit does with CHI2_1_MEDIAN. + // The clamp at zero is gone with them: median(max(x,0)) = max(median(x),0), so it never moved a + // positive median, and a negative median is the informative answer "not measurable here". std::vector> group_scatter; // (systematic b^2, I/sigma) for (const auto &g : gs) { if (g.n < 5) continue; const double mean = g.sum / g.n; - const double counting = asy_counting_scale * (g.sum_var / g.n); + const double counting = error_model_a * (g.sum_var / g.n); if (mean <= 0.0 || counting <= 0.0) continue; - const double variance = (g.sum_sq - g.sum * g.sum / g.n) / (g.n - 1); - group_scatter.push_back({std::max(variance - counting, 0.0) / (mean * mean), + const double k = g.n - 1; + const double median_of_chi2 = std::pow(1.0 - 2.0 / (9.0 * k), 3); + const double variance = (g.sum_sq - g.sum * g.sum / g.n) / k / median_of_chi2; + group_scatter.push_back({(variance - counting) / (mean * mean), mean / std::sqrt(counting)}); } - auto asymptote_above = [&](double snr_min, size_t min_groups) -> double { + // The threshold is relaxed on weak / radiation-damaged data that has too few strong reflections + // for the tight one. A tier that HAS enough groups gives its answer and is not retried lower + // because that answer came out small - the retry is what made the report flip between 1/b and an + // absurd value on consecutive merges of statistically identical data. + auto asymptote_above = [&](double snr_min, size_t min_groups) -> std::optional { std::vector b2; for (const auto &[b2_value, snr] : group_scatter) if (snr >= snr_min) b2.push_back(b2_value); - return b2.size() >= min_groups ? std::sqrt(median_of(b2)) : 0.0; + return b2.size() >= min_groups ? std::optional(median_of(b2)) : std::nullopt; }; - double b_asy = asymptote_above(20.0, 100); // tight threshold on data that supports it - if (b_asy <= 0.0) b_asy = asymptote_above(10.0, 50); // relaxed for weak / damaged data - if (b_asy > 0.0) error_model_b_asymptotic = b_asy; - + auto b2_asy = asymptote_above(20.0, 100); // tight threshold on data that supports it + if (!b2_asy) b2_asy = asymptote_above(10.0, 50); // relaxed for weak / damaged data + // A non-positive median means the strong equivalents reproduce each other to within counting + // statistics: the asymptote is below what this data can resolve, so report the whole-range b + // rather than an extreme extrapolated from noise. The asymptote can also only ever REFINE 1/b + // upwards - that is the whole reason it is measured - so a strong-group estimate that comes out + // WORSE than the fit's own b has not measured an asymptote at all: it means "strong" was + // selected on a sigma scale that the fit itself rejects, which is what happens on data too weak + // to have strong reflections. Inert on healthy data (b_asy sits 0-40% below b there). + if (b2_asy && *b2_asy > 0.0) + error_model_b_asymptotic = std::min(std::sqrt(*b2_asy), error_model_b); } // Guard a degenerate low-multiplicity fit: with too few symmetry equivalents both the (a, b) fit and // the per-group scatter collapse toward zero, and 1/b then reports an impossibly high asymptotic // I/sigma. Real macromolecular data does not exceed ISa ~50; past a generous cap report the asymptote - // as unmeasured rather than emit a spurious extreme. Only the REPORT is dropped: the sigma floor - // below keeps using the fitted value, because a degenerate fit is precisely where the raw - // 1/sqrt(sum_w) is least trustworthy and leaving the merged sigma uncapped would be the opposite of - // what this guard is for. + // as unmeasured rather than emit a spurious extreme. constexpr double MIN_ASYMPTOTIC_B = 0.01; // ISa cap 100 const double isa_reported = error_model_b_asymptotic >= MIN_ASYMPTOTIC_B ? 1.0 / error_model_b_asymptotic : 0.0; @@ -1929,16 +1940,19 @@ RotationScaleMerge::Result RotationScaleMerge::MergeAndStats(int n_groups, bool MergedReflection mr{}; mr.h = group_h[g]; mr.k = group_k[g]; mr.l = group_l[g]; mr.I = static_cast(a.swI / a.sw); - // The systematic floor caps a high-multiplicity merged sigma at the reproducibility that the - // strongest reflections actually reach, i.e. the asymptotic term, so merged I/sigma approaches ISa. - mr.sigma = SigmaWithSystematicFloor(1.0 / std::sqrt(a.sw), mr.I, error_model_b_asymptotic); + // Plain inverse-variance merged sigma. The error model's systematic term (b*I)^2 is measured + // from the scatter BETWEEN a reflection's symmetry equivalents, i.e. from exactly the part of + // the error that is NOT common to them, so it averages down over the multiplicity like the + // counting part and the merge must not hold it back. XDS behaves the same way: its merged + // I/sigma runs far above its own reported ISa in the strong low-resolution shells. + mr.sigma = static_cast(1.0 / std::sqrt(a.sw)); mr.I_half[0] = mr.I_half[1] = NAN; mr.sigma_half[0] = mr.sigma_half[1] = NAN; mr.d = a.d; if (a.nh[0] + a.nh[1] > 0 && a.swh[0] > 0.0 && a.swh[1] > 0.0) { for (int i = 0; i < 2; ++i) { mr.I_half[i] = static_cast(a.swIh[i] / a.swh[i]); - mr.sigma_half[i] = SigmaWithSystematicFloor(1.0 / std::sqrt(a.swh[i]), mr.I_half[i], error_model_b_asymptotic); + mr.sigma_half[i] = static_cast(1.0 / std::sqrt(a.swh[i])); } } if (!std::isfinite(a.d) || a.d <= 0.0f) continue; diff --git a/image_analysis/scale_merge/SearchSpaceGroup.cpp b/image_analysis/scale_merge/SearchSpaceGroup.cpp index af484c75..69a3d661 100644 --- a/image_analysis/scale_merge/SearchSpaceGroup.cpp +++ b/image_analysis/scale_merge/SearchSpaceGroup.cpp @@ -221,14 +221,11 @@ SearchSpaceGroupResult SearchSpaceGroup( pass_absence[i] = in_range; } - // The merge floors the merged sigma at b|I| (Merge.h SigmaWithSystematicFloor) so that ISa = 1/b - // stays the asymptotic I/sigma ceiling: no reflection in the merge can read above it. A fixed - // I/sigma cut is therefore not a per-reflection test but a switch on the error model - every - // reflection at the floor reads 1/b exactly, however strong it is, and on a merge whose ISa falls - // below the cut NOTHING passes, so every operator is left with no pairs and the point group - // collapses to 1. Measured over the rotation battery: max I/sigma equals 1/b on every merge, and - // the four crystals that lose symmetry are exactly the four whose search merge sits at ISa 3.5-4.0 - // - just above this cut - and drops below 3 when the integration background changes. + // A fixed I/sigma cut is a statement about the error model as much as about a reflection: on a weak + // merge, whose error-model sigmas are large for every reflection, NOTHING passes, every operator is + // left with no pairs and the point group collapses to 1. Measured over the rotation battery, the four + // crystals that lose symmetry are exactly the four whose search merge is weakest - just above this + // cut - and that drop below it when the integration background changes. // So cap the cut at the merge's own I/sigma quantile: the correlation stage always keeps at least // its strongest quarter. This is a no-op on any merge where the fixed cut already keeps that many. constexpr double MIN_PRESENT_FRACTION = 0.25; @@ -754,14 +751,13 @@ SearchSpaceGroupResult SearchSpaceGroup( // screw-axis violation; it only relaxes "present", so it cannot over-call a screw whose // predicted-absent class carries real intensity. // - // present_cut, not the fixed cut: merged sigma is floored at b*|I| (SigmaWithSystematicFloor), - // so no reflection can read I/sigma above ISa = 1/b. On a merge whose ISa sits at or below the - // fixed cut NOTHING is ever present - screw_violations is identically zero, so every screw axis - // passes unchallenged, and present_strong is zero, so the centering rescue below switches - // itself off on exactly the weak data it exists for. Stage A already caps the cut at the - // merge's own 75th percentile; reusing it here keeps the two stages on one definition. Where - // the fixed cut is already the smaller of the two - any merge with ISa comfortably above it - - // present_cut EQUALS it and this is a no-op. + // present_cut, not the fixed cut: on a merge weak enough that nothing clears the fixed cut, + // screw_violations is identically zero, so every screw axis passes unchallenged, and + // present_strong is zero, so the centering rescue below switches itself off on exactly the + // weak data it exists for. Stage A already caps the cut at the merge's own 75th percentile; + // reusing it here keeps the two stages on one definition. Where the fixed cut is already the + // smaller of the two - any merge with a healthy I/sigma - present_cut EQUALS it and this is + // a no-op. const bool present = IoverSigma[i] > present_cut && (opt.present_e_squared <= 0.0 || Esq[i] > opt.present_e_squared); @@ -971,7 +967,7 @@ std::string SearchSpaceGroupResultToText(const SearchSpaceGroupResult& result, os << " absent/viol = reflections the group predicts absent, and how many are nonetheless present.\n" " E2 screw / E2 row = median I/(shell) of the SCREW-absent reflections and of the rest of\n" " their axial rows - a real screw leaves the first far below the second. The columns are\n" - " the centering evidence; they say little about screws (the merged sigma floor pins I/sigma).\n"; + " the centering evidence; they say little about screws.\n"; if (result.best_space_group.has_value()) { os << "Best space group: " << result.best_space_group->short_name(); diff --git a/image_analysis/scale_merge/SearchSpaceGroup.h b/image_analysis/scale_merge/SearchSpaceGroup.h index 9326ddfb..765dcd73 100644 --- a/image_analysis/scale_merge/SearchSpaceGroup.h +++ b/image_analysis/scale_merge/SearchSpaceGroup.h @@ -47,8 +47,8 @@ struct SpaceGroupCandidateScore { double present_mean_i_over_sigma = 0.0; // Screw evidence, as median E^2 = I/(shell): the reflections a screw axis extinguishes against // the rest of that same axial row. This is the comparison the screw test makes, and the only one - // that means anything for it - the I/sigma means above are dominated by the merged-sigma floor - // (sigma >= b|I| pins I/sigma at ISa), so they read the same for absent and present alike. + // that means anything for it - the I/sigma means above are dominated by the error model's + // intensity-proportional term, so they read much the same for absent and present alike. double screw_absent_median_e_squared = 0.0; double screw_row_median_e_squared = 0.0; bool consistent = false; // absent class confirmed weak (few violations) diff --git a/rugnux/rugnux_cli.cpp b/rugnux/rugnux_cli.cpp index e7a39f82..8564a81c 100644 --- a/rugnux/rugnux_cli.cpp +++ b/rugnux/rugnux_cli.cpp @@ -1382,7 +1382,7 @@ static int RunRugnux(int argc, char **argv) { merge_engine.FilterByImageCC(experiment.GetScalingSettings().GetMinCCForImage() > 0.0); // Fit the (a, b) error model from symmetry-mate scatter before merging, exactly as the full // pipeline does (Rugnux.cpp). Without this the offline --scale merge would use the identity - // model and produce much worse stills intensities (no (b*I)^2 systematic term, no sigma floor). + // model and produce much worse stills intensities (no (b*I)^2 systematic term at all). merge_engine.RefineErrorModel(reflections); if (merge_engine.ErrorModelActive()) logger.Info("Error model: a={:.3f} b={:.3f} ISa={:.1f} chi2={:.2f}", merge_engine.ErrorModelA(), diff --git a/tests/SearchSpaceGroupTwinTest.cpp b/tests/SearchSpaceGroupTwinTest.cpp index e9a46e88..46bc8814 100644 --- a/tests/SearchSpaceGroupTwinTest.cpp +++ b/tests/SearchSpaceGroupTwinTest.cpp @@ -76,8 +76,8 @@ namespace { // model's b matching the systematic scatter that is actually there. {"sigmas 1.7x too small", 1.7, 0.05, std::nullopt}, // The other way a fitted error model misses: the statistical sigmas come out somewhat too - // LARGE while b - the asymptotic I/sigma ceiling, ISa = 1/b - is fitted 3x too optimistic, so - // the systematic scatter present is 3x what the merged sigmas' floor admits. + // LARGE while b - the asymptotic per-observation I/sigma, ISa = 1/b - is fitted 3x too + // optimistic, so the systematic scatter present is 3x what the merged sigmas admit. {"ISa 3x too optimistic", 0.6, 0.02, 0.06}, }; @@ -259,8 +259,8 @@ TEST_CASE("SearchSpaceGroup on a perfect merohedral twin returns one of the two } } -// THE property this harness exists for. Multiplicity changes only the sigmas - the random part of a -// merged sigma averages down as 1/sqrt(n) while the systematic floor b*|I| does not - so it changes +// THE property this harness exists for. Multiplicity changes only the sigmas - a merged sigma averages +// down as 1/sqrt(n) while the systematic error the crystal carries does not - so it changes // how well the SAME crystal is measured, never what its symmetry is. A symmetry decision that moves // when the same crystal is merged 2x instead of 18x is a defect of the criterion, not a property of // the data. diff --git a/tests/SyntheticMergedReflections.h b/tests/SyntheticMergedReflections.h index cc34e58d..395c487a 100644 --- a/tests/SyntheticMergedReflections.h +++ b/tests/SyntheticMergedReflections.h @@ -32,12 +32,11 @@ // alpha = 0.5 the two are identical and the twin is indistinguishable from real symmetry. // Setting the true group to the SUPERgroup instead gives the untwinned high-symmetry control. // -// * the MERGE MULTIPLICITY, modelled the way the real merge behaves (Merge.h, -// SigmaWithSystematicFloor): the random part of the merged sigma averages down as -// 1/sqrt(multiplicity) while the systematic part (b*I - absorption, partiality, beam flicker; -// correlated across a reflection's repeats) does not, so the merged sigma is -// max(sigma_statistical, b*|I|). Multiplicity therefore changes the sigmas but NOT the physics, -// and no symmetry decision may depend on it. +// * the MERGE MULTIPLICITY, modelled the way the real merge behaves: the error model gives one +// observation sigma^2 = sigma_counting^2 + (b*I)^2 and the inverse-variance merge of n of them +// divides that by n, so the merged sigma is sqrt(sigma_counting^2 + (b*I)^2)/sqrt(multiplicity). +// Multiplicity therefore changes the sigmas but NOT the physics, and no symmetry decision may +// depend on it. // // * an ERROR-MODEL MISCALIBRATION - real merged sigmas come out under-estimated (~1.7x), which is // what pushes the merge's reduced chi^2 to ~3 and switches SearchSpaceGroup between its @@ -201,7 +200,7 @@ namespace jfjoch_test { // Statistical error of one observation, and of the merge of n of them. const double sigma_one = std::sqrt(i_obs + p.background_variance); // Systematic error: a property of the reflection, identical in every observation - // of it, so it survives the merge - this is what the b*|I| sigma floor models. + // of it, so it survives the merge (and the merged sigma does not know about it). const double systematic = p.true_systematic_b.value_or(p.error_model_b) * i_obs * gauss(rng); @@ -218,18 +217,18 @@ namespace jfjoch_test { const double i_half = i_obs + systematic + sigma_stat_half * gauss(rng); r.I_half[half] = static_cast(i_half); r.sigma_half[half] = static_cast( - std::max(sigma_stat_half, p.error_model_b * std::abs(i_half)) / - p.sigma_miscalibration); + std::hypot(sigma_one, p.error_model_b * i_half) / + std::sqrt(static_cast(n_half[half])) / p.sigma_miscalibration); sum_n_i += n_half[half] * i_half; } const double i_merged = sum_n_i / n_obs; - const double sigma_stat = sigma_one / std::sqrt(static_cast(n_obs)); r.I = static_cast(i_merged); - // Merge.h SigmaWithSystematicFloor, then thrown off by the error-model - // miscalibration. - r.sigma = static_cast( - std::max(sigma_stat, p.error_model_b * std::abs(i_merged)) / p.sigma_miscalibration); + // The error model on one observation, averaged down by the merge, then thrown off + // by the error-model miscalibration. + r.sigma = static_cast(std::hypot(sigma_one, p.error_model_b * i_merged) / + std::sqrt(static_cast(n_obs)) / + p.sigma_miscalibration); merged.push_back(r); }