From f9ff89dc73c3f305339f211ac5e17c319dfa721a Mon Sep 17 00:00:00 2001 From: menzel Date: Tue, 25 Aug 2026 21:38:50 +0200 Subject: [PATCH] fix(cont_grid): apply the premove along the direction of travel The premove exists so the fast axis is already at speed when it reaches the first point and still moving past the last one. It was applied with a fixed sign: self.fast_start -= premove_distance self.fast_end += premove_distance which only lengthens an ascending line. On a descending one both ends move inward instead: the stage starts past the first point and stops before the last, with no run-up at either end, while frames_per_trigger -- computed earlier from the original range -- still expects the full travel, so the tail of the burst fires into a stage that has already arrived. When the premove exceeds half the range the two ends cross over and the line runs backwards; the test fixture's premove is 10.0 against a range of 2.0, so that is the case it covers. Latent rather than live: fast_axis_always_in_pos_dir defaults to True and normalises a descending request to an ascending line before any of this runs, so the wrong sign is only reachable when a caller passes the flag as False. The flag is a documented scan argument, so the path is real -- but no scan taken at the default has ever gone through it. Also pins the normalisation itself. It is worth a test of its own because it means a scan file records fast_start > fast_end while the stage ran the other way, which is a trap when reading data back: the first point of a line is at the numerically *lower* end regardless of what the request said. --- csaxs_bec/scans/scans_v4/cont_grid.py | 16 ++++- tests/tests_scans/test_cont_grid_scan.py | 86 +++++++++++++++++++++++- 2 files changed, 97 insertions(+), 5 deletions(-) diff --git a/csaxs_bec/scans/scans_v4/cont_grid.py b/csaxs_bec/scans/scans_v4/cont_grid.py index f9c7f32c..410bddef 100644 --- a/csaxs_bec/scans/scans_v4/cont_grid.py +++ b/csaxs_bec/scans/scans_v4/cont_grid.py @@ -171,9 +171,19 @@ class ContGrid(ScanBase): self.positions = deepcopy(positions[(frames_per_trigger - 1) :: frames_per_trigger, :]) - # Adjust premove - self.fast_start -= self._cont_motor_params["premove_distance"] - self.fast_end += self._cont_motor_params["premove_distance"] + # Adjust premove, along the direction of travel. + # + # The premove exists so the fast axis is already at speed when it reaches + # the first point and still moving past the last one. Applied with a fixed + # sign it only does that for an ascending line. On a descending line + # (fast_start > fast_end) both ends move *inward* instead: the stage starts + # past the first point and stops before the last, with no run-up at either + # end, while frames_per_trigger -- computed above from the original range -- + # still expects the full travel. When the premove exceeds half the range the + # two ends cross over and the line runs backwards. + direction = 1.0 if self.fast_end >= self.fast_start else -1.0 + self.fast_start -= direction * self._cont_motor_params["premove_distance"] + self.fast_end += direction * self._cont_motor_params["premove_distance"] self.actions.set_device_readout_priority(self.motors, priority="monitored") self.update_scan_info( diff --git a/tests/tests_scans/test_cont_grid_scan.py b/tests/tests_scans/test_cont_grid_scan.py index efeec8d2..f29df0e3 100644 --- a/tests/tests_scans/test_cont_grid_scan.py +++ b/tests/tests_scans/test_cont_grid_scan.py @@ -12,7 +12,13 @@ from bec_server.scan_server.tests.scan_hook_tests import ( CONT_GRID_HOOK_TESTS = [*DEFAULT_HOOK_TESTS, *PREMOVE_HOOK_TESTS] -def _assemble_cont_grid_scan(v4_scan_assembler, device_manager): +def _assemble_cont_grid_scan( + v4_scan_assembler, + device_manager, + fast_start=-1.0, + fast_stop=1.0, + fast_axis_always_in_pos_dir=True, +): custom_samx = MockCustomDevice( "samx", device_info={ @@ -78,7 +84,19 @@ def _assemble_cont_grid_scan(v4_scan_assembler, device_manager): device_manager.add_device(custom_samy, replace=True) device_manager.add_device(custom_mcs, replace=True) device_manager.add_device(custom_ddg1, replace=True) - return v4_scan_assembler("cont_grid", "samx", -1.0, 1.0, 1, "samy", -2.0, 2.0, 2, exp_time=0.1) + return v4_scan_assembler( + "cont_grid", + "samx", + fast_start, + fast_stop, + 1, + "samy", + -2.0, + 2.0, + 2, + exp_time=0.1, + fast_axis_always_in_pos_dir=fast_axis_always_in_pos_dir, + ) @pytest.mark.parametrize(("hook_name", "hook_tests"), CONT_GRID_HOOK_TESTS) @@ -110,6 +128,70 @@ def test_cont_grid_post_scan_waits_for_completion_and_moves_back_when_relative( assert completion_status.wait_calls == 1 +@pytest.mark.parametrize( + ("fast_start", "fast_stop"), [(-1.0, 1.0), (1.0, -1.0)], ids=["ascending", "descending"] +) +def test_cont_grid_premove_extends_the_line_in_both_directions( + v4_scan_assembler, device_manager, fast_start, fast_stop +): + """The premove must lengthen the travel whichever way the line runs. + + Applied with a fixed sign it lengthens an ascending line and *shortens* a + descending one, so the stage starts past the first point and stops before the + last with no run-up at either end -- while frames_per_trigger still expects the + full travel, leaving the tail of the burst firing into a stationary stage. + """ + scan = _assemble_cont_grid_scan( + v4_scan_assembler, + device_manager, + fast_start=fast_start, + fast_stop=fast_stop, + fast_axis_always_in_pos_dir=False, + ) + + scan.prepare_scan() + + premove = scan._cont_motor_params["premove_distance"] + assert premove > 0 # otherwise this test proves nothing + assert scan.fast_start == pytest.approx(fast_start - np.sign(fast_stop - fast_start) * premove) + assert scan.fast_end == pytest.approx(fast_stop + np.sign(fast_stop - fast_start) * premove) + # Travel is longer than requested, and still runs the way it was asked to. + assert abs(scan.fast_end - scan.fast_start) > abs(fast_stop - fast_start) + assert np.sign(scan.fast_end - scan.fast_start) == np.sign(fast_stop - fast_start) + + +def test_cont_grid_premove_does_not_reverse_a_descending_line( + v4_scan_assembler, device_manager +): + """The sharpest form of the bug: with a premove larger than half the range the + two adjusted ends cross over, and a line asked to run 1.0 -> -1.0 runs -1.0 -> + 1.0 instead. This fixture's premove is 10.0 against a range of 2.0, so the + crossover is what the unfixed code produces here.""" + scan = _assemble_cont_grid_scan( + v4_scan_assembler, + device_manager, + fast_start=1.0, + fast_stop=-1.0, + fast_axis_always_in_pos_dir=False, + ) + + scan.prepare_scan() + + assert scan._cont_motor_params["premove_distance"] > abs(1.0 - -1.0) / 2 + assert scan.fast_start > scan.fast_end + + +def test_cont_grid_normalises_direction_by_default(v4_scan_assembler, device_manager): + """A descending request is silently turned into an ascending line unless the + caller opts out. Worth pinning: it means a scan file records fast_start > + fast_end while the stage ran the other way, which is a trap when reading data + back, and it is what keeps the premove sign latent at the default.""" + scan = _assemble_cont_grid_scan(v4_scan_assembler, device_manager, fast_start=1.0, fast_stop=-1.0) + + assert scan.fast_start == -1.0 + assert scan.fast_end == 1.0 + + def test_cont_grid_prepare_scan_keeps_generated_positions_stable(v4_scan_assembler, device_manager): scan = _assemble_cont_grid_scan(v4_scan_assembler, device_manager)