Error model: harden the fit against pathological inputs (code review)
Build Packages / build:rpm (ubuntu2404_nocuda) (push) Successful in 25m44s
Build Packages / build:rpm (rocky8_nocuda) (push) Successful in 29m14s
Build Packages / build:rpm (rocky8) (push) Successful in 30m58s
Build Packages / build:rpm (ubuntu2204_nocuda) (push) Successful in 31m34s
Build Packages / build:rpm (rocky8_sls9) (push) Successful in 31m40s
Build Packages / build:rpm (rocky9_nocuda) (push) Successful in 32m5s
Build Packages / build:rpm (rocky9_sls9) (push) Successful in 33m52s
Build Packages / XDS test (durin plugin) (push) Successful in 19m26s
Build Packages / Generate python client (push) Successful in 38s
Build Packages / Build documentation (push) Successful in 1m30s
Build Packages / Create release (push) Skipped
Build Packages / XDS test (neggia plugin) (push) Successful in 20m15s
Build Packages / XDS test (JFJoch plugin) (push) Successful in 22m14s
Build Packages / build:rpm (ubuntu2404) (push) Successful in 23m41s
Build Packages / build:rpm (ubuntu2204) (push) Successful in 25m45s
Build Packages / build:rpm (rocky9) (push) Successful in 29m38s
Build Packages / DIALS test (push) Successful in 34m2s
Build Packages / Unit tests (push) Successful in 2h19m4s

Addresses code-review findings on RefineErrorModel:
- Floor the 1/dev^2 bin weight relative to the data scale (1e-3 of the median
  bin dev^2), not an absolute 1e-30: a near-zero-scatter bin could otherwise
  acquire a runaway weight and hijack the global (a,b) fit.
- Reject a near-collinear normal-equation system relatively (det > 1e-10*Ass*AII)
  instead of with an absolute threshold that an ill-conditioned fit can pass.
- Reset the model to identity at entry so any early return leaves it inactive
  rather than keeping a stale a/b alongside a freshly-cleared mean map (which
  would make CorrectedSigma fall back to the per-observation I).
- PixelRefine: correct the orient_prior comment - with the sweep on, the LSQ
  anchor is the swept orientation (intended), not the spot-centroid one.

Verified unchanged on the lyso test set (ISa 1.1, CC1/2 90.3%).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-06-12 18:53:02 +02:00
co-authored by Claude Opus 4.8
parent e6a50b45c7
commit c93d381dc8
2 changed files with 35 additions and 7 deletions
@@ -920,8 +920,11 @@ void PixelRefine::Run(const T *image,
BuildParameterBlocks(data, beam, dist_mm, detector_rot,
latt_vec0, latt_vec1, latt_vec2);
// Anchor for orientation regularization = the spot-centroid orientation we
// started from (captured before any pixel-level refinement moved it).
// Anchor for orientation regularization = the orientation the LSQ starts from
// (captured before the predict<->refine iterations move it). When the global
// sweep ran first this is the swept orientation, not the original spot-centroid
// one - which is intended: the regularizer keeps the LSQ near its own starting
// point, it is not meant to pull a deliberate sweep back.
if (iter == 0)
for (int i = 0; i < 3; ++i)
orient_prior[i] = latt_vec0[i];
+30 -5
View File
@@ -103,6 +103,14 @@ float MergeOnTheFly::CorrectedSigma(float I_corr, float sigma_corr, uint64_t hkl
}
void MergeOnTheFly::RefineErrorModel(const std::vector<IntegrationOutcome> &outcomes) {
// Reset to identity up front: every early return below then leaves the model
// inactive (CorrectedSigma returns sigma unchanged) rather than keeping a stale
// a/b from a previous call alongside a freshly-cleared mean map.
error_model_active = false;
error_model_a = 1.0;
error_model_b = 0.0;
error_model_mean_I.clear();
// --- 1. Collect accepted, scaled observations grouped by symmetry-equivalent hkl,
// applying exactly the filters AddImage uses. ---
struct Obs { float I, sigma; };
@@ -134,7 +142,6 @@ void MergeOnTheFly::RefineErrorModel(const std::vector<IntegrationOutcome> &outc
// (b*I)^2 term uses the reflection mean, so the mean (not I_i) is the abscissa. ---
struct Sample { double s2, I2, dev2; };
std::vector<Sample> samples;
error_model_mean_I.clear();
for (const auto &[key, obs]: groups) {
if (obs.size() < 2)
@@ -176,7 +183,9 @@ void MergeOnTheFly::RefineErrorModel(const std::vector<IntegrationOutcome> &outc
return v[v.size() / 2];
};
double Ass = 0, AsI = 0, AII = 0, Bs = 0, BI = 0;
// Per-intensity-bin medians of (sigma^2, <I>^2, dev2).
std::vector<double> bs2, bI2, bd2;
bs2.reserve(n_bins); bI2.reserve(n_bins); bd2.reserve(n_bins);
const size_t per = samples.size() / n_bins;
for (int bin = 0; bin < n_bins; ++bin) {
const size_t lo = bin * per;
@@ -188,8 +197,22 @@ void MergeOnTheFly::RefineErrorModel(const std::vector<IntegrationOutcome> &outc
vI2.push_back(samples[i].I2);
vd2.push_back(samples[i].dev2);
}
const double s2 = median(vs2), I2 = median(vI2), d2 = median(vd2);
const double wgt = 1.0 / std::max(d2 * d2, 1e-30);
bs2.push_back(median(vs2));
bI2.push_back(median(vI2));
bd2.push_back(median(vd2));
}
// Relative-weighted (1/dev2^2) least squares for (a, b^2). Floor the weight's dev2 at a
// small fraction of the typical bin dev2: an absolute floor (1e-30) does not stop a
// near-zero-scatter bin from acquiring a runaway weight and hijacking the fit, so the
// floor must scale with the data. The regression target keeps the unfloored dev2.
std::vector<double> bd2_sorted = bd2;
const double dev2_floor = std::max(1e-30, 1e-3 * median(bd2_sorted));
double Ass = 0, AsI = 0, AII = 0, Bs = 0, BI = 0;
for (int bin = 0; bin < n_bins; ++bin) {
const double s2 = bs2[bin], I2 = bI2[bin], d2 = bd2[bin];
const double d2w = std::max(d2, dev2_floor);
const double wgt = 1.0 / (d2w * d2w);
Ass += wgt * s2 * s2;
AsI += wgt * s2 * I2;
AII += wgt * I2 * I2;
@@ -197,8 +220,10 @@ void MergeOnTheFly::RefineErrorModel(const std::vector<IntegrationOutcome> &outc
BI += wgt * I2 * d2;
}
// Reject a near-collinear (ill-conditioned) system *relatively*: det lies in
// [0, Ass*AII] by Cauchy-Schwarz, so compare against that scale rather than 1e-30.
const double det = Ass * AII - AsI * AsI;
if (std::fabs(det) < 1e-30)
if (!(det > 1e-10 * Ass * AII))
return;
const double a = std::clamp((Bs * AII - BI * AsI) / det, 0.25, 100.0);
const double b2 = std::max((Ass * BI - AsI * Bs) / det, 0.0);