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)