From 3412ac86e2aac6950ffba28985a735e59194d538 Mon Sep 17 00:00:00 2001 From: Jakub Wlodek Date: Thu, 19 Sep 2024 09:34:14 -0400 Subject: [PATCH 1/9] Fix typo (#588) --- tests/epics/demo/test_demo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/epics/demo/test_demo.py b/tests/epics/demo/test_demo.py index f488412f27..82039c661e 100644 --- a/tests/epics/demo/test_demo.py +++ b/tests/epics/demo/test_demo.py @@ -228,7 +228,7 @@ async def test_set_velocity(mock_mover: demo.Mover) -> None: assert q.empty() -async def test_mover_disconncted(): +async def test_mover_disconnected(): with pytest.raises(NotConnected): async with DeviceCollector(timeout=0.1): m = demo.Mover("ca://PRE:", name="mover") From de084df871f83f3dd3d0a06e47021ee310ccba69 Mon Sep 17 00:00:00 2001 From: Jakub Wlodek Date: Thu, 19 Sep 2024 09:34:41 -0400 Subject: [PATCH 2/9] Use name provider instead of hdf name when calling path provider in ad HDF writer (#587) --- src/ophyd_async/epics/adcore/_hdf_writer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ophyd_async/epics/adcore/_hdf_writer.py b/src/ophyd_async/epics/adcore/_hdf_writer.py index 37ee4e6d72..b9034eded0 100644 --- a/src/ophyd_async/epics/adcore/_hdf_writer.py +++ b/src/ophyd_async/epics/adcore/_hdf_writer.py @@ -50,7 +50,7 @@ def __init__( async def open(self, multiplier: int = 1) -> dict[str, DataKey]: self._file = None - info = self._path_provider(device_name=self.hdf.name) + info = self._path_provider(device_name=self._name_provider()) # Set the directory creation depth first, since dir creation callback happens # when directory path PV is processed. From 0b6ab214001abd1276f5bf33e64382f980cb9b40 Mon Sep 17 00:00:00 2001 From: Jakub Wlodek Date: Thu, 19 Sep 2024 09:35:54 -0400 Subject: [PATCH 3/9] Add asyncio_default_fixture_loop_scope variable to pytest.ini to avoid deprecation warning (#584) --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index a9b8fc141b..23afbf19bc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -109,6 +109,7 @@ markers = [ "adsim: require the ADsim IOC to be running", ] asyncio_mode = "auto" +asyncio_default_fixture_loop_scope = "function" [tool.coverage.run] data_file = "/tmp/ophyd_async.coverage" From 35c07fee0a3263726cb39c52324b4ac3c728a345 Mon Sep 17 00:00:00 2001 From: Jakub Wlodek Date: Thu, 19 Sep 2024 09:42:33 -0400 Subject: [PATCH 4/9] Include chunk shape as a parameter in stream resource for HDF dataset (#544) * Adding record for num frames in chunk along with chunk_size field in desc * Attributes are saved all in a single chunk * Update tests to account for chunk_size datakey parameter * Chunk size should be in sres not desc * Move chunk size to sres parameters * Refactor tests to reflect changes * chunk size can be int or none Co-authored-by: Eugene * Update chunk size signal to non-zero in one of the AD test sets * Use correct chunk size for PandA, make sure we use chunk size auto * Add comment on chunk size * Make chunk_size a tuple that explicitly describes all chunk dims * Make sure chunk size is tuple even with one dim, update tests, simplify ad standard det tests * Make chunk_size always tuple of int, default to empty tuple * Use readback value to avoid disconnect between actual value and signal get * Follow import convention for tests * Make use of slicing for detector name in ad_standard_det_factory clearer * Rename chunk size to chunk shape * Add space for linting * Fix test * Fix merge conflict * Simplifying ad standard det factory fixture * Fix unawaited task issue * kinetix fixture doesn't need to be async --------- Co-authored-by: Eugene --- src/ophyd_async/core/_hdf_dataset.py | 3 ++ src/ophyd_async/epics/adcore/_core_io.py | 2 ++ src/ophyd_async/epics/adcore/_hdf_writer.py | 10 ++++++ src/ophyd_async/fastcs/panda/_writer.py | 6 +++- tests/epics/adaravis/test_aravis.py | 29 ++++++--------- tests/epics/adcore/test_writers.py | 3 ++ tests/epics/adkinetix/test_kinetix.py | 32 +++++++---------- tests/epics/advimba/test_vimba.py | 40 ++++++++++----------- tests/epics/conftest.py | 38 ++++++++++++++++++++ tests/fastcs/panda/test_hdf_panda.py | 2 ++ tests/fastcs/panda/test_writer.py | 7 +++- 11 files changed, 112 insertions(+), 60 deletions(-) create mode 100644 tests/epics/conftest.py diff --git a/src/ophyd_async/core/_hdf_dataset.py b/src/ophyd_async/core/_hdf_dataset.py index 5199eab8cf..79cb9c432a 100644 --- a/src/ophyd_async/core/_hdf_dataset.py +++ b/src/ophyd_async/core/_hdf_dataset.py @@ -20,6 +20,8 @@ class HDFDataset: dtype_numpy: str = "" multiplier: int = 1 swmr: bool = False + # Represents explicit chunk size written to disk. + chunk_shape: tuple[int, ...] = () SLICE_NAME = "AD_HDF5_SWMR_SLICE" @@ -66,6 +68,7 @@ def __init__( "dataset": ds.dataset, "swmr": ds.swmr, "multiplier": ds.multiplier, + "chunk_shape": ds.chunk_shape, }, uid=None, validate=True, diff --git a/src/ophyd_async/epics/adcore/_core_io.py b/src/ophyd_async/epics/adcore/_core_io.py index f15d48cd2e..7968579117 100644 --- a/src/ophyd_async/epics/adcore/_core_io.py +++ b/src/ophyd_async/epics/adcore/_core_io.py @@ -135,4 +135,6 @@ def __init__(self, prefix: str, name="") -> None: self.array_size0 = epics_signal_r(int, prefix + "ArraySize0") self.array_size1 = epics_signal_r(int, prefix + "ArraySize1") self.create_directory = epics_signal_rw(int, prefix + "CreateDirectory") + self.num_frames_chunks = epics_signal_r(int, prefix + "NumFramesChunks_RBV") + self.chunk_size_auto = epics_signal_rw_rbv(bool, prefix + "ChunkSizeAuto") super().__init__(prefix, name) diff --git a/src/ophyd_async/epics/adcore/_hdf_writer.py b/src/ophyd_async/epics/adcore/_hdf_writer.py index b9034eded0..bfffa67b89 100644 --- a/src/ophyd_async/epics/adcore/_hdf_writer.py +++ b/src/ophyd_async/epics/adcore/_hdf_writer.py @@ -56,6 +56,9 @@ async def open(self, multiplier: int = 1) -> dict[str, DataKey]: # when directory path PV is processed. await self.hdf.create_directory.set(info.create_dir_depth) + # Make sure we are using chunk auto-sizing + await asyncio.gather(self.hdf.chunk_size_auto.set(True)) + await asyncio.gather( self.hdf.num_extra_dims.set(0), self.hdf.lazy_open.set(True), @@ -84,6 +87,9 @@ async def open(self, multiplier: int = 1) -> dict[str, DataKey]: self._multiplier = multiplier outer_shape = (multiplier,) if multiplier > 1 else () + # Determine number of frames that will be saved per HDF chunk + frames_per_chunk = await self.hdf.num_frames_chunks.get_value() + # Add the main data self._datasets = [ HDFDataset( @@ -92,6 +98,7 @@ async def open(self, multiplier: int = 1) -> dict[str, DataKey]: shape=detector_shape, dtype_numpy=np_dtype, multiplier=multiplier, + chunk_shape=(frames_per_chunk, *detector_shape), ) ] # And all the scalar datasets @@ -118,6 +125,9 @@ async def open(self, multiplier: int = 1) -> dict[str, DataKey]: (), np_datatype, multiplier, + # NDAttributes appear to always be configured with + # this chunk size + chunk_shape=(16384,), ) ) diff --git a/src/ophyd_async/fastcs/panda/_writer.py b/src/ophyd_async/fastcs/panda/_writer.py index 5a1fbe7063..100af8b10e 100644 --- a/src/ophyd_async/fastcs/panda/_writer.py +++ b/src/ophyd_async/fastcs/panda/_writer.py @@ -105,7 +105,11 @@ async def _update_datasets(self) -> None: capture_table = await self.panda_data_block.datasets.get_value() self._datasets = [ - HDFDataset(dataset_name, "/" + dataset_name, [1], multiplier=1) + # TODO: Update chunk size to read signal once available in IOC + # Currently PandA IOC sets chunk size to 1024 points per chunk + HDFDataset( + dataset_name, "/" + dataset_name, [1], multiplier=1, chunk_shape=(1024,) + ) for dataset_name in capture_table["name"] ] diff --git a/tests/epics/adaravis/test_aravis.py b/tests/epics/adaravis/test_aravis.py index 3c34fa49eb..341ad280ed 100644 --- a/tests/epics/adaravis/test_aravis.py +++ b/tests/epics/adaravis/test_aravis.py @@ -1,11 +1,9 @@ import re import pytest -from bluesky.run_engine import RunEngine from ophyd_async.core import ( DetectorTrigger, - DeviceCollector, PathProvider, TriggerInfo, set_mock_value, @@ -14,14 +12,8 @@ @pytest.fixture -async def test_adaravis( - RE: RunEngine, - static_path_provider: PathProvider, -) -> adaravis.AravisDetector: - async with DeviceCollector(mock=True): - test_adaravis = adaravis.AravisDetector("ADARAVIS:", static_path_provider) - - return test_adaravis +def test_adaravis(ad_standard_det_factory) -> adaravis.AravisDetector: + return ad_standard_det_factory(adaravis.AravisDetector) @pytest.mark.parametrize("exposure_time", [0.0, 0.1, 1.0, 10.0, 100.0]) @@ -80,7 +72,7 @@ def test_gpio_pin_limited(test_adaravis: adaravis.AravisDetector): async def test_hints_from_hdf_writer(test_adaravis: adaravis.AravisDetector): - assert test_adaravis.hints == {"fields": ["test_adaravis"]} + assert test_adaravis.hints == {"fields": ["test_adaravis1"]} async def test_can_read(test_adaravis: adaravis.AravisDetector): @@ -98,9 +90,9 @@ async def test_decribe_describes_writer_dataset( await test_adaravis.stage() await test_adaravis.prepare(one_shot_trigger_info) assert await test_adaravis.describe() == { - "test_adaravis": { - "source": "mock+ca://ADARAVIS:HDF1:FullFileName_RBV", - "shape": (0, 0), + "test_adaravis1": { + "source": "mock+ca://ARAVIS1:HDF1:FullFileName_RBV", + "shape": (10, 10), "dtype": "array", "dtype_numpy": "|i1", "external": "STREAM:", @@ -125,12 +117,13 @@ async def test_can_collect( assert docs[0][0] == "stream_resource" stream_resource = docs[0][1] sr_uid = stream_resource["uid"] - assert stream_resource["data_key"] == "test_adaravis" + assert stream_resource["data_key"] == "test_adaravis1" assert stream_resource["uri"] == "file://localhost" + str(full_file_name) assert stream_resource["parameters"] == { "dataset": "/entry/data/data", "swmr": False, "multiplier": 1, + "chunk_shape": (1, 10, 10), } assert docs[1][0] == "stream_datum" stream_datum = docs[1][1] @@ -148,9 +141,9 @@ async def test_can_decribe_collect( await test_adaravis.stage() await test_adaravis.prepare(one_shot_trigger_info) assert (await test_adaravis.describe_collect()) == { - "test_adaravis": { - "source": "mock+ca://ADARAVIS:HDF1:FullFileName_RBV", - "shape": (0, 0), + "test_adaravis1": { + "source": "mock+ca://ARAVIS1:HDF1:FullFileName_RBV", + "shape": (10, 10), "dtype": "array", "dtype_numpy": "|i1", "external": "STREAM:", diff --git a/tests/epics/adcore/test_writers.py b/tests/epics/adcore/test_writers.py index ec729d1052..af32f86667 100644 --- a/tests/epics/adcore/test_writers.py +++ b/tests/epics/adcore/test_writers.py @@ -46,6 +46,9 @@ async def hdf_writer_with_stats( hdf = adcore.NDFileHDFIO("HDF:") stats = adcore.NDPluginStatsIO("FOO:") + # Set number of frames per chunk to something reasonable + set_mock_value(hdf.num_frames_chunks, 2) + return adcore.ADHDFWriter( hdf, static_path_provider, diff --git a/tests/epics/adkinetix/test_kinetix.py b/tests/epics/adkinetix/test_kinetix.py index a17be5e5b3..ae2f72462e 100644 --- a/tests/epics/adkinetix/test_kinetix.py +++ b/tests/epics/adkinetix/test_kinetix.py @@ -1,25 +1,18 @@ import pytest -from bluesky.run_engine import RunEngine from ophyd_async.core import ( DetectorTrigger, - DeviceCollector, StaticPathProvider, set_mock_value, ) from ophyd_async.core._detector import TriggerInfo from ophyd_async.epics import adkinetix +from ophyd_async.epics.adkinetix._kinetix_io import KinetixTriggerMode @pytest.fixture -async def test_adkinetix( - RE: RunEngine, - static_path_provider: StaticPathProvider, -) -> adkinetix.KinetixDetector: - async with DeviceCollector(mock=True): - test_adkinetix = adkinetix.KinetixDetector("KINETIX:", static_path_provider) - - return test_adkinetix +def test_adkinetix(ad_standard_det_factory): + return ad_standard_det_factory(adkinetix.KinetixDetector) async def test_get_deadtime( @@ -30,7 +23,7 @@ async def test_get_deadtime( async def test_trigger_modes(test_adkinetix: adkinetix.KinetixDetector): - set_mock_value(test_adkinetix.drv.trigger_mode, "Internal") + set_mock_value(test_adkinetix.drv.trigger_mode, KinetixTriggerMode.internal) async def setup_trigger_mode(trig_mode: DetectorTrigger): await test_adkinetix.controller.prepare( @@ -58,7 +51,7 @@ async def setup_trigger_mode(trig_mode: DetectorTrigger): async def test_hints_from_hdf_writer(test_adkinetix: adkinetix.KinetixDetector): - assert test_adkinetix.hints == {"fields": ["test_adkinetix"]} + assert test_adkinetix.hints == {"fields": ["test_adkinetix1"]} async def test_can_read(test_adkinetix: adkinetix.KinetixDetector): @@ -76,9 +69,9 @@ async def test_decribe_describes_writer_dataset( await test_adkinetix.stage() await test_adkinetix.prepare(one_shot_trigger_info) assert await test_adkinetix.describe() == { - "test_adkinetix": { - "source": "mock+ca://KINETIX:HDF1:FullFileName_RBV", - "shape": (0, 0), + "test_adkinetix1": { + "source": "mock+ca://KINETIX1:HDF1:FullFileName_RBV", + "shape": (10, 10), "dtype": "array", "dtype_numpy": "|i1", "external": "STREAM:", @@ -103,12 +96,13 @@ async def test_can_collect( assert docs[0][0] == "stream_resource" stream_resource = docs[0][1] sr_uid = stream_resource["uid"] - assert stream_resource["data_key"] == "test_adkinetix" + assert stream_resource["data_key"] == "test_adkinetix1" assert stream_resource["uri"] == "file://localhost" + str(full_file_name) assert stream_resource["parameters"] == { "dataset": "/entry/data/data", "swmr": False, "multiplier": 1, + "chunk_shape": (1, 10, 10), } assert docs[1][0] == "stream_datum" stream_datum = docs[1][1] @@ -126,9 +120,9 @@ async def test_can_decribe_collect( await test_adkinetix.stage() await test_adkinetix.prepare(one_shot_trigger_info) assert (await test_adkinetix.describe_collect()) == { - "test_adkinetix": { - "source": "mock+ca://KINETIX:HDF1:FullFileName_RBV", - "shape": (0, 0), + "test_adkinetix1": { + "source": "mock+ca://KINETIX1:HDF1:FullFileName_RBV", + "shape": (10, 10), "dtype": "array", "dtype_numpy": "|i1", "external": "STREAM:", diff --git a/tests/epics/advimba/test_vimba.py b/tests/epics/advimba/test_vimba.py index ec93cc07d3..a8990502c3 100644 --- a/tests/epics/advimba/test_vimba.py +++ b/tests/epics/advimba/test_vimba.py @@ -1,25 +1,22 @@ import pytest -from bluesky.run_engine import RunEngine from ophyd_async.core import ( DetectorTrigger, - DeviceCollector, PathProvider, set_mock_value, ) from ophyd_async.core._detector import TriggerInfo from ophyd_async.epics import advimba +from ophyd_async.epics.advimba._vimba_io import ( + VimbaExposeOutMode, + VimbaOnOff, + VimbaTriggerSource, +) @pytest.fixture -async def test_advimba( - RE: RunEngine, - static_path_provider: PathProvider, -) -> advimba.VimbaDetector: - async with DeviceCollector(mock=True): - test_advimba = advimba.VimbaDetector("VIMBA:", static_path_provider) - - return test_advimba +def test_advimba(ad_standard_det_factory) -> advimba.VimbaDetector: + return ad_standard_det_factory(advimba.VimbaDetector) async def test_get_deadtime( @@ -30,9 +27,9 @@ async def test_get_deadtime( async def test_arming_trig_modes(test_advimba: advimba.VimbaDetector): - set_mock_value(test_advimba.drv.trigger_source, "Freerun") - set_mock_value(test_advimba.drv.trigger_mode, "Off") - set_mock_value(test_advimba.drv.exposure_mode, "Timed") + set_mock_value(test_advimba.drv.trigger_source, VimbaTriggerSource.freerun) + set_mock_value(test_advimba.drv.trigger_mode, VimbaOnOff.off) + set_mock_value(test_advimba.drv.exposure_mode, VimbaExposeOutMode.timed) async def setup_trigger_mode(trig_mode: DetectorTrigger): await test_advimba.controller.prepare(TriggerInfo(number=1, trigger=trig_mode)) @@ -68,7 +65,7 @@ async def setup_trigger_mode(trig_mode: DetectorTrigger): async def test_hints_from_hdf_writer(test_advimba: advimba.VimbaDetector): - assert test_advimba.hints == {"fields": ["test_advimba"]} + assert test_advimba.hints == {"fields": ["test_advimba1"]} async def test_can_read(test_advimba: advimba.VimbaDetector): @@ -86,9 +83,9 @@ async def test_decribe_describes_writer_dataset( await test_advimba.stage() await test_advimba.prepare(one_shot_trigger_info) assert await test_advimba.describe() == { - "test_advimba": { - "source": "mock+ca://VIMBA:HDF1:FullFileName_RBV", - "shape": (0, 0), + "test_advimba1": { + "source": "mock+ca://VIMBA1:HDF1:FullFileName_RBV", + "shape": (10, 10), "dtype": "array", "dtype_numpy": "|i1", "external": "STREAM:", @@ -113,12 +110,13 @@ async def test_can_collect( assert docs[0][0] == "stream_resource" stream_resource = docs[0][1] sr_uid = stream_resource["uid"] - assert stream_resource["data_key"] == "test_advimba" + assert stream_resource["data_key"] == "test_advimba1" assert stream_resource["uri"] == "file://localhost" + str(full_file_name) assert stream_resource["parameters"] == { "dataset": "/entry/data/data", "swmr": False, "multiplier": 1, + "chunk_shape": (1, 10, 10), } assert docs[1][0] == "stream_datum" stream_datum = docs[1][1] @@ -136,9 +134,9 @@ async def test_can_decribe_collect( await test_advimba.stage() await test_advimba.prepare(one_shot_trigger_info) assert (await test_advimba.describe_collect()) == { - "test_advimba": { - "source": "mock+ca://VIMBA:HDF1:FullFileName_RBV", - "shape": (0, 0), + "test_advimba1": { + "source": "mock+ca://VIMBA1:HDF1:FullFileName_RBV", + "shape": (10, 10), "dtype": "array", "dtype_numpy": "|i1", "external": "STREAM:", diff --git a/tests/epics/conftest.py b/tests/epics/conftest.py new file mode 100644 index 0000000000..6d8f331765 --- /dev/null +++ b/tests/epics/conftest.py @@ -0,0 +1,38 @@ +from collections.abc import Callable + +import pytest +from bluesky.run_engine import RunEngine + +from ophyd_async.core._detector import StandardDetector +from ophyd_async.core._device import DeviceCollector +from ophyd_async.core._mock_signal_utils import set_mock_value + + +@pytest.fixture +def ad_standard_det_factory( + RE: RunEngine, + static_path_provider, +) -> Callable: + def generate_ad_standard_det( + ad_standard_detector_class, number=1 + ) -> StandardDetector: + # Dynamically generate a name based on the class of detector + detector_name = ad_standard_detector_class.__name__ + if detector_name.endswith("Detector"): + detector_name = detector_name[: -len("Detector")] + + with DeviceCollector(mock=True): + test_adstandard_det = ad_standard_detector_class( + f"{detector_name.upper()}{number}:", + static_path_provider, + name=f"test_ad{detector_name.lower()}{number}", + ) + + # Set number of frames per chunk and frame dimensions to something reasonable + set_mock_value(test_adstandard_det.hdf.num_frames_chunks, 1) + set_mock_value(test_adstandard_det.drv.array_size_x, 10) + set_mock_value(test_adstandard_det.drv.array_size_y, 10) + + return test_adstandard_det + + return generate_ad_standard_det diff --git a/tests/fastcs/panda/test_hdf_panda.py b/tests/fastcs/panda/test_hdf_panda.py index 7752a3263b..41f50d648d 100644 --- a/tests/fastcs/panda/test_hdf_panda.py +++ b/tests/fastcs/panda/test_hdf_panda.py @@ -168,6 +168,7 @@ def flying_plan(): "dataset": f"/{dataset_name}", "swmr": False, "multiplier": 1, + "chunk_shape": (1024,), }, } assert "test-panda.h5" in stream_resource["uri"] @@ -284,6 +285,7 @@ def flying_plan(): "dataset": f"/{dataset_name}", "swmr": False, "multiplier": 1, + "chunk_shape": (1024,), }, } assert "test-panda.h5" in stream_resource["uri"] diff --git a/tests/fastcs/panda/test_writer.py b/tests/fastcs/panda/test_writer.py index 5905c3c0e2..e7298568d1 100644 --- a/tests/fastcs/panda/test_writer.py +++ b/tests/fastcs/panda/test_writer.py @@ -209,7 +209,12 @@ def assert_resource_document(name, resource_doc): "data_key": name, "mimetype": "application/x-hdf5", "uri": "file://localhost" + str(tmp_path / "mock_panda" / "data.h5"), - "parameters": {"dataset": f"/{name}", "swmr": False, "multiplier": 1}, + "parameters": { + "dataset": f"/{name}", + "swmr": False, + "multiplier": 1, + "chunk_shape": (1024,), + }, } assert "mock_panda/data.h5" in resource_doc["uri"] From 1007da7459e7bf54b2aacf33af390ee1a42272af Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh Date: Thu, 19 Sep 2024 18:35:48 +0100 Subject: [PATCH 5/9] fixed typos (#589) --- src/ophyd_async/core/_detector.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ophyd_async/core/_detector.py b/src/ophyd_async/core/_detector.py index 125f1867aa..bacd43a279 100644 --- a/src/ophyd_async/core/_detector.py +++ b/src/ophyd_async/core/_detector.py @@ -194,7 +194,7 @@ def __init__( self._fly_status: WatchableAsyncStatus | None = None self._fly_start: float self._iterations_completed: int = 0 - self._intial_frame: int + self._initial_frame: int self._last_frame: int super().__init__(name) @@ -208,7 +208,7 @@ def writer(self) -> DetectorWriter: @AsyncStatus.wrap async def stage(self) -> None: - # Disarm the detector, stop filewriting. + # Disarm the detector, stop file writing. await self._check_config_sigs() await asyncio.gather(self.writer.close(), self.controller.disarm()) self._trigger_info = None From 8af94c9de8cce7a34173ee15508ab489073a11ef Mon Sep 17 00:00:00 2001 From: Eva Lott Date: Fri, 20 Sep 2024 09:25:58 +0100 Subject: [PATCH 6/9] Make table subclass enums be sequence enum rather than numpy string (#579) --------- Co-authored-by: Eva Lott Co-authored-by: Tom Cobb --- src/ophyd_async/core/_table.py | 119 +++++++++++-- src/ophyd_async/fastcs/panda/_table.py | 40 +---- tests/fastcs/panda/test_table.py | 237 ++++++++++++++++++++----- tests/fastcs/panda/test_trigger.py | 29 ++- 4 files changed, 320 insertions(+), 105 deletions(-) diff --git a/src/ophyd_async/core/_table.py b/src/ophyd_async/core/_table.py index ff54a01022..f36b60dceb 100644 --- a/src/ophyd_async/core/_table.py +++ b/src/ophyd_async/core/_table.py @@ -1,4 +1,5 @@ -from typing import TypeVar +from enum import Enum +from typing import TypeVar, get_args, get_origin import numpy as np from pydantic import BaseModel, ConfigDict, model_validator @@ -6,6 +7,13 @@ TableSubclass = TypeVar("TableSubclass", bound="Table") +def _concat(value1, value2): + if isinstance(value1, np.ndarray): + return np.concatenate((value1, value2)) + else: + return value1 + value2 + + class Table(BaseModel): """An abstraction of a Table of str to numpy array.""" @@ -13,34 +21,105 @@ class Table(BaseModel): @staticmethod def row(cls: type[TableSubclass], **kwargs) -> TableSubclass: # type: ignore - arrayified_kwargs = { - field_name: np.concatenate( - ( - (default_arr := field_value.default_factory()), # type: ignore - np.array([kwargs[field_name]], dtype=default_arr.dtype), + arrayified_kwargs = {} + for field_name, field_value in cls.model_fields.items(): + value = kwargs.pop(field_name) + if field_value.default_factory is None: + raise ValueError( + "`Table` models should have default factories for their " + "mutable empty columns." + ) + default_array = field_value.default_factory() + if isinstance(default_array, np.ndarray): + arrayified_kwargs[field_name] = np.array( + [value], dtype=default_array.dtype + ) + elif issubclass(type(value), Enum) and isinstance(value, str): + arrayified_kwargs[field_name] = [value] + else: + raise TypeError( + "Row column should be numpy arrays or sequence of string `Enum`." ) + if kwargs: + raise TypeError( + f"Unexpected keyword arguments {kwargs.keys()} for {cls.__name__}." ) - for field_name, field_value in cls.model_fields.items() - } return cls(**arrayified_kwargs) def __add__(self, right: TableSubclass) -> TableSubclass: """Concatenate the arrays in field values.""" - assert type(right) is type(self), ( - f"{right} is not a `Table`, or is not the same " - f"type of `Table` as {self}." - ) + if type(right) is not type(self): + raise RuntimeError( + f"{right} is not a `Table`, or is not the same " + f"type of `Table` as {self}." + ) return type(right)( **{ - field_name: np.concatenate( - (getattr(self, field_name), getattr(right, field_name)) + field_name: _concat( + getattr(self, field_name), getattr(right, field_name) ) for field_name in self.model_fields } ) + def numpy_dtype(self) -> np.dtype: + dtype = [] + for field_name, field_value in self.model_fields.items(): + if np.ndarray in ( + get_origin(field_value.annotation), + field_value.annotation, + ): + dtype.append((field_name, getattr(self, field_name).dtype)) + else: + enum_type = get_args(field_value.annotation)[0] + assert issubclass(enum_type, Enum) + enum_values = [element.value for element in enum_type] + max_length_in_enum = max(len(value) for value in enum_values) + dtype.append((field_name, np.dtype(f" list[np.ndarray]: + """Columns in the table can be lists of string enums or numpy arrays. + + This method returns the columns, converting the string enums to numpy arrays. + """ + + columns = [] + for field_name, field_value in self.model_fields.items(): + if np.ndarray in ( + get_origin(field_value.annotation), + field_value.annotation, + ): + columns.append(getattr(self, field_name)) + else: + enum_type = get_args(field_value.annotation)[0] + assert issubclass(enum_type, Enum) + enum_values = [element.value for element in enum_type] + max_length_in_enum = max(len(value) for value in enum_values) + dtype = np.dtype(f" "Table": first_length = len(next(iter(self))[1]) @@ -49,11 +128,15 @@ def validate_arrays(self) -> "Table": ), "Rows should all be of equal size." if not all( - np.issubdtype( - self.model_fields[field_name].default_factory().dtype, # type: ignore - field_value.dtype, + # Checks if the values are numpy subtypes if the array is a numpy array, + # or if the value is a string enum. + np.issubdtype(getattr(self, field_name).dtype, default_array.dtype) + if isinstance( + default_array := self.model_fields[field_name].default_factory(), # type: ignore + np.ndarray, ) - for field_name, field_value in self + else issubclass(get_args(field_value.annotation)[0], Enum) + for field_name, field_value in self.model_fields.items() ): raise ValueError( f"Cannot construct a `{type(self).__name__}`, " diff --git a/src/ophyd_async/fastcs/panda/_table.py b/src/ophyd_async/fastcs/panda/_table.py index 91fce9a298..a021d23fa8 100644 --- a/src/ophyd_async/fastcs/panda/_table.py +++ b/src/ophyd_async/fastcs/panda/_table.py @@ -4,7 +4,7 @@ import numpy as np import numpy.typing as npt -from pydantic import Field, field_validator, model_validator +from pydantic import Field, model_validator from pydantic_numpy.helper.annotation import NpArrayPydanticAnnotation from typing_extensions import TypedDict @@ -51,13 +51,7 @@ class SeqTrigger(str, Enum): ), Field(default_factory=lambda: np.array([], dtype=np.bool_)), ] -TriggerStr = Annotated[ - np.ndarray[tuple[int], np.dtype[np.unicode_]], - NpArrayPydanticAnnotation.factory( - data_type=np.unicode_, dimensions=1, strict_data_typing=False - ), - Field(default_factory=lambda: np.array([], dtype=np.dtype(" "SeqTable": - if isinstance(trigger, SeqTrigger): - trigger = trigger.value - return super().row(**locals()) - - @field_validator("trigger", mode="before") - @classmethod - def trigger_to_np_array(cls, trigger_column): - """ - The user can provide a list of SeqTrigger enum elements instead of a numpy str. - """ - if isinstance(trigger_column, Sequence) and all( - isinstance(trigger, SeqTrigger) for trigger in trigger_column - ): - trigger_column = np.array( - [trigger.value for trigger in trigger_column], dtype=np.dtype(" "SeqTable": diff --git a/tests/fastcs/panda/test_table.py b/tests/fastcs/panda/test_table.py index b34103250b..ed963c91f5 100644 --- a/tests/fastcs/panda/test_table.py +++ b/tests/fastcs/panda/test_table.py @@ -12,15 +12,18 @@ def test_seq_table_converts_lists(): seq_table_dict_with_lists = {field_name: [] for field_name, _ in SeqTable()} # Validation passes seq_table = SeqTable(**seq_table_dict_with_lists) - assert isinstance(seq_table.trigger, np.ndarray) - assert seq_table.trigger.dtype == np.dtype("U32") + for field_name, field_value in seq_table: + if field_name == "trigger": + assert field_value == [] + else: + assert np.array_equal(field_value, np.array([], dtype=field_value.dtype)) def test_seq_table_validation_errors(): with pytest.raises(ValidationError, match="81 validation errors for SeqTable"): SeqTable( repeats=0, - trigger="Immediate", + trigger=SeqTrigger.IMMEDIATE, position=0, time1=0, outa1=False, @@ -40,7 +43,7 @@ def test_seq_table_validation_errors(): large_seq_table = SeqTable( repeats=np.zeros(4095, dtype=np.int32), - trigger=np.array(["Immediate"] * 4095, dtype="U32"), + trigger=["Immediate"] * 4095, position=np.zeros(4095, dtype=np.int32), time1=np.zeros(4095, dtype=np.int32), outa1=np.zeros(4095, dtype=np.bool_), @@ -73,16 +76,25 @@ def test_seq_table_validation_errors(): wrong_types = { field_name: field_value.astype(np.unicode_) for field_name, field_value in row_one + if isinstance(field_value, np.ndarray) } SeqTable(**wrong_types) + with pytest.raises( + TypeError, + match="Row column should be numpy arrays or sequence of string `Enum`", + ): + SeqTable.row(trigger="A") def test_seq_table_pva_conversion(): pva_dict = { "repeats": np.array([1, 2, 3, 4], dtype=np.int32), - "trigger": np.array( - ["Immediate", "Immediate", "BITC=0", "Immediate"], dtype=np.dtype("U32") - ), + "trigger": [ + SeqTrigger.IMMEDIATE, + SeqTrigger.IMMEDIATE, + SeqTrigger.BITC_0, + SeqTrigger.IMMEDIATE, + ], "position": np.array([1, 2, 3, 4], dtype=np.int32), "time1": np.array([1, 0, 1, 0], dtype=np.int32), "outa1": np.array([1, 0, 1, 0], dtype=np.bool_), @@ -102,7 +114,7 @@ def test_seq_table_pva_conversion(): row_wise_dicts = [ { "repeats": 1, - "trigger": "Immediate", + "trigger": SeqTrigger.IMMEDIATE, "position": 1, "time1": 1, "outa1": 1, @@ -121,7 +133,7 @@ def test_seq_table_pva_conversion(): }, { "repeats": 2, - "trigger": "Immediate", + "trigger": SeqTrigger.IMMEDIATE, "position": 2, "time1": 0, "outa1": 0, @@ -140,7 +152,7 @@ def test_seq_table_pva_conversion(): }, { "repeats": 3, - "trigger": "BITC=0", + "trigger": SeqTrigger.BITC_0, "position": 3, "time1": 1, "outa1": 1, @@ -159,7 +171,7 @@ def test_seq_table_pva_conversion(): }, { "repeats": 4, - "trigger": "Immediate", + "trigger": SeqTrigger.IMMEDIATE, "position": 4, "time1": 0, "outa1": 0, @@ -178,12 +190,20 @@ def test_seq_table_pva_conversion(): }, ] + def _assert_col_equal(column1, column2): + if isinstance(column1, np.ndarray): + assert np.array_equal(column1, column2) + assert column1.dtype == column2.dtype + else: + assert column1 == column2 + assert all(isinstance(x, SeqTrigger) for x in column1) + assert all(isinstance(x, SeqTrigger) for x in column2) + seq_table_from_pva_dict = SeqTable(**pva_dict) for (_, column1), column2 in zip( seq_table_from_pva_dict, pva_dict.values(), strict=False ): - assert np.array_equal(column1, column2) - assert column1.dtype == column2.dtype + _assert_col_equal(column1, column2) seq_table_from_rows = reduce( lambda x, y: x + y, @@ -192,41 +212,174 @@ def test_seq_table_pva_conversion(): for (_, column1), column2 in zip( seq_table_from_rows, pva_dict.values(), strict=False ): - assert np.array_equal(column1, column2) - assert column1.dtype == column2.dtype + _assert_col_equal(column1, column2) # Idempotency applied_twice_to_pva_dict = SeqTable(**pva_dict).model_dump(mode="python") for column1, column2 in zip( applied_twice_to_pva_dict.values(), pva_dict.values(), strict=False ): - assert np.array_equal(column1, column2) - assert column1.dtype == column2.dtype + _assert_col_equal(column1, column2) + + assert np.array_equal( + seq_table_from_pva_dict.numpy_columns(), + [ + np.array([1, 2, 3, 4], dtype=np.int32), + np.array( + [ + "Immediate", + "Immediate", + "BITC=0", + "Immediate", + ], + dtype=" Date: Fri, 20 Sep 2024 07:46:49 -0400 Subject: [PATCH 7/9] Reset completed iterations counter to 0 once target iterations are reached and detector is disarmed (#590) * Reset completed iterations counter to 0 once target iterations are reached and detector is disarmed * Fix tests * Add test for double kickoff to increase coverage * Make linter happy --- src/ophyd_async/core/_detector.py | 6 ++- tests/core/test_flyer.py | 74 ++++++++++++++++++++++++++++ tests/fastcs/panda/test_hdf_panda.py | 12 ++++- 3 files changed, 90 insertions(+), 2 deletions(-) diff --git a/src/ophyd_async/core/_detector.py b/src/ophyd_async/core/_detector.py index bacd43a279..3d18cccf31 100644 --- a/src/ophyd_async/core/_detector.py +++ b/src/ophyd_async/core/_detector.py @@ -314,7 +314,10 @@ async def prepare(self, value: TriggerInfo) -> None: async def kickoff(self): assert self._trigger_info, "Prepare must be called before kickoff!" if self._iterations_completed >= self._trigger_info.iteration: - raise Exception(f"Kickoff called more than {self._trigger_info.iteration}") + raise Exception( + f"Kickoff called more than the configured number of " + f"{self._trigger_info.iteration} iteration(s)!" + ) self._iterations_completed += 1 @WatchableAsyncStatus.wrap @@ -340,6 +343,7 @@ async def complete(self): if index >= self._trigger_info.number: break if self._iterations_completed == self._trigger_info.iteration: + self._iterations_completed = 0 await self.controller.wait_for_idle() async def describe_collect(self) -> dict[str, DataKey]: diff --git a/tests/core/test_flyer.py b/tests/core/test_flyer.py index b9a3186134..c7345ee03a 100644 --- a/tests/core/test_flyer.py +++ b/tests/core/test_flyer.py @@ -186,6 +186,7 @@ def flying_plan(): yield from bps.complete(flyer, wait=False, group="complete") for detector in detectors: yield from bps.complete(detector, wait=False, group="complete") + assert flyer._trigger_logic.state == TriggerState.null # Manually incremenet the index as if a frame was taken @@ -206,6 +207,12 @@ def flying_plan(): name="main_stream", ) yield from bps.wait(group="complete") + + for detector in detectors: + # Since we set number of iterations to 1 (default), + # make sure it gets reset on complete + assert detector._iterations_completed == 0 + yield from bps.close_run() yield from bps.unstage_all(flyer, *detectors) @@ -227,6 +234,73 @@ def flying_plan(): ] +async def test_hardware_triggered_flyable_too_many_kickoffs( + RE: RunEngine, detectors: tuple[StandardDetector] +): + trigger_logic = DummyTriggerLogic() + flyer = StandardFlyer(trigger_logic, [], name="flyer") + trigger_info = TriggerInfo( + number=1, trigger=DetectorTrigger.constant_gate, deadtime=2, livetime=2 + ) + + def flying_plan(): + yield from bps.stage_all(*detectors, flyer) + assert flyer._trigger_logic.state == TriggerState.stopping + + # move the flyer to the correct place, before fly scanning. + # Prepare the flyer first to get the trigger info for the detectors + yield from bps.prepare(flyer, 1, wait=True) + + # prepare detectors second. + for detector in detectors: + yield from bps.prepare( + detector, + trigger_info, + wait=True, + ) + + yield from bps.open_run() + yield from bps.declare_stream(*detectors, name="main_stream", collect=True) + + for _ in range(2): + yield from bps.kickoff(flyer) + for detector in detectors: + yield from bps.kickoff(detector) + + yield from bps.complete(flyer, wait=False, group="complete") + for detector in detectors: + yield from bps.complete(detector, wait=False, group="complete") + + assert flyer._trigger_logic.state == TriggerState.null + + # Manually incremenet the index as if a frame was taken + for detector in detectors: + detector.writer.index += 1 + + yield from bps.wait(group="complete") + + yield from bps.collect( + *detectors, + return_payload=False, + name="main_stream", + ) + + for detector in detectors: + # Since we set number of iterations to 1 (default), + # make sure it gets reset on complete + assert detector._iterations_completed == 0 + + yield from bps.close_run() + + yield from bps.unstage_all(flyer, *detectors) + + # fly scan + with pytest.raises( + Exception, match="Kickoff called more than the configured number" + ): + RE(flying_plan()) + + # To do: Populate configuration signals async def test_describe_configuration(): flyer = StandardFlyer(DummyTriggerLogic(), [], name="flyer") diff --git a/tests/fastcs/panda/test_hdf_panda.py b/tests/fastcs/panda/test_hdf_panda.py index 41f50d648d..8b47670817 100644 --- a/tests/fastcs/panda/test_hdf_panda.py +++ b/tests/fastcs/panda/test_hdf_panda.py @@ -224,10 +224,12 @@ def flying_plan(): yield from bps.declare_stream(mock_hdf_panda, name="main_stream", collect=True) - for _ in range(iteration): + for i in range(iteration): set_mock_value(flyer.trigger_logic.seq.active, 1) + yield from bps.kickoff(flyer, wait=True) yield from bps.kickoff(mock_hdf_panda) + assert mock_hdf_panda._iterations_completed == i + 1 yield from bps.complete(flyer, wait=False, group="complete") yield from bps.complete(mock_hdf_panda, wait=False, group="complete") @@ -250,6 +252,14 @@ def flying_plan(): name="main_stream", ) yield from bps.wait(group="complete") + + # Make sure first complete doesn't reset iterations completed + if i == 0: + assert mock_hdf_panda._iterations_completed == 1 + + # Make sure the number of iterations completed is set to 0 after final complete. + assert mock_hdf_panda._iterations_completed == 0 + yield from bps.close_run() yield from bps.unstage_all(flyer, mock_hdf_panda) From e7cef5bebce8f079bf44fb842bb8a26bf2554fa3 Mon Sep 17 00:00:00 2001 From: Jakub Wlodek Date: Sat, 21 Sep 2024 08:49:57 -0400 Subject: [PATCH 8/9] Simplify ad standard det tests (#592) * Refactor ADPilatus tests to use the ad_standard_det_factory * Add default signals for sim detector config * Fix type annotation * Overhaul sim detector tests to use ad standard det factory, improve factory with mock put callback * Fix tests for remaining detectors to account for new changes to factory fixture * Make sure to include config_sigs passed to init * Add suggestions from review --- src/ophyd_async/epics/adsimdetector/_sim.py | 2 +- tests/conftest.py | 2 +- tests/epics/adaravis/test_aravis.py | 10 +- tests/epics/adkinetix/test_kinetix.py | 13 +- tests/epics/adpilatus/test_pilatus.py | 54 ++--- .../adpilatus/test_pilatus_controller.py | 46 ----- .../adsimdetector/test_adsim_controller.py | 32 --- tests/epics/adsimdetector/test_sim.py | 186 +++++++++--------- tests/epics/advimba/test_vimba.py | 13 +- tests/epics/conftest.py | 26 ++- 10 files changed, 154 insertions(+), 230 deletions(-) delete mode 100644 tests/epics/adpilatus/test_pilatus_controller.py delete mode 100644 tests/epics/adsimdetector/test_adsim_controller.py diff --git a/src/ophyd_async/epics/adsimdetector/_sim.py b/src/ophyd_async/epics/adsimdetector/_sim.py index 5a23b47744..acc640899a 100644 --- a/src/ophyd_async/epics/adsimdetector/_sim.py +++ b/src/ophyd_async/epics/adsimdetector/_sim.py @@ -30,6 +30,6 @@ def __init__( lambda: self.name, adcore.ADBaseDatasetDescriber(self.drv), ), - config_sigs=config_sigs, + config_sigs=(self.drv.acquire_period, self.drv.acquire_time, *config_sigs), name=name, ) diff --git a/tests/conftest.py b/tests/conftest.py index d71738ae85..0dfd2c75c6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -220,7 +220,7 @@ def create_static_dir_provider_given_fp(fp: FilenameProvider): @pytest.fixture def static_path_provider( - static_path_provider_factory: callable, + static_path_provider_factory: Callable, static_filename_provider: FilenameProvider, ): return static_path_provider_factory(static_filename_provider) diff --git a/tests/epics/adaravis/test_aravis.py b/tests/epics/adaravis/test_aravis.py index 341ad280ed..ee40fd6fc5 100644 --- a/tests/epics/adaravis/test_aravis.py +++ b/tests/epics/adaravis/test_aravis.py @@ -83,9 +83,6 @@ async def test_can_read(test_adaravis: adaravis.AravisDetector): async def test_decribe_describes_writer_dataset( test_adaravis: adaravis.AravisDetector, one_shot_trigger_info: TriggerInfo ): - set_mock_value(test_adaravis._writer.hdf.file_path_exists, True) - set_mock_value(test_adaravis._writer.hdf.capture, True) - assert await test_adaravis.describe() == {} await test_adaravis.stage() await test_adaravis.prepare(one_shot_trigger_info) @@ -106,10 +103,7 @@ async def test_can_collect( one_shot_trigger_info: TriggerInfo, ): path_info = static_path_provider() - full_file_name = path_info.directory_path / "foo.h5" - set_mock_value(test_adaravis.hdf.full_file_name, str(full_file_name)) - set_mock_value(test_adaravis._writer.hdf.file_path_exists, True) - set_mock_value(test_adaravis._writer.hdf.capture, True) + full_file_name = path_info.directory_path / f"{path_info.filename}.h5" await test_adaravis.stage() await test_adaravis.prepare(one_shot_trigger_info) docs = [(name, doc) async for name, doc in test_adaravis.collect_asset_docs(1)] @@ -135,8 +129,6 @@ async def test_can_collect( async def test_can_decribe_collect( test_adaravis: adaravis.AravisDetector, one_shot_trigger_info: TriggerInfo ): - set_mock_value(test_adaravis._writer.hdf.file_path_exists, True) - set_mock_value(test_adaravis._writer.hdf.capture, True) assert (await test_adaravis.describe_collect()) == {} await test_adaravis.stage() await test_adaravis.prepare(one_shot_trigger_info) diff --git a/tests/epics/adkinetix/test_kinetix.py b/tests/epics/adkinetix/test_kinetix.py index ae2f72462e..5e460f63d1 100644 --- a/tests/epics/adkinetix/test_kinetix.py +++ b/tests/epics/adkinetix/test_kinetix.py @@ -51,7 +51,7 @@ async def setup_trigger_mode(trig_mode: DetectorTrigger): async def test_hints_from_hdf_writer(test_adkinetix: adkinetix.KinetixDetector): - assert test_adkinetix.hints == {"fields": ["test_adkinetix1"]} + assert test_adkinetix.hints == {"fields": [test_adkinetix.name]} async def test_can_read(test_adkinetix: adkinetix.KinetixDetector): @@ -62,9 +62,6 @@ async def test_can_read(test_adkinetix: adkinetix.KinetixDetector): async def test_decribe_describes_writer_dataset( test_adkinetix: adkinetix.KinetixDetector, one_shot_trigger_info: TriggerInfo ): - set_mock_value(test_adkinetix._writer.hdf.file_path_exists, True) - set_mock_value(test_adkinetix._writer.hdf.capture, True) - assert await test_adkinetix.describe() == {} await test_adkinetix.stage() await test_adkinetix.prepare(one_shot_trigger_info) @@ -85,10 +82,8 @@ async def test_can_collect( one_shot_trigger_info: TriggerInfo, ): path_info = static_path_provider() - full_file_name = path_info.directory_path / "foo.h5" - set_mock_value(test_adkinetix.hdf.full_file_name, str(full_file_name)) - set_mock_value(test_adkinetix._writer.hdf.file_path_exists, True) - set_mock_value(test_adkinetix._writer.hdf.capture, True) + full_file_name = path_info.directory_path / f"{path_info.filename}.h5" + await test_adkinetix.stage() await test_adkinetix.prepare(one_shot_trigger_info) docs = [(name, doc) async for name, doc in test_adkinetix.collect_asset_docs(1)] @@ -114,8 +109,6 @@ async def test_can_collect( async def test_can_decribe_collect( test_adkinetix: adkinetix.KinetixDetector, one_shot_trigger_info: TriggerInfo ): - set_mock_value(test_adkinetix._writer.hdf.file_path_exists, True) - set_mock_value(test_adkinetix._writer.hdf.capture, True) assert (await test_adkinetix.describe_collect()) == {} await test_adkinetix.stage() await test_adkinetix.prepare(one_shot_trigger_info) diff --git a/tests/epics/adpilatus/test_pilatus.py b/tests/epics/adpilatus/test_pilatus.py index 7d6145f5a5..192c466a13 100644 --- a/tests/epics/adpilatus/test_pilatus.py +++ b/tests/epics/adpilatus/test_pilatus.py @@ -3,37 +3,24 @@ from unittest.mock import patch import pytest -from bluesky.run_engine import RunEngine from ophyd_async.core import ( DetectorTrigger, - DeviceCollector, - PathProvider, TriggerInfo, set_mock_value, ) -from ophyd_async.epics import adpilatus +from ophyd_async.epics import adcore, adpilatus @pytest.fixture -async def test_adpilatus( - RE: RunEngine, - static_path_provider: PathProvider, -) -> adpilatus.PilatusDetector: - async with DeviceCollector(mock=True): - test_adpilatus = adpilatus.PilatusDetector("PILATUS:", static_path_provider) - - return test_adpilatus - - -async def test_deadtime_overridable(static_path_provider: PathProvider): - async with DeviceCollector(mock=True): - test_adpilatus = adpilatus.PilatusDetector( - "PILATUS:", - static_path_provider, - readout_time=adpilatus.PilatusReadoutTime.pilatus2, - ) - pilatus_controller = test_adpilatus.controller +def test_adpilatus(ad_standard_det_factory) -> adpilatus.PilatusDetector: + return ad_standard_det_factory(adpilatus.PilatusDetector) + + +async def test_deadtime_overridable(test_adpilatus: adpilatus.PilatusDetector): + pilatus_controller = test_adpilatus._controller + pilatus_controller._readout_time = adpilatus.PilatusReadoutTime.pilatus2 + # deadtime invariant with exposure time assert pilatus_controller.get_deadtime(0) == 2.28e-3 @@ -110,7 +97,7 @@ async def _trigger( async def test_hints_from_hdf_writer(test_adpilatus: adpilatus.PilatusDetector): - assert test_adpilatus.hints == {"fields": ["test_adpilatus"]} + assert test_adpilatus.hints == {"fields": [test_adpilatus.name]} async def test_unsupported_trigger_excepts(test_adpilatus: adpilatus.PilatusDetector): @@ -144,3 +131,24 @@ async def dummy_open(multiplier: int = 0): ) assert (await test_adpilatus.drv.acquire_time.get_value()) == 1.0 assert (await test_adpilatus.drv.acquire_period.get_value()) == 1.0 + 950e-6 + + +async def test_pilatus_controller(test_adpilatus: adpilatus.PilatusDetector): + pilatus = test_adpilatus._controller + pilatus_driver = pilatus._drv + set_mock_value(pilatus_driver.armed, True) + await pilatus.prepare(TriggerInfo(number=1, trigger=DetectorTrigger.constant_gate)) + await pilatus.arm() + await pilatus.wait_for_idle() + + assert await pilatus_driver.num_images.get_value() == 1 + assert await pilatus_driver.image_mode.get_value() == adcore.ImageMode.multiple + assert ( + await pilatus_driver.trigger_mode.get_value() + == adpilatus.PilatusTriggerMode.ext_enable + ) + assert await pilatus_driver.acquire.get_value() is True + + await pilatus.disarm() + + assert await pilatus_driver.acquire.get_value() is False diff --git a/tests/epics/adpilatus/test_pilatus_controller.py b/tests/epics/adpilatus/test_pilatus_controller.py deleted file mode 100644 index bb825b8744..0000000000 --- a/tests/epics/adpilatus/test_pilatus_controller.py +++ /dev/null @@ -1,46 +0,0 @@ -import pytest - -from ophyd_async.core import DetectorTrigger, DeviceCollector, set_mock_value -from ophyd_async.core._detector import TriggerInfo -from ophyd_async.epics import adcore, adpilatus - - -@pytest.fixture -async def pilatus_driver(RE) -> adpilatus.PilatusDriverIO: - async with DeviceCollector(mock=True): - drv = adpilatus.PilatusDriverIO("DRIVER:") - - return drv - - -@pytest.fixture -async def pilatus( - RE, pilatus_driver: adpilatus.PilatusDriverIO -) -> adpilatus.PilatusController: - async with DeviceCollector(mock=True): - controller = adpilatus.PilatusController(pilatus_driver, readout_time=2.28) - - return controller - - -async def test_pilatus_controller( - RE, - pilatus: adpilatus.PilatusController, - pilatus_driver: adpilatus.PilatusDriverIO, -): - set_mock_value(pilatus_driver.armed, True) - await pilatus.prepare(TriggerInfo(number=1, trigger=DetectorTrigger.constant_gate)) - await pilatus.arm() - await pilatus.wait_for_idle() - - assert await pilatus_driver.num_images.get_value() == 1 - assert await pilatus_driver.image_mode.get_value() == adcore.ImageMode.multiple - assert ( - await pilatus_driver.trigger_mode.get_value() - == adpilatus.PilatusTriggerMode.ext_enable - ) - assert await pilatus_driver.acquire.get_value() is True - - await pilatus.disarm() - - assert await pilatus_driver.acquire.get_value() is False diff --git a/tests/epics/adsimdetector/test_adsim_controller.py b/tests/epics/adsimdetector/test_adsim_controller.py deleted file mode 100644 index 64d53ce590..0000000000 --- a/tests/epics/adsimdetector/test_adsim_controller.py +++ /dev/null @@ -1,32 +0,0 @@ -from unittest.mock import patch - -import pytest - -from ophyd_async.core import DeviceCollector -from ophyd_async.core._detector import DetectorTrigger, TriggerInfo -from ophyd_async.epics import adcore, adsimdetector - - -@pytest.fixture -async def ad(RE) -> adsimdetector.SimController: - async with DeviceCollector(mock=True): - drv = adcore.ADBaseIO("DRIVER:") - controller = adsimdetector.SimController(drv) - - return controller - - -async def test_ad_controller(RE, ad: adsimdetector.SimController): - with patch("ophyd_async.core._signal.wait_for_value", return_value=None): - await ad.prepare(TriggerInfo(number=1, trigger=DetectorTrigger.internal)) - await ad.arm() - await ad.wait_for_idle() - - driver = ad.driver - assert await driver.num_images.get_value() == 1 - assert await driver.image_mode.get_value() == adcore.ImageMode.multiple - assert await driver.acquire.get_value() is True - - await ad.disarm() - - assert await driver.acquire.get_value() is False diff --git a/tests/epics/adsimdetector/test_sim.py b/tests/epics/adsimdetector/test_sim.py index fd99fd0497..637c3c6ed7 100644 --- a/tests/epics/adsimdetector/test_sim.py +++ b/tests/epics/adsimdetector/test_sim.py @@ -4,46 +4,40 @@ from collections import defaultdict from pathlib import Path from typing import cast +from unittest.mock import patch import bluesky.plan_stubs as bps import bluesky.preprocessors as bpp import pytest -from bluesky import RunEngine -from bluesky.utils import new_uid +from bluesky.run_engine import RunEngine import ophyd_async.plan_stubs as ops from ophyd_async.core import ( AsyncStatus, DetectorTrigger, - DeviceCollector, - StandardDetector, StaticFilenameProvider, StaticPathProvider, TriggerInfo, assert_emitted, - callback_on_mock_put, set_mock_value, ) from ophyd_async.epics import adcore, adsimdetector -async def make_detector(prefix: str, name: str, tmp_path: Path): - fp = StaticFilenameProvider(f"test-{new_uid()}") - dp = StaticPathProvider(fp, tmp_path) - - async with DeviceCollector(mock=True): - det = adsimdetector.SimDetector(prefix, dp, name=name) - det._config_sigs = [det.drv.acquire_time, det.drv.acquire] +@pytest.fixture +def test_adsimdetector(ad_standard_det_factory): + return ad_standard_det_factory(adsimdetector.SimDetector) - def _set_full_file_name(val, *args, **kwargs): - set_mock_value(det.hdf.full_file_name, str(tmp_path / val)) - callback_on_mock_put(det.hdf.file_name, _set_full_file_name) +@pytest.fixture +def two_test_adsimdetectors(ad_standard_det_factory): + deta = ad_standard_det_factory(adsimdetector.SimDetector) + detb = ad_standard_det_factory(adsimdetector.SimDetector, number=2) - return det + return deta, detb -def count_sim(dets: list[StandardDetector], times: int = 1): +def count_sim(dets: list[adsimdetector.SimDetector], times: int = 1): """Test plan to do the equivalent of bp.count for a sim detector.""" yield from bps.stage_all(*dets) @@ -79,38 +73,8 @@ def count_sim(dets: list[StandardDetector], times: int = 1): yield from bps.unstage_all(*dets) -@pytest.fixture -async def single_detector(RE: RunEngine, tmp_path: Path) -> StandardDetector: - detector = await make_detector(prefix="TEST:", name="test", tmp_path=tmp_path) - - set_mock_value(detector._controller.driver.array_size_x, 10) - set_mock_value(detector._controller.driver.array_size_y, 20) - return detector - - -@pytest.fixture -async def two_detectors(tmp_path: Path): - deta = await make_detector(prefix="PREFIX1:", name="testa", tmp_path=tmp_path) - detb = await make_detector(prefix="PREFIX2:", name="testb", tmp_path=tmp_path) - - # Simulate backend IOCs being in slightly different states - for i, det in enumerate((deta, detb)): - # accessing the hidden objects just for neat typing - controller = det._controller - writer = det._writer - - set_mock_value(controller.driver.acquire_time, 0.8 + i) - set_mock_value(controller.driver.image_mode, adcore.ImageMode.continuous) - set_mock_value(writer.hdf.num_capture, 1000) - set_mock_value(writer.hdf.num_captured, 0) - set_mock_value(writer.hdf.file_path_exists, True) - set_mock_value(controller.driver.array_size_x, 1024 + i) - set_mock_value(controller.driver.array_size_y, 768 + i) - yield deta, detb - - async def test_two_detectors_fly_different_rate( - two_detectors: list[adsimdetector.SimDetector], RE: RunEngine + two_test_adsimdetectors: list[adsimdetector.SimDetector], RE: RunEngine ): trigger_info = TriggerInfo( number=15, @@ -131,37 +95,37 @@ def assert_n_stream_datums( assert seq_nums["start"] == start assert seq_nums["stop"] == stop - @bpp.stage_decorator(two_detectors) + @bpp.stage_decorator(two_test_adsimdetectors) @bpp.run_decorator() def fly_plan(): - for det in two_detectors: + for det in two_test_adsimdetectors: yield from bps.prepare(det, trigger_info, wait=True, group="prepare") - yield from bps.declare_stream(*two_detectors, name="primary") + yield from bps.declare_stream(*two_test_adsimdetectors, name="primary") - for det in two_detectors: + for det in two_test_adsimdetectors: yield from bps.trigger(det, wait=False, group="trigger_cleanup") # det[0] captures 5 frames, but we do not emit a StreamDatum as det[1] has not - set_mock_value(two_detectors[0].hdf.num_captured, 5) + set_mock_value(two_test_adsimdetectors[0].hdf.num_captured, 5) - yield from bps.collect(*two_detectors) + yield from bps.collect(*two_test_adsimdetectors) assert_n_stream_datums(0) # det[0] captures 10 frames, but we do not emit a StreamDatum as det[1] has not - set_mock_value(two_detectors[0].hdf.num_captured, 10) - yield from bps.collect(*two_detectors) + set_mock_value(two_test_adsimdetectors[0].hdf.num_captured, 10) + yield from bps.collect(*two_test_adsimdetectors) assert_n_stream_datums(0) # det[1] has caught up to first 7 frames, emit streamDatum for seq_num {1,7} - set_mock_value(two_detectors[1].hdf.num_captured, 7) - yield from bps.collect(*two_detectors) + set_mock_value(two_test_adsimdetectors[1].hdf.num_captured, 7) + yield from bps.collect(*two_test_adsimdetectors) assert_n_stream_datums(2, 1, 8) - for det in two_detectors: + for det in two_test_adsimdetectors: set_mock_value(det.hdf.num_captured, 15) # emits stream datum for seq_num {8, 15} - yield from bps.collect(*two_detectors) + yield from bps.collect(*two_test_adsimdetectors) assert_n_stream_datums(4, 8, 16) # Trigger has complete as all expected frames written @@ -174,7 +138,7 @@ def fly_plan(): async def test_two_detectors_step( - two_detectors: list[StandardDetector], + two_test_adsimdetectors: list[adsimdetector.SimDetector], RE: RunEngine, ): names = [] @@ -183,20 +147,22 @@ async def test_two_detectors_step( RE.subscribe(lambda _, doc: docs.append(doc)) [ set_mock_value(cast(adcore.ADHDFWriter, det._writer).hdf.file_path_exists, True) - for det in two_detectors + for det in two_test_adsimdetectors ] - controller_a = cast(adsimdetector.SimController, two_detectors[0].controller) - writer_a = cast(adcore.ADHDFWriter, two_detectors[0].writer) - writer_b = cast(adcore.ADHDFWriter, two_detectors[1].writer) - info_a = writer_a._path_provider(device_name=writer_a.hdf.name) - info_b = writer_b._path_provider(device_name=writer_b.hdf.name) + controller_a = cast( + adsimdetector.SimController, two_test_adsimdetectors[0].controller + ) + writer_a = cast(adcore.ADHDFWriter, two_test_adsimdetectors[0].writer) + writer_b = cast(adcore.ADHDFWriter, two_test_adsimdetectors[1].writer) + info_a = writer_a._path_provider(device_name=writer_a._name_provider()) + info_b = writer_b._path_provider(device_name=writer_b._name_provider()) file_name_a = None file_name_b = None def plan(): nonlocal file_name_a, file_name_b - yield from count_sim(two_detectors, times=1) + yield from count_sim(two_test_adsimdetectors, times=1) drv = controller_a.driver assert False is (yield from bps.rd(drv.acquire)) @@ -229,38 +195,51 @@ def plan(): ] _, descriptor, sra, sda, srb, sdb, event, _ = docs - assert descriptor["configuration"]["testa"]["data"]["testa-drv-acquire_time"] == 0.8 - assert descriptor["configuration"]["testb"]["data"]["testb-drv-acquire_time"] == 1.8 - assert descriptor["data_keys"]["testa"]["shape"] == (768, 1024) - assert descriptor["data_keys"]["testb"]["shape"] == (769, 1025) + assert descriptor["configuration"]["test_adsim1"]["data"][ + "test_adsim1-drv-acquire_time" + ] == pytest.approx(0.8) + assert descriptor["configuration"]["test_adsim2"]["data"][ + "test_adsim2-drv-acquire_time" + ] == pytest.approx(1.8) + assert descriptor["data_keys"]["test_adsim1"]["shape"] == (10, 10) + assert descriptor["data_keys"]["test_adsim2"]["shape"] == (11, 11) assert sda["stream_resource"] == sra["uid"] assert sdb["stream_resource"] == srb["uid"] - assert srb["uri"] == "file://localhost" + str(info_b.directory_path / file_name_b) - assert sra["uri"] == "file://localhost" + str(info_a.directory_path / file_name_a) + assert ( + srb["uri"] + == "file://localhost" + str(info_b.directory_path / info_b.filename) + ".h5" + ) + assert ( + sra["uri"] + == "file://localhost" + str(info_a.directory_path / info_a.filename) + ".h5" + ) assert event["data"] == {} async def test_detector_writes_to_file( - RE: RunEngine, single_detector: StandardDetector, tmp_path: Path + RE: RunEngine, test_adsimdetector: adsimdetector.SimDetector, tmp_path: Path ): names = [] docs = [] RE.subscribe(lambda name, _: names.append(name)) RE.subscribe(lambda _, doc: docs.append(doc)) set_mock_value( - cast(adcore.ADHDFWriter, single_detector._writer).hdf.file_path_exists, True + cast(adcore.ADHDFWriter, test_adsimdetector._writer).hdf.file_path_exists, True ) - RE(count_sim([single_detector], times=3)) + RE(count_sim([test_adsimdetector], times=3)) assert await cast( - adcore.ADHDFWriter, single_detector.writer + adcore.ADHDFWriter, test_adsimdetector.writer ).hdf.file_path.get_value() == str(tmp_path) descriptor_index = names.index("descriptor") - assert docs[descriptor_index].get("data_keys").get("test").get("shape") == (20, 10) + assert docs[descriptor_index].get("data_keys").get("test_adsim1").get("shape") == ( + 10, + 10, + ) assert names == [ "start", "descriptor", @@ -275,39 +254,41 @@ async def test_detector_writes_to_file( ] -async def test_read_and_describe_detector(single_detector: StandardDetector): - describe = await single_detector.describe_configuration() - read = await single_detector.read_configuration() +async def test_read_and_describe_detector( + test_adsimdetector: adsimdetector.SimDetector, +): + describe = await test_adsimdetector.describe_configuration() + read = await test_adsimdetector.read_configuration() assert describe == { - "test-drv-acquire_time": { - "source": "mock+ca://TEST:cam1:AcquireTime_RBV", + "test_adsim1-drv-acquire_time": { + "source": "mock+ca://SIM1:cam1:AcquireTime_RBV", "dtype": "number", "dtype_numpy": " Callable: +) -> Callable[[StandardDetector, int], StandardDetector]: def generate_ad_standard_det( ad_standard_detector_class, number=1 ) -> StandardDetector: @@ -28,10 +29,27 @@ def generate_ad_standard_det( name=f"test_ad{detector_name.lower()}{number}", ) + def on_set_file_path_callback(value, **kwargs): + if os.path.exists(value): + set_mock_value(test_adstandard_det.hdf.file_path_exists, True) + set_mock_value( + test_adstandard_det.hdf.full_file_name, + f"{value}/{static_path_provider._filename_provider(device_name=test_adstandard_det.name)}.h5", + ) + + callback_on_mock_put( + test_adstandard_det.hdf.file_path, on_set_file_path_callback + ) + + # Set some sensible defaults to mimic a real detector setup + set_mock_value(test_adstandard_det.drv.acquire_time, (number - 0.2)) + set_mock_value(test_adstandard_det.drv.acquire_period, float(number)) + set_mock_value(test_adstandard_det.hdf.capture, True) + # Set number of frames per chunk and frame dimensions to something reasonable set_mock_value(test_adstandard_det.hdf.num_frames_chunks, 1) - set_mock_value(test_adstandard_det.drv.array_size_x, 10) - set_mock_value(test_adstandard_det.drv.array_size_y, 10) + set_mock_value(test_adstandard_det.drv.array_size_x, (9 + number)) + set_mock_value(test_adstandard_det.drv.array_size_y, (9 + number)) return test_adstandard_det From 55f3552e667569d872d4f4b5ffc1df4d64601a17 Mon Sep 17 00:00:00 2001 From: Jakub Wlodek Date: Fri, 27 Sep 2024 04:13:30 -0400 Subject: [PATCH 9/9] Remove config sigs kwarg from flyer since use case for them was unclear. (#593) --- src/ophyd_async/core/_flyer.py | 19 ++----------------- tests/core/test_flyer.py | 16 ++-------------- tests/epics/adcore/test_scans.py | 2 +- tests/fastcs/panda/test_hdf_panda.py | 4 ++-- tests/plan_stubs/test_fly.py | 8 ++++---- 5 files changed, 11 insertions(+), 38 deletions(-) diff --git a/src/ophyd_async/core/_flyer.py b/src/ophyd_async/core/_flyer.py index 4414db36c9..3902f10f7c 100644 --- a/src/ophyd_async/core/_flyer.py +++ b/src/ophyd_async/core/_flyer.py @@ -1,14 +1,11 @@ from abc import ABC, abstractmethod -from collections.abc import Sequence from typing import Generic -from bluesky.protocols import Flyable, Preparable, Reading, Stageable -from event_model import DataKey +from bluesky.protocols import Flyable, Preparable, Stageable from ._device import Device -from ._signal import SignalR from ._status import AsyncStatus -from ._utils import T, merge_gathered_dicts +from ._utils import T class TriggerLogic(ABC, Generic[T]): @@ -39,11 +36,9 @@ class StandardFlyer( def __init__( self, trigger_logic: TriggerLogic[T], - configuration_signals: Sequence[SignalR] = (), name: str = "", ): self._trigger_logic = trigger_logic - self._configuration_signals = tuple(configuration_signals) super().__init__(name=name) @property @@ -73,13 +68,3 @@ async def kickoff(self) -> None: @AsyncStatus.wrap async def complete(self) -> None: await self._trigger_logic.complete() - - async def describe_configuration(self) -> dict[str, DataKey]: - return await merge_gathered_dicts( - [sig.describe() for sig in self._configuration_signals] - ) - - async def read_configuration(self) -> dict[str, Reading]: - return await merge_gathered_dicts( - [sig.read() for sig in self._configuration_signals] - ) diff --git a/tests/core/test_flyer.py b/tests/core/test_flyer.py index c7345ee03a..5c2b3e83ed 100644 --- a/tests/core/test_flyer.py +++ b/tests/core/test_flyer.py @@ -151,7 +151,7 @@ def append_and_print(name, doc): RE.subscribe(append_and_print) trigger_logic = DummyTriggerLogic() - flyer = StandardFlyer(trigger_logic, [], name="flyer") + flyer = StandardFlyer(trigger_logic, name="flyer") trigger_info = TriggerInfo( number=1, trigger=DetectorTrigger.constant_gate, deadtime=2, livetime=2 ) @@ -238,7 +238,7 @@ async def test_hardware_triggered_flyable_too_many_kickoffs( RE: RunEngine, detectors: tuple[StandardDetector] ): trigger_logic = DummyTriggerLogic() - flyer = StandardFlyer(trigger_logic, [], name="flyer") + flyer = StandardFlyer(trigger_logic, name="flyer") trigger_info = TriggerInfo( number=1, trigger=DetectorTrigger.constant_gate, deadtime=2, livetime=2 ) @@ -301,18 +301,6 @@ def flying_plan(): RE(flying_plan()) -# To do: Populate configuration signals -async def test_describe_configuration(): - flyer = StandardFlyer(DummyTriggerLogic(), [], name="flyer") - assert await flyer.describe_configuration() == {} - - -# To do: Populate configuration signals -async def test_read_configuration(): - flyer = StandardFlyer(DummyTriggerLogic(), [], name="flyer") - assert await flyer.read_configuration() == {} - - @pytest.mark.parametrize( ["kwargs", "error_msg"], [ diff --git a/tests/epics/adcore/test_scans.py b/tests/epics/adcore/test_scans.py index f5264e9786..4757b2f8b3 100644 --- a/tests/epics/adcore/test_scans.py +++ b/tests/epics/adcore/test_scans.py @@ -102,7 +102,7 @@ def test_hdf_writer_fails_on_timeout_with_flyscan( ) trigger_logic = DummyTriggerLogic() - flyer = StandardFlyer(trigger_logic, [], name="flyer") + flyer = StandardFlyer(trigger_logic, name="flyer") trigger_info = TriggerInfo( number=1, trigger=DetectorTrigger.constant_gate, deadtime=2, livetime=2 ) diff --git a/tests/fastcs/panda/test_hdf_panda.py b/tests/fastcs/panda/test_hdf_panda.py index 8b47670817..fe5e42e524 100644 --- a/tests/fastcs/panda/test_hdf_panda.py +++ b/tests/fastcs/panda/test_hdf_panda.py @@ -91,7 +91,7 @@ def append_and_print(name, doc): exposure = 1 trigger_logic = StaticSeqTableTriggerLogic(mock_hdf_panda.seq[1]) - flyer = StandardFlyer(trigger_logic, [], name="flyer") + flyer = StandardFlyer(trigger_logic, name="flyer") def flying_plan(): yield from bps.stage_all(mock_hdf_panda, flyer) @@ -207,7 +207,7 @@ def append_and_print(name, doc): exposure = 1 trigger_logic = StaticSeqTableTriggerLogic(mock_hdf_panda.seq[1]) - flyer = StandardFlyer(trigger_logic, [], name="flyer") + flyer = StandardFlyer(trigger_logic, name="flyer") def flying_plan(): iteration = 2 diff --git a/tests/plan_stubs/test_fly.py b/tests/plan_stubs/test_fly.py index 866ee5763b..6508ba377d 100644 --- a/tests/plan_stubs/test_fly.py +++ b/tests/plan_stubs/test_fly.py @@ -190,7 +190,7 @@ def __init__( configuration_signals: Sequence[SignalR] = ..., name: str = "", ): - super().__init__(trigger_logic, configuration_signals, name) + super().__init__(trigger_logic, name) @AsyncStatus.wrap async def kickoff(self) -> None: @@ -207,7 +207,7 @@ async def complete(self) -> None: async def seq_flyer(mock_panda): # Make flyer trigger_logic = StaticSeqTableTriggerLogic(mock_panda.seq[1]) - flyer = MockFlyer(trigger_logic, [], name="flyer") + flyer = MockFlyer(trigger_logic, name="flyer") return flyer @@ -216,7 +216,7 @@ async def seq_flyer(mock_panda): async def pcomp_flyer(mock_panda): # Make flyer trigger_logic = StaticPcompTriggerLogic(mock_panda.pcomp[1]) - flyer = MockFlyer(trigger_logic, [], name="flyer") + flyer = MockFlyer(trigger_logic, name="flyer") return flyer @@ -250,7 +250,7 @@ def append_and_print(name, doc): shutter_time = 0.004 trigger_logic = StaticSeqTableTriggerLogic(mock_panda.seq[1]) - flyer = StandardFlyer(trigger_logic, [], name="flyer") + flyer = StandardFlyer(trigger_logic, name="flyer") def flying_plan(): yield from bps.stage_all(*detector_list, flyer)