From 92a615085df9e87ee813baecb27c5a524bd88ca7 Mon Sep 17 00:00:00 2001 From: Filip Leonarski Date: Wed, 2 Sep 2026 10:56:21 +0200 Subject: [PATCH] grid scan: the snake reverses on the acquisition row, not the display row GetElementPosFast_step took the snake parity from GetElementPosSlow_step, which is the DISPLAY row: it is the acquisition row r = image / n_fast, flipped to (n_slow-1) - r when the slow step is negative. So for a negative slow step the parity it hands back is parity(n_slow-1) XOR parity(r), and with an even n_slow that is inverted on every row - the whole raster comes out mirrored along the fast axis. An odd n_slow leaves it correct, so the same scan collected with 20 or 25 images disagreed about where image 0 sat: n_fast=5, fast +1.5 um, slow -2.5 um, snake on gives images 0..4 at fast index 4,3,2,1,0 with 4 rows and 0,1,2,3,4 with 5 rows. A positive slow step was correct at both counts, and so was every non-snake configuration. Snake means the stage reverses direction on alternate rows in acquisition order, so the parity has to come from the acquisition row. Taking it from image_number / n_fast directly makes the fast index independent of the slow axis and of the row count, and drops the call into the display-row function that caused the coupling. vertical_scan only relabels which axis is fast, so it was wrong in exactly the same way and is fixed by the same line. Affected files: written by an affected build, with snake on, a negative grid slow step (step_y for a horizontal scan, step_x for a vertical one), and an even number of rows. Their /entry/sample/transformations/grid_scan_x or _y is mirrored along the fast axis, as was the grid map in the frontend and the viewer - both mirrored together, which is why neither showed it. Tests: the interaction of snake with the step signs was never asserted, only each in isolation, so add a table over snake x {+,- fast step} x {+,- slow step} x {even, odd row count} x {horizontal, vertical} asserting positions, plus a case running one affected configuration through GetXContainer_m / GetYContainer_m and Rearrange. Every pre-existing assertion is unchanged and still passes; only the four negative-slow, even-row cells of the product move. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EFEJG6WBQv8th4UJFNe53N --- common/GridScanSettings.cpp | 7 ++- docs/CHANGELOG.md | 1 + tests/GridScanSettingsTest.cpp | 93 ++++++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 2 deletions(-) diff --git a/common/GridScanSettings.cpp b/common/GridScanSettings.cpp index 26295fdae..2fd6a9ed4 100644 --- a/common/GridScanSettings.cpp +++ b/common/GridScanSettings.cpp @@ -86,11 +86,14 @@ int64_t GridScanSettings::GetElementPosFast_step(int64_t image_number) const { throw JFJochException(JFJochExceptionCategory::InputParameterInvalid, "Image out of bounds"); - int64_t slow = GetElementPosSlow_step(image_number); + // Snake reverses the stage direction on alternate rows in ACQUISITION order, so the parity has + // to come from the acquisition row, not from the display row GetElementPosSlow_step returns + // (the two differ by (n_slow-1) - row when the slow step is negative). + int64_t row = image_number / n_fast; int64_t fast = image_number % n_fast; if (GetGridElemFast_um() < 0) fast = (n_fast - 1) - fast; - if (snake_scan && (slow % 2 == 1)) + if (snake_scan && (row % 2 == 1)) fast = (n_fast - 1) - fast; return fast; } diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 398cca775..b491bb452 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -25,6 +25,7 @@ * `rugnux` refines only the detector-tilt component the data determine, holding the one along the rotation axis, so a beam-centre error is no longer reported as a tilt. * `jfjoch_writer` writes `direct_beam_x`/`direct_beam_y` in the HDF5 master - where the undeflected beam lands - beside the `beam_center_x`/`beam_center_y` PONI. * `jfjoch_writer` writes `/entry/MX/peakCountUnfiltered` in the HDF5 master beside the other per-image spot counts, instead of only in the data files. +* A snake grid scan with a negative slow step and an even number of rows no longer has its positions mirrored along the fast axis in the HDF5 master and the grid map, so the positions recorded for that configuration change. * `jfjoch_broker` sends `direct_beam_x`/`direct_beam_y` on the CBOR start message. * `dataset_settings` takes `beam_size_x_um`/`beam_size_y_um`, the size of the X-ray beam at the sample, and `jfjoch_writer` writes them as `incident_beam_size` in the HDF5 master. * `dataset_settings` accepts any `smargon.chi_deg`, which was restricted to 0-90 degrees. diff --git a/tests/GridScanSettingsTest.cpp b/tests/GridScanSettingsTest.cpp index c8e7e94c4..66a28bc8f 100644 --- a/tests/GridScanSettingsTest.cpp +++ b/tests/GridScanSettingsTest.cpp @@ -533,3 +533,96 @@ TEST_CASE("GridScanSettings GetYContainer_m", "[GridScanSettings][container]") { REQUIRE(y_neg[4] == Catch::Approx(0.0)); REQUIRE(y_neg[5] == Catch::Approx(0.0)); } + +namespace { + struct SnakeCase { + const char *name; + float fast_step_um; + float slow_step_um; + int64_t n_slow; + // Expected (fast, slow) element position, one entry per image, in acquisition order. + std::vector> expected; + }; +} + +// A snake reverses the stage on alternate rows in acquisition order, so an image's fast position +// cannot depend on how many rows the scan ended up having. Every case below is listed for an even +// and an odd row count for exactly that reason: the fast column of the two must agree image by +// image (the slow column does not, because a negative slow step measures from the last row). +TEST_CASE("GridScanSettings snake raster with negative steps", "[GridScanSettings]") { + constexpr int64_t n_fast = 3; + + const std::vector cases = { + {"fast +, slow +, 2 rows", 1.5f, 2.5f, 2, + {{0, 0}, {1, 0}, {2, 0}, {2, 1}, {1, 1}, {0, 1}}}, + {"fast +, slow +, 3 rows", 1.5f, 2.5f, 3, + {{0, 0}, {1, 0}, {2, 0}, {2, 1}, {1, 1}, {0, 1}, {0, 2}, {1, 2}, {2, 2}}}, + {"fast +, slow -, 2 rows", 1.5f, -2.5f, 2, + {{0, 1}, {1, 1}, {2, 1}, {2, 0}, {1, 0}, {0, 0}}}, + {"fast +, slow -, 3 rows", 1.5f, -2.5f, 3, + {{0, 2}, {1, 2}, {2, 2}, {2, 1}, {1, 1}, {0, 1}, {0, 0}, {1, 0}, {2, 0}}}, + {"fast -, slow +, 2 rows", -1.5f, 2.5f, 2, + {{2, 0}, {1, 0}, {0, 0}, {0, 1}, {1, 1}, {2, 1}}}, + {"fast -, slow +, 3 rows", -1.5f, 2.5f, 3, + {{2, 0}, {1, 0}, {0, 0}, {0, 1}, {1, 1}, {2, 1}, {2, 2}, {1, 2}, {0, 2}}}, + {"fast -, slow -, 2 rows", -1.5f, -2.5f, 2, + {{2, 1}, {1, 1}, {0, 1}, {0, 0}, {1, 0}, {2, 0}}}, + {"fast -, slow -, 3 rows", -1.5f, -2.5f, 3, + {{2, 2}, {1, 2}, {0, 2}, {0, 1}, {1, 1}, {2, 1}, {2, 0}, {1, 0}, {0, 0}}}, + }; + + for (const auto &c : cases) { + for (bool vertical : {false, true}) { + INFO(c.name << (vertical ? ", vertical" : ", horizontal")); + + GridScanSettings grid(n_fast, + vertical ? c.slow_step_um : c.fast_step_um, + vertical ? c.fast_step_um : c.slow_step_um, + true, vertical); + grid.ImageNum(n_fast * c.n_slow); + REQUIRE(grid.GetNSlow() == c.n_slow); + + for (size_t i = 0; i < c.expected.size(); i++) { + const auto image = static_cast(i); + const int64_t fast = vertical ? grid.GetElementPosY_step(image) + : grid.GetElementPosX_step(image); + const int64_t slow = vertical ? grid.GetElementPosX_step(image) + : grid.GetElementPosY_step(image); + INFO("image " << i); + CHECK(fast == c.expected[i].first); + CHECK(slow == c.expected[i].second); + } + } + } +} + +// The same configuration seen through the two consumers of the element positions: the NXmx +// transformation containers and the scan-result rearrangement. +TEST_CASE("GridScanSettings snake with negative slow step, even row count", "[GridScanSettings][container][rearrange]") { + GridScanSettings grid(3, 1.0f, -10.0f, true, false); + grid.ImageNum(6); // 2 rows of 3 + + const auto x_m = grid.GetXContainer_m(6); + const std::vector x_expected = {0.0e-6, 1.0e-6, 2.0e-6, 2.0e-6, 1.0e-6, 0.0e-6}; + for (size_t i = 0; i < x_expected.size(); i++) { + INFO("image " << i); + CHECK(x_m[i] == Catch::Approx(x_expected[i])); + } + + const auto y_m = grid.GetYContainer_m(6); + const std::vector y_expected = {10.0e-6, 10.0e-6, 10.0e-6, 0.0, 0.0, 0.0}; + for (size_t i = 0; i < y_expected.size(); i++) { + INFO("image " << i); + CHECK(y_m[i] == Catch::Approx(y_expected[i])); + } + + // Last row acquired is the top row of the grid, and it was acquired right-to-left. + const std::vector input = {0, 1, 2, 3, 4, 5}; + const std::vector output = grid.Rearrange(input); + const std::vector rearranged_expected = {5, 4, 3, 0, 1, 2}; + REQUIRE(output.size() == rearranged_expected.size()); + for (size_t i = 0; i < rearranged_expected.size(); i++) { + INFO("element " << i); + CHECK(output[i] == Catch::Approx(rearranged_expected[i])); + } +}