From 164f15c90336eb2bb61e8f6dea990113237831ff Mon Sep 17 00:00:00 2001 From: Filip Leonarski Date: Fri, 31 Jul 2026 15:02:11 +0200 Subject: [PATCH] geom_refinement: stop committing refinements that did not converge Four of the seven ceres::Solve calls in image_analysis obtained a Solver::Summary and never looked at it, so a solve that failed numerically had its parameters written back and was reported as success. StillsPartialityRefine and both PostRefine solves already gated on IsSolutionUsable(); this brings the rest to the same contract. IsSolutionUsable() is the right test rather than checking for CONVERGENCE: it accepts a solve that ran out of iterations or wall-clock time but still descended, which is exactly what the real-time callers depend on when they set max_solver_time instead of max_num_iterations. Only FAILURE and USER_FAILURE are rejected. XtalOptimizer checks before the write-back, so a failed refinement now leaves the caller's geom and latt untouched instead of half-updated. GeometryRefiner folds it into result.ok, which previously reported success from spot and frame counts alone. RingOptimizer returns a geometry by value that both callers assign straight back over their input, so it hands back the unchanged reference rather than a diverged beam centre. Co-Authored-By: Claude Opus 5 (1M context) --- image_analysis/geom_refinement/GeometryRefiner.cpp | 7 ++++++- image_analysis/geom_refinement/RingOptimizer.cpp | 6 ++++++ image_analysis/geom_refinement/XtalOptimizer.cpp | 10 ++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/image_analysis/geom_refinement/GeometryRefiner.cpp b/image_analysis/geom_refinement/GeometryRefiner.cpp index b10ca118..b7eb486e 100644 --- a/image_analysis/geom_refinement/GeometryRefiner.cpp +++ b/image_analysis/geom_refinement/GeometryRefiner.cpp @@ -174,6 +174,10 @@ GeometryRefinerResult RefineGlobalGeometry(const DiffractionGeometry &nominal_ge return 0.3 - (0.3 - 0.1) * t; }; + // Whether the last solve that ran produced a usable solution; folded into result.ok below, + // which otherwise reports success on spot and frame counts alone. + bool solve_usable = false; + for (int round = 0; round < settings.rounds; ++round) { EffectiveCell(system, cell_len, cell_ang, eff_len, eff_alpha, eff_beta, eff_gamma); @@ -265,6 +269,7 @@ GeometryRefinerResult RefineGlobalGeometry(const DiffractionGeometry &nominal_ge ceres::Solver::Summary summary; ceres::Solve(options, &problem, &summary); + solve_usable = summary.IsSolutionUsable(); result.frames_used = frames_used; result.spots_used = spots_used; @@ -298,7 +303,7 @@ GeometryRefinerResult RefineGlobalGeometry(const DiffractionGeometry &nominal_ge result.median_residual_px = residuals[residuals.size() / 2]; } - result.ok = result.spots_used >= settings.min_spots_total && result.frames_used > 0; + result.ok = solve_usable && result.spots_used >= settings.min_spots_total && result.frames_used > 0; result.beam_x_px = beam[0]; result.beam_y_px = beam[1]; result.distance_mm = distance_mm; diff --git a/image_analysis/geom_refinement/RingOptimizer.cpp b/image_analysis/geom_refinement/RingOptimizer.cpp index 626f66f8..1a120180 100644 --- a/image_analysis/geom_refinement/RingOptimizer.cpp +++ b/image_analysis/geom_refinement/RingOptimizer.cpp @@ -94,6 +94,12 @@ DiffractionGeometry RingOptimizer::Run(const std::vector &in // Run optimization ceres::Solve(options, &problem, &summary); + // A failed fit must not move the detector geometry. Both callers assign the result straight + // back over the geometry they passed in, so handing back the reference leaves the calibration + // where it was instead of committing a diverged beam centre and distance. + if (!summary.IsSolutionUsable()) + return reference; + DiffractionGeometry refined_geom(reference); refined_geom.BeamX_pxl(center_x).BeamY_pxl(center_y).DetectorDistance_mm(distance) .PoniRot1_rad(rot1).PoniRot2_rad(rot2); diff --git a/image_analysis/geom_refinement/XtalOptimizer.cpp b/image_analysis/geom_refinement/XtalOptimizer.cpp index c769e871..a2843d78 100644 --- a/image_analysis/geom_refinement/XtalOptimizer.cpp +++ b/image_analysis/geom_refinement/XtalOptimizer.cpp @@ -356,6 +356,13 @@ bool XtalOptimizerInternal(XtalOptimizerData &data, // Run optimization ceres::Solve(options, &problem, &summary); + // Only a genuine numerical failure is rejected here: a solve that ran out of iterations or + // out of time but still descended counts as usable, which is what the real-time caller + // relies on when it sets max_solver_time. Checked before anything is written back, so a + // failed refinement leaves data untouched rather than committing half a fit. + if (!summary.IsSolutionUsable()) + return false; + if (data.refine_beam_center) { data.beam_corr_x = data.geom.GetBeamX_pxl() - beam[0]; data.beam_corr_y = data.geom.GetBeamY_pxl() - beam[1]; @@ -490,6 +497,9 @@ bool XtalOptimizerRotationOnly(XtalOptimizerData &data, ceres::Solver::Summary summary; ceres::Solve(options, &problem, &summary); + if (!summary.IsSolutionUsable()) + return false; + // Apply rotation to direct-lattice vectors. // ceres::AngleAxisToRotationMatrix writes a **row-major** 3×3 matrix, // and Eigen's << operator also fills row-by-row, so the assignment