feat(eiger,ddg2): acquire over the gated window, not the whole period
CI for csaxs_bec / test (push) Failing after 5s
CI for csaxs_bec / test (push) Failing after 5s
scan_info's exp_time is the trigger PERIOD. DDG2 gates for exp_time minus a
readout time, but the Eiger was sent the full exp_time as image_time_us, so
the detector acquired 180 us past the falling edge of its own gate and
finished only 20 us before the next trigger -- that 20 us being JungfrauJoch's
internal board-readout allowance, and the entire margin available. Measured on
a scope as a 172.8 us overhang.
Four independent notions of "readout time" existed:
500 us EIGER*_READOUT_TIME_US validation floor only, no effect on anything
200 us DDG2 DEFAULT_READOUT_TIMES["ab"] sets the gate width
20 us JungfrauJoch deployment config applied internally by JFJoch
- scan parameter readout_time honoured by the Falcon, ignored by DDG2
They are now one. DDG2 derives the gap from the scan's readout_time, floored by
its configured value (scans default it to 0), and exposes effective_readout_times().
The Eiger subtracts the same number from image_time_us. The Falcon already used
the scan value, so it needs no change.
The Eiger's 500 us constant is split in two, because it was doing two jobs: a
MIN_EXP_TIME validation floor keeps today's behaviour exactly, while the new
EIGER_READOUT_TIME defaults to 2e-4 to match the delay generator. The _US suffix
on constants holding seconds is dropped in all three modules.
Both devices log the effective exposure window at on_stage, so a period/exposure
mismatch is visible in the logs rather than only on an oscilloscope.
NOTE: this shortens the delivered exposure by the readout time -- 200 us, i.e.
0.5% at 40 ms -- for scans that do not set readout_time. Users should be told
before this is deployed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KLnmUurqcNd1FiDY5M2uZr
This commit is contained in:
1 parent
bb39833d83
commit
77fc2525a0
6 files changed
+185
-25
No files matched your search
@@ -120,7 +120,7 @@ class DDG2(PSIDeviceBase, DelayGeneratorCSAXS):
|
||||
"""
|
||||
|
||||
# Methods exposed to the BEC client, i.e. callable as dev.ddg2.<name>()
|
||||
USER_ACCESS = ["get_readout_times", "set_readout_times"]
|
||||
USER_ACCESS = ["get_readout_times", "set_readout_times", "effective_readout_times"]
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
@@ -144,6 +144,25 @@ class DDG2(PSIDeviceBase, DelayGeneratorCSAXS):
|
||||
if readout_times:
|
||||
self.set_readout_times(**readout_times)
|
||||
|
||||
def effective_readout_times(self) -> dict[str, float]:
|
||||
"""Return the readout times actually in force for the upcoming scan.
|
||||
|
||||
The scan may request a readout time of its own; it wins when it is larger than
|
||||
this device's configured value. Scans default readout_time to 0, so the
|
||||
configured value is the floor rather than a fallback.
|
||||
|
||||
The result defines the exposure window -- period minus gap -- that every
|
||||
detector in the scan is expected to integrate over, whether it reads the gate
|
||||
(Falcon) or runs its own timer from the rising edge (Eiger).
|
||||
|
||||
Returns:
|
||||
dict[str, float]: effective readout time per channel pair, in seconds.
|
||||
"""
|
||||
scan_readout = 0.0
|
||||
if self.scan_parameters is not None:
|
||||
scan_readout = float(getattr(self.scan_parameters, "readout_time", 0.0) or 0.0)
|
||||
return {ch: max(scan_readout, rt) for ch, rt in self._readout_times.items()}
|
||||
|
||||
def get_readout_times(self) -> dict[str, float]:
|
||||
"""Return the readout times per channel pair, in seconds.
|
||||
|
||||
@@ -270,10 +289,14 @@ class DDG2(PSIDeviceBase, DelayGeneratorCSAXS):
|
||||
|
||||
# NOTE Check if the exposure time is longer than all readout times.
|
||||
# Raise a ValueError if requested exposure time is too short.
|
||||
if any(exp_time <= rt for rt in self._readout_times.values()):
|
||||
# NOTE The gap is the scan's readout_time when the scan asks for one, otherwise this
|
||||
# device's configured value. Scans default readout_time to 0, hence the max().
|
||||
# Every detector must derive its exposure window from the same number -- see
|
||||
# effective_readout_times().
|
||||
readouts = self.effective_readout_times()
|
||||
if any(exp_time <= rt for rt in readouts.values()):
|
||||
raise ValueError(
|
||||
f"Exposure time {exp_time} is too short for the readout times "
|
||||
f"{self._readout_times}"
|
||||
f"Exposure time {exp_time} is too short for the readout times {readouts}"
|
||||
)
|
||||
|
||||
#########################################
|
||||
@@ -288,13 +311,11 @@ class DDG2(PSIDeviceBase, DelayGeneratorCSAXS):
|
||||
|
||||
# Calculate the pulse width for the channel pair 'ab'. The readout time is the
|
||||
# gap left between consecutive triggers; see set_readout_times().
|
||||
burst_pulse_width = exp_time - self._readout_times["ab"]
|
||||
if self._readout_times["ab"] != DEFAULT_READOUT_TIMES["ab"]:
|
||||
logger.info(
|
||||
f"DDG {self.name} using non-default readout time "
|
||||
f"{self._readout_times['ab']}s for 'ab' (default "
|
||||
f"{DEFAULT_READOUT_TIMES['ab']}s); pulse width {burst_pulse_width}s"
|
||||
)
|
||||
burst_pulse_width = exp_time - readouts["ab"]
|
||||
logger.info(
|
||||
f"DDG {self.name} exposure window: period {exp_time}s, gap {readouts['ab']}s, "
|
||||
f"pulse width {burst_pulse_width}s"
|
||||
)
|
||||
|
||||
# Trigger detectors with delay 0, and pulse width = exp_time - readout_time
|
||||
self.set_delay_pairs(channel="ab", delay=0, width=burst_pulse_width)
|
||||
|
||||
@@ -8,10 +8,15 @@ A few notes on setup and operation of the Eiger detectors through the JungfrauJo
|
||||
- Changes in energy may take time, good to implement logic that only resets energy if needed.
|
||||
- For the Eiger, the frame_time_us in DetectorSettings is ignored, only the frame_time_us in
|
||||
the DatasetSettings is relevant
|
||||
- The bit_depth will be adjusted automatically based on the exp_time. Here, we need to ensure
|
||||
that subsequent triggers properly consider the readout_time of the boards. For the Eiger detectors
|
||||
at cSAXS, a readout time of 20us is configured through the JungfrauJoch deployment config. This
|
||||
setting is sufficiently large for the detectors if they run in parallel mode.
|
||||
- The bit_depth will be adjusted automatically based on the exp_time (32 bit above 2622 us),
|
||||
and in 32 bit the detector accumulates ~2.3 ms subframes. JungfrauJoch's own deployment config
|
||||
budgets 20 us of board readout between frames, which is right for 16 bit parallel mode but far
|
||||
smaller than the subframe granularity -- so a frame can outlast the trigger period without
|
||||
anything reporting an error.
|
||||
- scan_info's exp_time is the trigger PERIOD, not the exposure. The delay generator gates for
|
||||
exp_time - readout_time, and on_stage sends that same window as image_time_us so the detector
|
||||
is not still acquiring when the next trigger arrives. The readout time must agree with DDG2's
|
||||
DEFAULT_READOUT_TIMES["ab"]; a scan-level readout_time overrides both when larger.
|
||||
- beam_center and detector settings are required input arguments, thus, they may be set to wrong
|
||||
values for acquisitions to start. Please keep this in mind.
|
||||
|
||||
@@ -65,7 +70,14 @@ if TYPE_CHECKING: # pragma no cover
|
||||
|
||||
logger = bec_logger.logger
|
||||
|
||||
EIGER_READOUT_TIME_US = 500e-6 # 500 microseconds in s
|
||||
# NOTE Two distinct numbers that used to be one. MIN_EXP_TIME is a validation floor
|
||||
# (its value is inherited from the former EIGER_READOUT_TIME_US and has no documented
|
||||
# provenance). EIGER_READOUT_TIME is the gap left between the end of one acquisition and
|
||||
# the next trigger; it must match the gap the delay generator leaves in the pulse train,
|
||||
# DDG2's DEFAULT_READOUT_TIMES["ab"], or the detector is told to acquire for longer than
|
||||
# the trigger period allows.
|
||||
MIN_EXP_TIME = 500e-6 # s
|
||||
EIGER_READOUT_TIME = 2e-4 # s -- matches DDG2 DEFAULT_READOUT_TIMES["ab"]
|
||||
|
||||
|
||||
class EigerError(Exception):
|
||||
@@ -106,7 +118,7 @@ class Eiger(PSIDeviceBase):
|
||||
detector_distance: float = 100.0,
|
||||
beam_center: tuple[int, int] = (0, 0),
|
||||
scan_info: ScanInfo = None,
|
||||
readout_time: float = EIGER_READOUT_TIME_US,
|
||||
readout_time: float = EIGER_READOUT_TIME,
|
||||
device_manager=None,
|
||||
**kwargs,
|
||||
):
|
||||
@@ -344,9 +356,21 @@ class Eiger(PSIDeviceBase):
|
||||
|
||||
# Setting up exp_time and num_triggers acquisition parameter
|
||||
exp_time = self.scan_parameters.exp_time
|
||||
if exp_time <= self._readout_time: # Exp_time must be at least the readout time
|
||||
if exp_time <= MIN_EXP_TIME:
|
||||
raise ValueError(
|
||||
f"Value error on device {self.name}: Exposure time {exp_time}s is less than readout time {self._readout_time}s."
|
||||
f"Value error on device {self.name}: Exposure time {exp_time}s is below the "
|
||||
f"minimum exposure time {MIN_EXP_TIME}s."
|
||||
)
|
||||
# NOTE exp_time is the trigger PERIOD, not the exposure. The delay generator gates
|
||||
# for exp_time - readout_time, so the detector must acquire for the same window or
|
||||
# it is still busy when the next trigger arrives. Scans default readout_time to 0,
|
||||
# hence the max() against this device's configured value.
|
||||
readout_time = max(self.scan_parameters.readout_time, self._readout_time)
|
||||
acquisition_time = exp_time - readout_time
|
||||
if acquisition_time <= 0:
|
||||
raise ValueError(
|
||||
f"Value error on device {self.name}: readout time {readout_time}s leaves no "
|
||||
f"acquisition time within the trigger period {exp_time}s."
|
||||
)
|
||||
self._num_triggers = int(
|
||||
self.scan_parameters.num_points * self.scan_parameters.frames_per_trigger
|
||||
@@ -369,7 +393,7 @@ class Eiger(PSIDeviceBase):
|
||||
|
||||
# Create dataset settings for API call.
|
||||
data_settings = DatasetSettings(
|
||||
image_time_us=int(exp_time * 1e6),
|
||||
image_time_us=int(acquisition_time * 1e6),
|
||||
ntrigger=self._num_triggers,
|
||||
file_prefix=path,
|
||||
beam_x_pxl=float(self._beam_center[0]),
|
||||
@@ -379,6 +403,10 @@ class Eiger(PSIDeviceBase):
|
||||
)
|
||||
logger.debug(f"Setting data_settings: {yaml.dump(data_settings.to_dict(), indent=4)}")
|
||||
prep_time = time.time()
|
||||
logger.info(
|
||||
f"{self.name} exposure window: period {exp_time}s, readout {readout_time}s, "
|
||||
f"acquisition {acquisition_time}s"
|
||||
)
|
||||
self.jfj_client.wait_for_idle(timeout=10) # Ensure we are in IDLE state
|
||||
self.jfj_client.start(settings=data_settings) # Takes around ~0.6s -> 6s currently.
|
||||
|
||||
|
||||
@@ -11,7 +11,9 @@ from typing import TYPE_CHECKING
|
||||
|
||||
from csaxs_bec.devices.jungfraujoch.eiger import Eiger
|
||||
|
||||
EIGER1_5M_READOUT_TIME_US = 500e-6 # 500 microseconds in s
|
||||
# Gap between the end of an acquisition and the next trigger; must match the
|
||||
# delay generator's gap (DDG2 DEFAULT_READOUT_TIMES["ab"]).
|
||||
EIGER1_5M_READOUT_TIME = 2e-4 # s
|
||||
DETECTOR_NAME = "EIGER 1.5M"
|
||||
|
||||
|
||||
@@ -45,7 +47,7 @@ class Eiger1_5M(Eiger):
|
||||
super().__init__(
|
||||
name=name,
|
||||
detector_name=DETECTOR_NAME,
|
||||
readout_time=EIGER1_5M_READOUT_TIME_US,
|
||||
readout_time=EIGER1_5M_READOUT_TIME,
|
||||
detector_distance=detector_distance,
|
||||
beam_center=beam_center,
|
||||
scan_info=scan_info,
|
||||
|
||||
@@ -20,7 +20,9 @@ if TYPE_CHECKING: # pragma no cover
|
||||
from bec_lib.devicemanager import ScanInfo
|
||||
from bec_server.device_server.device_server import DeviceManagerDS
|
||||
|
||||
EIGER9M_READOUT_TIME_US = 500e-6 # 500 microseconds in s
|
||||
# Gap between the end of an acquisition and the next trigger; must match the
|
||||
# delay generator's gap (DDG2 DEFAULT_READOUT_TIMES["ab"]).
|
||||
EIGER9M_READOUT_TIME = 2e-4 # s
|
||||
DETECTOR_NAME = "EIGER 9M" # "EIGER 9M""
|
||||
|
||||
|
||||
@@ -49,7 +51,7 @@ class Eiger9M(Eiger):
|
||||
super().__init__(
|
||||
name=name,
|
||||
detector_name=DETECTOR_NAME,
|
||||
readout_time=EIGER9M_READOUT_TIME_US,
|
||||
readout_time=EIGER9M_READOUT_TIME,
|
||||
detector_distance=detector_distance,
|
||||
beam_center=beam_center,
|
||||
scan_info=scan_info,
|
||||
|
||||
@@ -675,3 +675,33 @@ def test_ddg2_too_long_readout_time_is_caught_at_stage(mock_ddg2: DDG2):
|
||||
ddg.set_readout_times(ab=3e-3)
|
||||
with pytest.raises(ValueError):
|
||||
ddg.stage()
|
||||
|
||||
|
||||
def test_ddg2_effective_readout_uses_the_scan_value_when_larger(mock_ddg2: DDG2):
|
||||
"""A scan-level readout_time widens the gap; the configured value is the floor."""
|
||||
ddg = mock_ddg2
|
||||
exp_time = 0.1
|
||||
ddg.on_connected()
|
||||
ddg.scan_info.msg.info["exp_time"] = exp_time
|
||||
ddg.scan_info.msg.info["frames_per_trigger"] = 1
|
||||
ddg.scan_info.msg.info["readout_time"] = 3e-3
|
||||
|
||||
ddg.stage()
|
||||
assert ddg.effective_readout_times()["ab"] == 3e-3
|
||||
assert np.isclose(ddg.ab.width.get(), exp_time - 3e-3)
|
||||
ddg.unstage()
|
||||
|
||||
|
||||
def test_ddg2_effective_readout_falls_back_to_the_configured_value(mock_ddg2: DDG2):
|
||||
"""Scans default readout_time to 0, so the device value must still apply."""
|
||||
ddg = mock_ddg2
|
||||
exp_time = 0.1
|
||||
ddg.on_connected()
|
||||
ddg.scan_info.msg.info["exp_time"] = exp_time
|
||||
ddg.scan_info.msg.info["frames_per_trigger"] = 1
|
||||
ddg.scan_info.msg.info["readout_time"] = 0.0
|
||||
|
||||
ddg.stage()
|
||||
assert ddg.effective_readout_times()["ab"] == DDG2_DEFAULT_READOUT_TIMES["ab"]
|
||||
assert np.isclose(ddg.ab.width.get(), exp_time - DDG2_DEFAULT_READOUT_TIMES["ab"])
|
||||
ddg.unstage()
|
||||
@@ -22,6 +22,7 @@ from ophyd import Staged
|
||||
from ophyd_devices.utils.psi_device_base_utils import DeviceStatus
|
||||
|
||||
from csaxs_bec.devices.jungfraujoch.eiger import EigerError
|
||||
from csaxs_bec.devices.jungfraujoch.eiger import EIGER_READOUT_TIME, MIN_EXP_TIME
|
||||
from csaxs_bec.devices.jungfraujoch.eiger_1_5m import Eiger1_5M
|
||||
from csaxs_bec.devices.jungfraujoch.eiger_9m import Eiger9M
|
||||
|
||||
@@ -416,7 +417,8 @@ def test_eiger_on_stage(eiger_1_5m):
|
||||
assert file_msg.hinted_h5_entries == {"data": "entry/data/data"}
|
||||
|
||||
data_settings = DatasetSettings(
|
||||
image_time_us=int(scan_msg.scan_parameters["exp_time"] * 1e6),
|
||||
# exp_time is the trigger period; the detector acquires for the gated window
|
||||
image_time_us=int((scan_msg.scan_parameters["exp_time"] - EIGER_READOUT_TIME) * 1e6),
|
||||
ntrigger=int(scan_msg.num_points * scan_msg.scan_parameters["frames_per_trigger"]),
|
||||
file_prefix=os.path.relpath(eiger._full_path, start="/sls/x12sa/data").removesuffix(
|
||||
"_master.h5"
|
||||
@@ -460,3 +462,78 @@ def test_eiger_preview_callback(eiger_1_5m):
|
||||
mock_preview_put.assert_called_once_with(msg["data"]["default"])
|
||||
else:
|
||||
mock_preview_put.assert_not_called()
|
||||
|
||||
|
||||
def test_eiger_acquisition_matches_the_gated_window(eiger_1_5m):
|
||||
"""The detector must acquire for exp_time - readout, not for the whole period.
|
||||
|
||||
exp_time is the trigger period. The delay generator gates for exp_time - readout, so
|
||||
sending the full period leaves the detector still acquiring when the next trigger
|
||||
arrives -- 20 us of margin, which is what JungfrauJoch subtracts internally.
|
||||
"""
|
||||
eiger = eiger_1_5m
|
||||
exp_time = eiger.scan_info.msg.scan_parameters["exp_time"]
|
||||
|
||||
with (
|
||||
mock.patch.object(eiger.jfj_client, "wait_for_idle", return_value=True),
|
||||
mock.patch.object(eiger.jfj_client, "start") as mock_start,
|
||||
mock.patch.object(
|
||||
eiger.jfj_client.api, "status_get", return_value=BrokerStatus(state="Idle")
|
||||
),
|
||||
):
|
||||
eiger.stage()
|
||||
|
||||
sent = mock_start.call_args.kwargs["settings"].image_time_us
|
||||
assert sent == int((exp_time - EIGER_READOUT_TIME) * 1e6)
|
||||
assert sent < int(exp_time * 1e6)
|
||||
|
||||
|
||||
def test_eiger_scan_readout_time_wins_when_larger(eiger_1_5m):
|
||||
"""A scan asking for a wider gap must widen the acquisition window too."""
|
||||
eiger = eiger_1_5m
|
||||
exp_time = eiger.scan_info.msg.scan_parameters["exp_time"]
|
||||
eiger.scan_info.msg.info["readout_time"] = 3e-3 # > EIGER_READOUT_TIME
|
||||
|
||||
with (
|
||||
mock.patch.object(eiger.jfj_client, "wait_for_idle", return_value=True),
|
||||
mock.patch.object(eiger.jfj_client, "start") as mock_start,
|
||||
mock.patch.object(
|
||||
eiger.jfj_client.api, "status_get", return_value=BrokerStatus(state="Idle")
|
||||
),
|
||||
):
|
||||
eiger.stage()
|
||||
|
||||
assert mock_start.call_args.kwargs["settings"].image_time_us == int((exp_time - 3e-3) * 1e6)
|
||||
|
||||
|
||||
def test_eiger_rejects_a_readout_that_fills_the_period(eiger_1_5m):
|
||||
"""A gap wider than the period leaves nothing to acquire; fail rather than send <= 0."""
|
||||
eiger = eiger_1_5m
|
||||
exp_time = eiger.scan_info.msg.scan_parameters["exp_time"]
|
||||
eiger.scan_info.msg.info["readout_time"] = exp_time + 1e-3
|
||||
|
||||
with (
|
||||
mock.patch.object(eiger.jfj_client, "wait_for_idle", return_value=True),
|
||||
mock.patch.object(eiger.jfj_client, "start"),
|
||||
mock.patch.object(
|
||||
eiger.jfj_client.api, "status_get", return_value=BrokerStatus(state="Idle")
|
||||
),
|
||||
):
|
||||
with pytest.raises(ValueError):
|
||||
eiger.stage()
|
||||
|
||||
|
||||
def test_eiger_minimum_exposure_check_is_unchanged(eiger_1_5m):
|
||||
"""Splitting readout from the validation floor must not relax the minimum exposure."""
|
||||
eiger = eiger_1_5m
|
||||
eiger.scan_info.msg.info["exp_time"] = MIN_EXP_TIME / 2
|
||||
|
||||
with (
|
||||
mock.patch.object(eiger.jfj_client, "wait_for_idle", return_value=True),
|
||||
mock.patch.object(eiger.jfj_client, "start"),
|
||||
mock.patch.object(
|
||||
eiger.jfj_client.api, "status_get", return_value=BrokerStatus(state="Idle")
|
||||
),
|
||||
):
|
||||
with pytest.raises(ValueError):
|
||||
eiger.stage()
|
||||
Reference in new issue
Block a user