diff --git a/csaxs_bec/devices/epics/delay_generator_csaxs/ddg_1.py b/csaxs_bec/devices/epics/delay_generator_csaxs/ddg_1.py index ad05299d..6bf5f793 100644 --- a/csaxs_bec/devices/epics/delay_generator_csaxs/ddg_1.py +++ b/csaxs_bec/devices/epics/delay_generator_csaxs/ddg_1.py @@ -21,7 +21,7 @@ DELAY PAIRS: PULSE train for the MCS card. The MCS card needs one extra pulse to forward points. DELAY CHANNELS: -- a = t0 + 2ms (2ms delay to allow the shutter to open) +- a = t0 + DEFAULT_SHUTTER_TO_OPEN_DELAY (to allow the shutter to open) - b = a + 1us (short pulse) - c = t0 - d = a + exp_time * burst_count @@ -92,6 +92,81 @@ DEFAULT_IO_CONFIG: dict[AllChannelNames, ChannelConfig] = { DEFAULT_TRIGGER_SOURCE: TRIGGERSOURCE = TRIGGERSOURCE.SINGLE_SHOT +# NOTE How long the shutter is given to open before the first gate of a burst. +# The shutter fires on `cd` at t0 and the acquisition on `ab` is held back by this +# much, so whatever the shutter still owes at that point is lost from the FIRST +# point of every line and from nothing else. +# +# Measured 2026-08-25 from the first-point deficit in four commissioning scans, +# against the 2e-3 that was in place when they were taken: +# +# scan exp_time first/near lost +# 324 15 ms 0.655 5.18 ms +# 254 20 ms 0.732 5.36 ms +# 411 50 ms 0.881 5.93 ms +# 450 50 ms 0.879 6.04 ms +# +# The deficit is a fixed time, not a fixed fraction: it varies by 17% across a +# 3.3x range of exposure while the fraction varies by 2.9x. That is what makes it +# a shutter opening late rather than anything dose-dependent. The same deficit +# appears on the integrated scattering -- a different detector behind a different +# gate -- so the cause sits upstream of both readout chains. +# +# 2e-3 already allowed, ~5.6e-3 still lost => the shutter needs about 7.6 ms, +# rounded up to 8 ms. The spread across the four scans is 0.9 ms, so the third +# digit is not meaningful, and overshooting costs only the difference in dead +# time at the start of each line. +# To re-measure after changing this, take the first point of each line over the +# mean of its next few neighbours, averaged over all lines, and multiply the +# shortfall by exp_time. That is the exposure still being lost, in seconds. +# Compare against its own neighbours rather than the whole-line mean: a scan +# whose intensity drifts along the line otherwise reports a deficit that is not +# there. +DEFAULT_SHUTTER_TO_OPEN_DELAY = 8e-3 + +# Guard rail for set_shutter_to_open_delay. The delay is paid once per line, so a +# fat-fingered value would quietly stretch every line rather than fail. +MAX_SHUTTER_TO_OPEN_DELAY = 50e-3 + +# NOTE How long it takes a move issued by the scan server to reach the motor. +# +# cont_grid issues the line move and fires the burst a few milliseconds later, on +# the assumption that the only thing in between is the acceleration ramp. It is +# not: the move first has to cross the scan server -> device server -> EPICS -> +# controller chain, and that latency does not shrink when the scan is slow while +# the computed acc_time does. The burst therefore starts before the stage. +# +# Measured 2026-08-25 from the position readback of five commissioning scans, as +# the distance the stage was behind the trigger grid once the lag stopped growing: +# +# scan exp_time v_cmd lag latency +# 254 20 ms 0.5 mm/s 29.7 pts 0.594 s +# 450 50 ms 0.1 mm/s 11.9 pts 0.595 s +# 324 15 ms 0.667 mm/s 41.5 pts 0.623 s +# 473 100 ms 0.065 mm/s 6.3 pts 0.630 s +# 411 50 ms 0.1 mm/s 12.7 pts 0.635 s +# +# Constant to +-3.5% across a 6.7x range of exposure and a 10x range of velocity, +# on two axes, which is what makes compensating with a constant sound. In scan +# 473, sampled 74 times per line, the lag appears in the first interval and then +# holds at 6.3-6.4 points for the rest of the line while the velocity sits at +# exactly the commanded 0.065 mm/s: a start-up offset, not a velocity error. +# +# Deliberately NOT solved by polling for motion. Observing the readback costs a +# round trip of this same ~0.6 s, so it would trade a systematic offset for a +# jitter of similar size -- and a constant offset displaces every line equally, +# where a varying one shears the image line by line and cannot be undone +# afterwards. The real fix is to trigger the DDG from the motor +# (scan_type: hardware_triggered), taking the round trip out of the timing chain +# altogether; this is the stopgap until then. +# +# Set to 0 to get the previous behaviour back without redeploying. +DEFAULT_MOVE_START_LATENCY = 0.6 + +# Guard rail for set_move_start_latency. Paid once per line, like the shutter +# delay, so an implausible value costs scan time rather than raising. +MAX_MOVE_START_LATENCY = 5.0 + # NOTE Default readout times for each channel, can be adapted as needed. # These values are relevant to calculate proper widths of the timing signals. # They also define a minimum exposure time that can be used as they are subtracted @@ -103,7 +178,7 @@ DEFAULT_READOUT_TIMES = {"ab": 2e-4, "cd": 2e-4, "ef": 2e-4, "gh": 2e-4} # 0.2 # If the trigger scheme changes, adapt the values here together with the README and # PDF `trigger_scheme_ddg1_ddg2.pdf`. DEFAULT_REFERENCES: list[tuple[LiteralChannels, CHANNELREFERENCE]] = [ - ("A", CHANNELREFERENCE.T0), # T0 + 2ms delay + ("A", CHANNELREFERENCE.T0), # T0 + shutter-to-open delay ("B", CHANNELREFERENCE.A), ("C", CHANNELREFERENCE.T0), # T0 ("D", CHANNELREFERENCE.C), @@ -140,6 +215,9 @@ class DDG1(PSIDeviceBase, DelayGeneratorCSAXS): "keep_shutter_open_during_scan", "set_trigger", "get_shutter_to_open_delay", + "set_shutter_to_open_delay", + "get_move_start_latency", + "set_move_start_latency", "prepare_mcs_on_trigger", ] @@ -173,7 +251,8 @@ class DDG1(PSIDeviceBase, DelayGeneratorCSAXS): super().__init__( name=name, prefix=prefix, scan_info=scan_info, device_manager=device_manager, **kwargs ) - self._shutter_to_open_delay = 2e-3 + self._shutter_to_open_delay = DEFAULT_SHUTTER_TO_OPEN_DELAY + self._move_start_latency = DEFAULT_MOVE_START_LATENCY self.device_manager = device_manager self._poll_thread = threading.Thread(target=self._poll_event_status, daemon=True) self._poll_thread_run_event = threading.Event() @@ -244,11 +323,70 @@ class DDG1(PSIDeviceBase, DelayGeneratorCSAXS): """Get the current delay that is set to open the shutter before the exposure time.""" return self._shutter_to_open_delay + def get_move_start_latency(self) -> float: + """Get the delay a scan should leave between issuing a line move and triggering.""" + return self._move_start_latency + + def set_move_start_latency(self, latency: float) -> None: + """Set how long a scan waits after issuing a line move before it triggers. + + This is not a property of the delay generator. It is the latency of the + scan server -> device server -> EPICS -> controller path, and it lives here + because this is where cont_grid already fetches its trigger timing. Too + small and the burst starts before the stage does, so the first points of + every line pile up at the line start; too large and the stage runs past + the start of the line before the first gate fires. + + Set to 0 for the behaviour that preceded this parameter. + + Args: + latency (float): Delay in seconds. + + Raises: + ValueError: If negative or above MAX_MOVE_START_LATENCY. + """ + latency = float(latency) + if not 0 <= latency <= MAX_MOVE_START_LATENCY: + raise ValueError( + f"Move-start latency must be between 0 and {MAX_MOVE_START_LATENCY} s," + f" got {latency}." + ) + self._move_start_latency = latency + + def set_shutter_to_open_delay(self, delay: float) -> None: + """Set how long the shutter is given to open before the first gate fires. + + Exists so the value can be converged without redeploying the device server. + Too small and the first point of every line is under-exposed; too large and + every line is stretched by the difference for nothing. To measure the + result, see the note at DEFAULT_SHUTTER_TO_OPEN_DELAY. + + Args: + delay (float): Delay in seconds. 0 starts the acquisition together with + the shutter trigger. + + Raises: + ValueError: If the delay is negative or above MAX_SHUTTER_TO_OPEN_DELAY. + """ + delay = float(delay) + if not 0 <= delay <= MAX_SHUTTER_TO_OPEN_DELAY: + raise ValueError( + f"Shutter-to-open delay must be between 0 and" + f" {MAX_SHUTTER_TO_OPEN_DELAY} s, got {delay}." + ) + self._shutter_to_open_delay = delay + def keep_shutter_open_during_scan(self, open: True) -> None: """ Method to configure the delay generator for keeping the shutter open during a scans. - This means that the additional delay to open the shutter needs to be removed (2e-3) - from the timing of the signals. + This means that the additional delay to open the shutter needs to be removed + from the timing of the signals: a shutter that is already open owes nothing + to the first gate. + + NOTE Switching this off restores DEFAULT_SHUTTER_TO_OPEN_DELAY, and so + discards any value set with set_shutter_to_open_delay. Tune with the setter + and leave this alone, or change the default if the tuned value is the one + the beamline should keep. Args: open (bool): If True, the shutter will be kept open during the scan. @@ -257,7 +395,7 @@ class DDG1(PSIDeviceBase, DelayGeneratorCSAXS): if open is True: self._shutter_to_open_delay = 0 else: - self._shutter_to_open_delay = 2e-3 + self._shutter_to_open_delay = DEFAULT_SHUTTER_TO_OPEN_DELAY def on_stage(self) -> None: """ @@ -274,8 +412,10 @@ class DDG1(PSIDeviceBase, DelayGeneratorCSAXS): - We check if any default burst parameters need to be set, and set them if needed. - We calculate the burst pulse width based on the exposure time and frames_per_trigger. - We set the burst_period and the shutter signal (delay pairs cd) to be - exposure_time * frames_per_trigger + 3ms (2ms for shutter to open, 1ms to close). - - We set the delay pairs ab to be 2ms delayed (to allow the shutter to open) with a width of 1us to trigger DDG2. + exposure_time * frames_per_trigger + twice the shutter-to-open delay + (once to open, once to close). + - We set the delay pairs ab to be delayed by the shutter-to-open delay (to + allow the shutter to open) with a width of 1us to trigger DDG2. - We set the delay pairs ef to be triggered after the shutter closes with a width of 1us to trigger the MCS card. - Finally, we add a short sleep to ensure that the IOC and DDG HW process the values properly. """ @@ -333,7 +473,7 @@ class DDG1(PSIDeviceBase, DelayGeneratorCSAXS): self.burst_period.put(total_exposure_time) # Trigger DDG2 - # a = t0 + 2ms, b = a + 1us + # a = t0 + shutter-to-open delay, b = a + 1us # a has reference to t0, b has reference to a # AB is delayed by the shutter opening time, and the falling edge indicates the shutter has # fully closed, it has to be considered as the blocking signal for the next acquisition to start. @@ -343,7 +483,8 @@ class DDG1(PSIDeviceBase, DelayGeneratorCSAXS): # Trigger shutter # d = c/t0 + self._shutter_to_open_delay + exp_time * burst_count + 1ms # c has reference to t0, d has reference to c - # Shutter opens without delay at t0, closes after exp_time * burst_count + 2ms (self._shutter_to_open_delay) + # Shutter opens without delay at t0, closes after + # exp_time * burst_count + self._shutter_to_open_delay self.set_delay_pairs(channel="cd", delay=0, width=shutter_width) self.set_delay_pairs( diff --git a/csaxs_bec/scans/scans_v4/cont_grid.py b/csaxs_bec/scans/scans_v4/cont_grid.py index f9c7f32c..e5566d03 100644 --- a/csaxs_bec/scans/scans_v4/cont_grid.py +++ b/csaxs_bec/scans/scans_v4/cont_grid.py @@ -44,6 +44,7 @@ class ContinuousMotorParameter(TypedDict): acc_time: float | None premove_distance: float | None shutter_open_delay: float | None + move_start_latency: float | None num_lines: int | None @@ -270,7 +271,15 @@ class ContGrid(ScanBase): self.fast_axis.velocity.set(self._cont_motor_params["target_velocity"]).wait(timeout=5) self.fast_axis.acceleration.set(self._cont_motor_params["acc_time"]).wait(timeout=5) move_status = self.actions.set(motors, positions, wait=False) - time.sleep(self._cont_motor_params["acc_time"]) + # Wait for the move to reach the motor, and then for the stage to get up to + # speed. Only the second of those is acc_time; the first is the latency of + # the scan server -> device server -> EPICS -> controller path, which does + # not shrink with the scan velocity while acc_time does. Without it the + # burst starts before the stage and the first points of every line pile up + # at the line start. See DEFAULT_MOVE_START_LATENCY for the measurement. + time.sleep( + self._cont_motor_params["move_start_latency"] + self._cont_motor_params["acc_time"] + ) self.ddg1.trigger_shot.put(1) while not move_status.done: self.actions.read_monitored_devices(wait=True) @@ -350,6 +359,7 @@ class ContGrid(ScanBase): def _fetch_device_params(self): self._cont_motor_params["shutter_open_delay"] = self.ddg1.get_shutter_to_open_delay() + self._cont_motor_params["move_start_latency"] = self.ddg1.get_move_start_latency() self._cont_motor_params["original_acceleration"] = self.fast_axis.acceleration.get() self._cont_motor_params["original_velocity"] = self.fast_axis.velocity.get() self._cont_motor_params["base_velocity"] = self.fast_axis.base_velocity.get() diff --git a/tests/tests_devices/test_delay_generator_csaxs.py b/tests/tests_devices/test_delay_generator_csaxs.py index a484dae3..c19e3468 100644 --- a/tests/tests_devices/test_delay_generator_csaxs.py +++ b/tests/tests_devices/test_delay_generator_csaxs.py @@ -22,6 +22,12 @@ from csaxs_bec.devices.epics.delay_generator_csaxs.ddg_1 import ( from csaxs_bec.devices.epics.delay_generator_csaxs.ddg_1 import ( DEFAULT_TRIGGER_SOURCE as DDG1_DEFAULT_TRIGGER_SOURCE, ) +from csaxs_bec.devices.epics.delay_generator_csaxs.ddg_1 import ( + DEFAULT_MOVE_START_LATENCY, + DEFAULT_SHUTTER_TO_OPEN_DELAY, + MAX_MOVE_START_LATENCY, + MAX_SHUTTER_TO_OPEN_DELAY, +) from csaxs_bec.devices.epics.delay_generator_csaxs.ddg_1 import PROC_EVENT_MODE from csaxs_bec.devices.epics.delay_generator_csaxs.ddg_2 import ( DEFAULT_IO_CONFIG as DDG2_DEFAULT_IO_CONFIG, @@ -272,6 +278,90 @@ def test_ddg1_prepare_mcs(mock_ddg1: DDG1, mock_mcs_csaxs: MCSCardCSAXS): assert st.success is True +def test_ddg1_move_start_latency_default(mock_ddg1: DDG1): + assert mock_ddg1.get_move_start_latency() == DEFAULT_MOVE_START_LATENCY + + +def test_ddg1_move_start_latency_round_trips(mock_ddg1: DDG1): + mock_ddg1.set_move_start_latency(0.25) + assert mock_ddg1.get_move_start_latency() == 0.25 + # zero is the escape hatch back to the timing that preceded the parameter + mock_ddg1.set_move_start_latency(0) + assert mock_ddg1.get_move_start_latency() == 0 + + +@pytest.mark.parametrize("bad", [-0.1, MAX_MOVE_START_LATENCY + 0.1]) +def test_ddg1_move_start_latency_is_bounded(mock_ddg1: DDG1, bad: float): + with pytest.raises(ValueError): + mock_ddg1.set_move_start_latency(bad) + assert mock_ddg1.get_move_start_latency() == DEFAULT_MOVE_START_LATENCY + + +def test_ddg1_move_start_latency_is_reachable_from_the_client(mock_ddg1: DDG1): + """cont_grid reads it over RPC and users tune it over RPC; without these it is + unreachable from either.""" + assert "get_move_start_latency" in DDG1.USER_ACCESS + assert "set_move_start_latency" in DDG1.USER_ACCESS + + +def test_ddg1_move_start_latency_is_independent_of_the_shutter_delay(mock_ddg1: DDG1): + """Two timing constants on the same device with different natures -- one is + hardware (how long the shutter takes to open), one is software (how long a move + takes to reach the motor). Neither may disturb the other.""" + mock_ddg1.set_move_start_latency(0.25) + mock_ddg1.set_shutter_to_open_delay(0.009) + assert mock_ddg1.get_move_start_latency() == 0.25 + mock_ddg1.keep_shutter_open_during_scan(True) + assert mock_ddg1.get_move_start_latency() == 0.25 + + +def test_ddg1_shutter_delay_default(mock_ddg1: DDG1): + assert mock_ddg1.get_shutter_to_open_delay() == DEFAULT_SHUTTER_TO_OPEN_DELAY + + +def test_ddg1_shutter_delay_reaches_the_trigger_channel(mock_ddg1: DDG1): + """The point of the setter: the value must land on `ab`, which gates the + acquisition, while the shutter on `cd` keeps firing at t0.""" + exp_time, frames = 0.1, 10 + mock_ddg1.scan_info.msg.info["exp_time"] = exp_time + mock_ddg1.scan_info.msg.info["frames_per_trigger"] = frames + mock_ddg1.fast_shutter_control._read_pv.mock_data = 0 + + mock_ddg1.set_shutter_to_open_delay(9e-3) + mock_ddg1.stage() + + assert np.isclose(mock_ddg1.ab.delay.get(), 9e-3) + assert np.isclose(mock_ddg1.cd.delay.get(), 0) + assert np.isclose(mock_ddg1.cd.width.get(), 9e-3 + exp_time * frames) + assert np.isclose(mock_ddg1.burst_period.get(), 2 * 9e-3 + exp_time * frames + 3e-6) + mock_ddg1.unstage() + + +@pytest.mark.parametrize("bad", [-1e-3, MAX_SHUTTER_TO_OPEN_DELAY + 1e-3]) +def test_ddg1_shutter_delay_is_bounded(mock_ddg1: DDG1, bad: float): + """A fat-fingered value would stretch every line rather than fail, so it is + refused instead of applied.""" + with pytest.raises(ValueError): + mock_ddg1.set_shutter_to_open_delay(bad) + assert mock_ddg1.get_shutter_to_open_delay() == DEFAULT_SHUTTER_TO_OPEN_DELAY + + +def test_ddg1_shutter_delay_is_reachable_from_the_client(mock_ddg1: DDG1): + """Without this the setter cannot be called over RPC, which is the whole + reason it exists -- the value needs tuning without a redeploy.""" + assert "set_shutter_to_open_delay" in DDG1.USER_ACCESS + + +def test_ddg1_keeping_the_shutter_open_discards_a_tuned_delay(mock_ddg1: DDG1): + """Documents a sharp edge: the two methods write the same attribute, so + toggling this reverts a tuned value to the default rather than restoring it.""" + mock_ddg1.set_shutter_to_open_delay(9e-3) + mock_ddg1.keep_shutter_open_during_scan(True) + assert mock_ddg1.get_shutter_to_open_delay() == 0 + mock_ddg1.keep_shutter_open_during_scan(False) + assert mock_ddg1.get_shutter_to_open_delay() == DEFAULT_SHUTTER_TO_OPEN_DELAY + + def test_ddg1_stage(mock_ddg1: DDG1): """Test the on_stage method of DDG1.""" exp_time = 0.1 @@ -295,7 +385,7 @@ def test_ddg1_stage(mock_ddg1: DDG1): assert np.isclose(mock_ddg1.burst_period.get(), total_exposure) # Trigger DDG2 through EXT/EN - assert np.isclose(mock_ddg1.ab.delay.get(), 2e-3) + assert np.isclose(mock_ddg1.ab.delay.get(), DEFAULT_SHUTTER_TO_OPEN_DELAY) assert np.isclose(mock_ddg1.ab.width.get(), shutter_width) # Shutter channel cd assert np.isclose(mock_ddg1.cd.delay.get(), 0) diff --git a/tests/tests_scans/test_cont_grid_scan.py b/tests/tests_scans/test_cont_grid_scan.py index efeec8d2..e9f04ab9 100644 --- a/tests/tests_scans/test_cont_grid_scan.py +++ b/tests/tests_scans/test_cont_grid_scan.py @@ -65,6 +65,9 @@ def _assemble_cont_grid_scan(v4_scan_assembler, device_manager): signal_read_values={"ddg1": 0.0}, ) custom_ddg1.get_shutter_to_open_delay = mock.MagicMock(return_value=2e-3) + # 0.0 so the hook tests do not spend a real 0.6 s per line; the value itself + # is exercised explicitly further down. + custom_ddg1.get_move_start_latency = mock.MagicMock(return_value=0.0) custom_mcs = MockCustomDevice( "mcs", device_info={ @@ -110,6 +113,58 @@ def test_cont_grid_post_scan_waits_for_completion_and_moves_back_when_relative( assert completion_status.wait_calls == 1 +def test_cont_grid_waits_for_the_move_to_reach_the_motor_before_triggering( + v4_scan_assembler, device_manager, nth_done_status_mock +): + """The burst must not start before the stage does. + + at_each_point issues the line move and then sleeps before firing the trigger. + That sleep used to be acc_time alone, which covers the acceleration ramp and + nothing else -- while the move still has to cross the scan server -> device + server -> EPICS -> controller path first. acc_time shrinks with the scan + velocity; that path does not, so on a slow scan the burst began about 0.6 s + before the stage moved and the first points of every line piled up at the + line start. + """ + scan = _assemble_cont_grid_scan(v4_scan_assembler, device_manager) + scan.device_manager.devices["ddg1"].get_move_start_latency = mock.MagicMock( + return_value=0.35 + ) + scan.prepare_scan() + + scan.actions.set = mock.MagicMock(return_value=nth_done_status_mock(resolve_after=1)) + scan.ddg1 = mock.MagicMock() + # after prepare_scan, so acc_time is still the value the real mock device gave + scan.fast_axis = mock.MagicMock() + scan.actions.read_monitored_devices = mock.MagicMock() + + with mock.patch("csaxs_bec.scans.scans_v4.cont_grid.time.sleep") as sleep: + scan.at_each_point(scan.motors, scan.positions[0], nth_done_status_mock(resolve_after=1)) + + assert scan._cont_motor_params["move_start_latency"] == 0.35 + sleep.assert_called_once_with(0.35 + scan._cont_motor_params["acc_time"]) + + +def test_cont_grid_move_start_latency_of_zero_restores_the_previous_timing( + v4_scan_assembler, device_manager, nth_done_status_mock +): + """The escape hatch: setting the latency to 0 on the device reproduces exactly + what the scan did before this parameter existed, with no redeploy.""" + scan = _assemble_cont_grid_scan(v4_scan_assembler, device_manager) + scan.prepare_scan() + + scan.actions.set = mock.MagicMock(return_value=nth_done_status_mock(resolve_after=1)) + scan.ddg1 = mock.MagicMock() + # after prepare_scan, so acc_time is still the value the real mock device gave + scan.fast_axis = mock.MagicMock() + scan.actions.read_monitored_devices = mock.MagicMock() + + with mock.patch("csaxs_bec.scans.scans_v4.cont_grid.time.sleep") as sleep: + scan.at_each_point(scan.motors, scan.positions[0], nth_done_status_mock(resolve_after=1)) + + sleep.assert_called_once_with(scan._cont_motor_params["acc_time"]) + + 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)