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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EFEJG6WBQv8th4UJFNe53N

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;
 }

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.

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<std::pair<int64_t, int64_t>> 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<SnakeCase> 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<int64_t>(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<double> 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<double> 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<float> input = {0, 1, 2, 3, 4, 5};
+    const std::vector<float> output = grid.Rearrange(input);
+    const std::vector<float> 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]));
+    }
+}