From 5c6dedea71b0e7af29cabbf6a31780f25edd8ca7 Mon Sep 17 00:00:00 2001 From: Filip Leonarski Date: Sun, 20 Sep 2026 18:59:59 +0200 Subject: [PATCH] Do not read the oscillation width as the rotation step SweepLayout rejected any inter-frame difference below half the header's oscillation width as read-back jitter. A series that steps by less than it exposes - overlapping wedges, or a writer that puts the whole sweep's range in that field - then had every difference rejected, no step was found, and the files fell into the "series that never turns" branch, which lays them out end to end and, unlike the gapped path, said nothing. The jitter bound is now absolute (1e-4 deg, below any step an instrument makes and above 32-bit float read-back noise), and the end-to-end fallback warns when the angles do span a rotation. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_013nW6FNRP1bBJJ8pfHiByAT --- reader/SweepLayout.cpp | 24 ++++++++++++++++++++---- tests/SweepLayoutTest.cpp | 21 +++++++++++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/reader/SweepLayout.cpp b/reader/SweepLayout.cpp index 981fa0137..67cda450f 100644 --- a/reader/SweepLayout.cpp +++ b/reader/SweepLayout.cpp @@ -125,14 +125,20 @@ Layout Place(const std::vector &frames, const std::string &logger_name) { // Angle_increment is not used for this: it is the oscillation WIDTH, which a series with // overlapping or spaced wedges does not step by. // - // A difference smaller than half the oscillation width is not a step but jitter in the recorded - // angle: no instrument slices finer than it exposes, so wedges overlapping twofold would be a - // read-back wobble, and taking one as the step would spread the sweep over millions of slots. + // A difference below this is jitter in the recorded angle rather than a step, and taking one as + // the step would spread the sweep over millions of slots. The bound is ABSOLUTE and far below + // any move an instrument makes: these headers write their angles as 32-bit floats, which is a + // few parts in 100000 - under 1e-4 deg even at 360 - while the finest rotation step collected is + // a thousandth of a degree. Deriving it from the oscillation width instead, as this used to, is + // the same mistake the paragraph above warns of: a series with overlapping wedges steps by far + // less than it exposes, and some writers put the whole sweep's range in that field, so every + // inter-frame difference was rejected, the guess stayed zero and a turning series fell into the + // "never turns" branch below and was laid out end to end. // // It is only a guess, and deliberately the crudest one: a single recorded difference, and the // one biased furthest low by the read-back noise. It is used below to count STEPS between // neighbours, never to place a frame outright. - const double too_fine = 0.5 * std::abs(f0.increment_deg); + constexpr double too_fine = 1e-4; // degrees double guess = 0; for (size_t i = 1; i < frames.size(); i++) { const double d = Fold(angle[i] - angle[i - 1]); @@ -144,6 +150,16 @@ Layout Place(const std::vector &frames, const std::string &logger_name) { // A series that never turns: a grid scan, a set of stills, or a single image. There is no sweep // to place anything on, so the files are the slots and the header's nominal increment stands. if (guess == 0) { + // ... unless the angles say it DOES turn, over the series if never between two neighbours. + // Then this layout is a guess - the files end to end, at the nominal increment - and it has + // to say so, as the gapped path below does. Nothing else in this branch reports anything. + const double turned = std::abs(angle.back() - angle[0]); + if (turned > std::max(std::abs(f0.increment_deg), 1e-3)) + logger.Warning("The angles of these {} images span {:.4f} deg, but no two neighbours are " + "more than {:g} deg apart, so no rotation step can be read from them. They " + "are laid out one after another at the header's {:.4f} deg - check that " + "this is the sweep the files describe.", + frames.size(), turned, too_fine, f0.increment_deg); out.files.reserve(frames.size()); for (const auto &f : frames) out.files.push_back(f.path); diff --git a/tests/SweepLayoutTest.cpp b/tests/SweepLayoutTest.cpp index ee472c4d2..a3ef530d3 100644 --- a/tests/SweepLayoutTest.cpp +++ b/tests/SweepLayoutTest.cpp @@ -153,3 +153,24 @@ TEST_CASE("SweepLayout_RepeatedAngleRefused") { // Two sweeps of the same crystal concatenated: the second covers angles the first already has. CHECK_THROWS_AS(sweep::Place(Series({0.0, 0.1, 0.2, 0.1, 0.2}, 0.1), "test"), JFJochException); } + +TEST_CASE("SweepLayout_StepFinerThanTheOscillationWidth") { + // The header field is the oscillation WIDTH, and a series can step by far less than it exposes - + // or, on some writers, hold the whole sweep's range in that field. Taking it for the step + // rejected every inter-frame difference as jitter: the sweep then read as one that never turns + // and was laid out end to end, every frame past the first at the wrong angle. + std::vector angles; + for (int i = 0; i < 100; i++) + angles.push_back(30.0 + 0.1 * i); + + const auto wedges = sweep::Place(Series(angles, 1.0), "test"); // 1 deg wedges, 0.1 deg apart + REQUIRE(wedges.files.size() == 100); + CHECK(wedges.present == 100); + CHECK(wedges.start_deg == Catch::Approx(30.0)); + CHECK(wedges.increment_deg == Catch::Approx(0.1)); + + const auto total = sweep::Place(Series(angles, 10.0), "test"); // the field holds the range + REQUIRE(total.files.size() == 100); + CHECK(total.present == 100); + CHECK(total.increment_deg == Catch::Approx(0.1)); +}