From 3e56d9692199f3db6be32593ce0e4c5ec116eca1 Mon Sep 17 00:00:00 2001 From: Filip Leonarski Date: Thu, 30 Jul 2026 22:03:18 +0200 Subject: [PATCH] Stills partiality: adopt the refined tilt only when it fits better RefineOne re-measured the image's correlation to the reference after writing the refined partialities - because --min-image-cc drops images by it - and then ignored what it measured. A crystal the tilt model suits worse than the fixed partiality it replaces kept the refined model anyway, and the refinement is on by default. Compare against the CC the crystal arrived with and put it back untouched when the refinement does not improve it, which is the same state a crystal with too few reflections to fit ends in. Also four things noted in review and left until now: AdaptiveThresholdTest.cpp was listed twice in the test target, AdaptiveThreshold.h was the one header in image_analysis/spot_finding not in its library's source list, CLAUDE.md said update_version.sh rewrites VERSION when it only reads it, and the CHANGELOG did not mention that image_scale_b is gone from the plot_type enum - which breaks a client that asks for that plot. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 5 +++-- docs/CHANGELOG.md | 2 +- .../scale_merge/StillsPartialityRefine.cpp | 22 +++++++++++++++++++ image_analysis/spot_finding/CMakeLists.txt | 1 + tests/CMakeLists.txt | 1 - 5 files changed, 27 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 4a4086fc..1d238217 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -208,8 +208,9 @@ the system, stays unchanged. - Python client → `python-client/` (and `gen_python_client.sh`, published as PyPI `jfjoch-client`). - TypeScript frontend client → `frontend/src/client/` (hey-api `openapi-ts`, `npm run openapi`). -When you change `jfjoch_api.yaml`, regenerate the relevant client(s); for a version bump run -`update_version.sh` (also rewrites `VERSION`, `frontend/src/version.ts`, and the Redoc html). +When you change `jfjoch_api.yaml`, regenerate the relevant client(s); for a version bump write the new version into `VERSION` and run +`update_version.sh` (which reads it and rewrites `frontend/src/version.ts`, `docs/conf.py`, the +python client and the Redoc html). ## Frontend diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 7bf13818..2d795fec 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -15,7 +15,7 @@ This is an UNSTABLE release. It includes many experimental features, as well as * rugnux: New `--search-min-zeta` (rotation default 0.85). * rugnux: Reports how close a symmetry axis lies to the spindle. * rugnux: Azimuthal-integration and spot-finding resolution limits default to the detector — including **rotation** data, which no longer keeps a 1.5 Å spot-finding limit (`--spot-high-resolution ` still sets one). -* **API (breaking for older clients)**: `azim_int_settings.high_q_recipA` and `spot_finding_settings.high_resolution_limit` are no longer `required`. Both now mean "as far as the detector reaches" when unset, and are **omitted** from the response instead of carrying a placeholder value. A client generated from an rc.160-or-earlier spec that assumes the field is always present (the generated C++ `from_json` does `j.at(...)`) raises when it is missing. Because the azimuthal-integration limit now defaults to unset, a stock rc.161 broker omits `high_q_recipA` from `GET /config/azim_int` with no operator action. Regenerate the client (`jfjoch-client` 1.0.0-rc.161, `frontend/src/client`) or read both fields as optional. +* **API (breaking for older clients)**: the `image_scale_b` value is gone from the `plot_type` enum, so a client that requests that plot now gets an error rather than a curve. Also, `azim_int_settings.high_q_recipA` and `spot_finding_settings.high_resolution_limit` are no longer `required`. Both now mean "as far as the detector reaches" when unset, and are **omitted** from the response instead of carrying a placeholder value. A client generated from an rc.160-or-earlier spec that assumes the field is always present (the generated C++ `from_json` does `j.at(...)`) raises when it is missing. Because the azimuthal-integration limit now defaults to unset, a stock rc.161 broker omits `high_q_recipA` from `GET /config/azim_int` with no operator action. Regenerate the client (`jfjoch-client` 1.0.0-rc.161, `frontend/src/client`) or read both fields as optional. * rugnux: Several non-helping stills scaling/detection knobs removed. * Reader: A stored dataset is read as **int32**, so the saturation cap comes from the file's own `saturation_value` instead of a container depth that no longer matched the pixels handed out (32-bit EIGER2 files were capping at 32767 and dropping their strongest reflections; 16- and 8-bit files were capping at half their range). * jfjoch_viewer: Image rendering and interaction performance improved. diff --git a/image_analysis/scale_merge/StillsPartialityRefine.cpp b/image_analysis/scale_merge/StillsPartialityRefine.cpp index 02b77254..1df3787c 100644 --- a/image_analysis/scale_merge/StillsPartialityRefine.cpp +++ b/image_analysis/scale_merge/StillsPartialityRefine.cpp @@ -293,6 +293,16 @@ double StillsPartialityRefine::RefineOne(IntegrationOutcome &outcome, } } + // Keep what the crystal came in with. This refinement is on by default, so a crystal the tilt model + // happens to suit WORSE than the fixed partiality it replaces must not be made worse by it - and + // whether it suits is only known once the corrections are written and the CC re-measured. + const auto cc_before = outcome.image_scale_cc; + const auto g_before = outcome.image_scale_g; + std::vector> before; // partiality, image_scale_corr + before.reserve(outcome.reflections.size()); + for (const auto &r: outcome.reflections) + before.emplace_back(r.partiality, r.image_scale_corr); + // Write the refined partiality + scale correction onto every reflection of the crystal (not only the // fit subset), so the merge sees a consistent model. image_scale_corr = rlp / (partiality * G), the // same composition ScaleOnTheFly writes. @@ -313,6 +323,18 @@ double StillsPartialityRefine::RefineOne(IntegrationOutcome &outcome, // the CC of the data that is actually merged. const auto [cc, cc_n] = ImageReferenceCC(outcome.reflections, reference, hkl_key_generator_, d_min_limit_, min_partiality_); + + // Adopt the refined model only if it correlates with the reference at least as well as the model it + // replaces. Rejecting puts the crystal back exactly as it arrived, which is the same state a + // crystal with too few reflections to fit ends in. + if (cc_before.has_value() && std::isfinite(*cc_before) && std::isfinite(cc) && cc < *cc_before) { + for (size_t i = 0; i < outcome.reflections.size(); ++i) { + outcome.reflections[i].partiality = before[i].first; + outcome.reflections[i].image_scale_corr = before[i].second; + } + outcome.image_scale_g = g_before; + return 0.0; // nothing adopted, so no tilt to report + } outcome.image_scale_cc = cc; outcome.image_scale_cc_n = cc_n; diff --git a/image_analysis/spot_finding/CMakeLists.txt b/image_analysis/spot_finding/CMakeLists.txt index 59c39afb..8164f10d 100644 --- a/image_analysis/spot_finding/CMakeLists.txt +++ b/image_analysis/spot_finding/CMakeLists.txt @@ -3,6 +3,7 @@ ADD_LIBRARY(JFJochSpotFinding STATIC ImageSpotFinderCPU.h AdaptiveSpotFinderCPU.cpp AdaptiveSpotFinderCPU.h + AdaptiveThreshold.h SpotUtils.cpp SpotUtils.h SpotFindingSettings.h diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index bd5b38ec..1e3959e9 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -66,7 +66,6 @@ ADD_EXECUTABLE(jfjoch_test ImageSpotFinderCPUTest.cpp ImageSpotFinderGPUTest.cpp AdaptiveSpotFinderGPUTest.cpp - AdaptiveThresholdTest.cpp CalcBraggPredictionTest.cpp SpotUtilsTest.cpp LatticeSearchTest.cpp