From 3d08366e2703931de903670bdf59e5a01fceb41f Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Wed, 10 Apr 2024 16:47:06 +0100 Subject: [PATCH 1/3] (#1245) Use eiger bit depth for xray centring nexus file --- .../flyscan_xray_centre_plan.py | 2 + .../panda_flyscan_xray_centre_plan.py | 2 + .../callbacks/rotation/nexus_callback.py | 4 -- .../callbacks/xray_centre/nexus_callback.py | 17 +++-- .../callbacks/conftest.py | 14 ++++ .../xray_centre/test_nexus_handler.py | 68 +++++++++++++++++-- 6 files changed, 91 insertions(+), 16 deletions(-) diff --git a/src/hyperion/experiment_plans/flyscan_xray_centre_plan.py b/src/hyperion/experiment_plans/flyscan_xray_centre_plan.py index 202996dfb..f343f245c 100755 --- a/src/hyperion/experiment_plans/flyscan_xray_centre_plan.py +++ b/src/hyperion/experiment_plans/flyscan_xray_centre_plan.py @@ -39,6 +39,7 @@ from hyperion.device_setup_plans.read_hardware_for_setup import ( read_hardware_for_ispyb_during_collection, read_hardware_for_ispyb_pre_collection, + read_hardware_for_nexus_writer, read_hardware_for_zocalo, ) from hyperion.device_setup_plans.setup_zebra import ( @@ -234,6 +235,7 @@ def run_gridscan( yield from read_hardware_for_ispyb_during_collection( fgs_composite.attenuator, fgs_composite.flux, fgs_composite.dcm ) + yield from read_hardware_for_nexus_writer(fgs_composite.eiger) fgs_motors = fgs_composite.fast_grid_scan diff --git a/src/hyperion/experiment_plans/panda_flyscan_xray_centre_plan.py b/src/hyperion/experiment_plans/panda_flyscan_xray_centre_plan.py index f68cc8dda..06d304cb2 100755 --- a/src/hyperion/experiment_plans/panda_flyscan_xray_centre_plan.py +++ b/src/hyperion/experiment_plans/panda_flyscan_xray_centre_plan.py @@ -22,6 +22,7 @@ from hyperion.device_setup_plans.read_hardware_for_setup import ( read_hardware_for_ispyb_during_collection, read_hardware_for_ispyb_pre_collection, + read_hardware_for_nexus_writer, ) from hyperion.device_setup_plans.setup_panda import ( disarm_panda_for_gridscan, @@ -114,6 +115,7 @@ def run_gridscan( yield from read_hardware_for_ispyb_during_collection( fgs_composite.attenuator, fgs_composite.flux, fgs_composite.dcm ) + yield from read_hardware_for_nexus_writer(fgs_composite.eiger) fgs_motors = fgs_composite.panda_fast_grid_scan diff --git a/src/hyperion/external_interaction/callbacks/rotation/nexus_callback.py b/src/hyperion/external_interaction/callbacks/rotation/nexus_callback.py index e54edb21f..391807876 100644 --- a/src/hyperion/external_interaction/callbacks/rotation/nexus_callback.py +++ b/src/hyperion/external_interaction/callbacks/rotation/nexus_callback.py @@ -2,9 +2,6 @@ from typing import TYPE_CHECKING, Dict -import numpy as np -from numpy.typing import DTypeLike - from hyperion.external_interaction.callbacks.plan_reactive_callback import ( PlanReactiveCallback, ) @@ -42,7 +39,6 @@ def __init__(self) -> None: self.parameters: RotationInternalParameters | None = None self.writer: NexusWriter | None = None self.descriptors: Dict[str, EventDescriptor] = {} - self.data_bit_depth: DTypeLike = np.uint16 def activity_gated_descriptor(self, doc: EventDescriptor): self.descriptors[doc["uid"]] = doc diff --git a/src/hyperion/external_interaction/callbacks/xray_centre/nexus_callback.py b/src/hyperion/external_interaction/callbacks/xray_centre/nexus_callback.py index dc3188c4a..485f15cf9 100644 --- a/src/hyperion/external_interaction/callbacks/xray_centre/nexus_callback.py +++ b/src/hyperion/external_interaction/callbacks/xray_centre/nexus_callback.py @@ -7,6 +7,7 @@ ) from hyperion.external_interaction.nexus.nexus_utils import ( create_beam_and_attenuator_parameters, + vds_type_based_on_bit_depth, ) from hyperion.external_interaction.nexus.write_nexus import NexusWriter from hyperion.log import NEXUS_LOGGER @@ -65,11 +66,8 @@ def activity_gated_descriptor(self, doc: EventDescriptor): self.descriptors[doc["uid"]] = doc def activity_gated_event(self, doc: Event) -> Event | None: - event_descriptor = self.descriptors.get(doc["descriptor"]) - if ( - event_descriptor - and event_descriptor.get("name") == CONST.PLAN.ISPYB_TRANSMISSION_FLUX_READ - ): + assert (event_descriptor := self.descriptors.get(doc["descriptor"])) is not None + if event_descriptor.get("name") == CONST.PLAN.ISPYB_TRANSMISSION_FLUX_READ: data = doc["data"] for nexus_writer in [self.nexus_writer_1, self.nexus_writer_2]: assert nexus_writer, "Nexus callback did not receive start doc" @@ -80,7 +78,14 @@ def activity_gated_event(self, doc: Event) -> Event | None: data["attenuator_actual_transmission"], ) ) - nexus_writer.create_nexus_file() + if event_descriptor.get("name") == CONST.PLAN.NEXUS_READ: + NEXUS_LOGGER.info(f"Nexus handler received event from read hardware {doc}") + for nexus_writer in [self.nexus_writer_1, self.nexus_writer_2]: + vds_data_type = vds_type_based_on_bit_depth( + doc["data"]["eiger_bit_depth"] + ) + assert nexus_writer, "Nexus callback did not receive start doc" + nexus_writer.create_nexus_file(vds_data_type) NEXUS_LOGGER.info(f"Nexus file created at {nexus_writer.full_filename}") return super().activity_gated_event(doc) diff --git a/tests/unit_tests/external_interaction/callbacks/conftest.py b/tests/unit_tests/external_interaction/callbacks/conftest.py index a8c01cb8d..f81b383cc 100644 --- a/tests/unit_tests/external_interaction/callbacks/conftest.py +++ b/tests/unit_tests/external_interaction/callbacks/conftest.py @@ -132,6 +132,11 @@ class TestData: "run_start": "d8bee3ee-f614-4e7a-a516-25d6b9e87ef3", "name": CONST.PLAN.ZOCALO_HW_READ, } # type: ignore + test_descriptor_document_nexus_read: EventDescriptor = { + "uid": "aaaaaa", + "run_start": "d8bee3ee-f614-4e7a-a516-25d6b9e87ef3", + "name": CONST.PLAN.NEXUS_READ, + } # type: ignore test_event_document_pre_data_collection: Event = { "descriptor": "bd45c2e5-2b85-4280-95d7-a9a15800a78b", "time": 1666604299.828203, @@ -160,6 +165,15 @@ class TestData: "uid": "29033ecf-e052-43dd-98af-c7cdd62e8174", "filled": {}, } + test_event_document_nexus_read: Event = { + "uid": "29033ecf-e052-43dd-98af-c7cdd62e8175", + "time": 1709654583.9770422, + "data": {"eiger_bit_depth": "16"}, + "timestamps": {"eiger_bit_depth": 1666604299.8220396}, + "seq_num": 1, + "filled": {}, + "descriptor": "aaaaaa", + } test_event_document_zocalo_hardware: Event = { "uid": "29033ecf-e052-43dd-98af-c7cdd62e8175", "time": 1709654583.9770422, diff --git a/tests/unit_tests/external_interaction/callbacks/xray_centre/test_nexus_handler.py b/tests/unit_tests/external_interaction/callbacks/xray_centre/test_nexus_handler.py index 67ea31536..57be8805d 100644 --- a/tests/unit_tests/external_interaction/callbacks/xray_centre/test_nexus_handler.py +++ b/tests/unit_tests/external_interaction/callbacks/xray_centre/test_nexus_handler.py @@ -1,6 +1,9 @@ +from copy import deepcopy from unittest.mock import MagicMock, patch +import numpy as np import pytest +from numpy.typing import DTypeLike from hyperion.external_interaction.callbacks.xray_centre.nexus_callback import ( GridscanNexusFileCallback, @@ -15,7 +18,7 @@ def nexus_writer(): yield nw -def test_writers_not_setup_on_plan_start_doc( +def test_writers_not_sDTypeLikeetup_on_plan_start_doc( nexus_writer: MagicMock, ): nexus_handler = GridscanNexusFileCallback() @@ -25,14 +28,67 @@ def test_writers_not_setup_on_plan_start_doc( @patch("hyperion.external_interaction.callbacks.xray_centre.nexus_callback.NexusWriter") -def test_writers_dont_create_on_init_but_do_on_ispyb_event( +def test_writers_dont_create_on_init_but_do_on_nexus_read_event( mock_nexus_writer: MagicMock, ): + mock_nexus_writer.side_effect = [MagicMock(), MagicMock()] nexus_handler = GridscanNexusFileCallback() assert nexus_handler.nexus_writer_1 is None assert nexus_handler.nexus_writer_2 is None + nexus_handler.activity_gated_start(TestData.test_start_document) + nexus_handler.activity_gated_descriptor( + TestData.test_descriptor_document_nexus_read + ) + + nexus_handler.activity_gated_event(TestData.test_event_document_nexus_read) + + assert nexus_handler.nexus_writer_1 is not None + assert nexus_handler.nexus_writer_2 is not None + nexus_handler.nexus_writer_1.create_nexus_file.assert_called_once() + nexus_handler.nexus_writer_2.create_nexus_file.assert_called_once() + + +@pytest.mark.parametrize( + ["bit_depth", "vds_type"], + [ + (8, np.uint8), + (16, np.uint16), + (32, np.uint32), + ], +) +@patch("hyperion.external_interaction.callbacks.xray_centre.nexus_callback.NexusWriter") +def test_given_different_bit_depths_then_writers_created_wth_correct_VDS_size( + mock_nexus_writer: MagicMock, + bit_depth: int, + vds_type: DTypeLike, +): + mock_nexus_writer.side_effect = [MagicMock(), MagicMock()] + nexus_handler = GridscanNexusFileCallback() + + nexus_handler.activity_gated_start(TestData.test_start_document) + nexus_handler.activity_gated_descriptor( + TestData.test_descriptor_document_nexus_read + ) + event_doc = deepcopy(TestData.test_event_document_nexus_read) + event_doc["data"]["eiger_bit_depth"] = bit_depth + + nexus_handler.activity_gated_event(event_doc) + + assert nexus_handler.nexus_writer_1 is not None + assert nexus_handler.nexus_writer_2 is not None + nexus_handler.nexus_writer_1.create_nexus_file.assert_called_once_with(vds_type) + nexus_handler.nexus_writer_2.create_nexus_file.assert_called_once_with(vds_type) + + +@patch("hyperion.external_interaction.callbacks.xray_centre.nexus_callback.NexusWriter") +def test_beam_and_attenuator_set_on_ispyb_transmission_event( + mock_nexus_writer: MagicMock, +): + mock_nexus_writer.side_effect = [MagicMock(), MagicMock()] + nexus_handler = GridscanNexusFileCallback() + nexus_handler.activity_gated_start(TestData.test_start_document) nexus_handler.activity_gated_descriptor( TestData.test_descriptor_document_during_data_collection @@ -41,10 +97,10 @@ def test_writers_dont_create_on_init_but_do_on_ispyb_event( TestData.test_event_document_during_data_collection ) - assert nexus_handler.nexus_writer_1 is not None - assert nexus_handler.nexus_writer_2 is not None - nexus_handler.nexus_writer_1.create_nexus_file.assert_called() - nexus_handler.nexus_writer_2.create_nexus_file.assert_called() + for writer in [nexus_handler.nexus_writer_1, nexus_handler.nexus_writer_2]: + assert writer is not None + assert writer.attenuator is not None + assert writer.beam is not None def test_sensible_error_if_writing_triggered_before_params_received( From f3d304030e1d473769964eabe4bf15db2625a73b Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Wed, 10 Apr 2024 17:05:52 +0100 Subject: [PATCH 2/3] (#1245) Remove default nexus VDS bit depth --- .../external_interaction/nexus/write_nexus.py | 8 +++++--- .../nexus/test_write_nexus.py | 16 ++++++++-------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/hyperion/external_interaction/nexus/write_nexus.py b/src/hyperion/external_interaction/nexus/write_nexus.py index 5a16a927f..3d315ba13 100644 --- a/src/hyperion/external_interaction/nexus/write_nexus.py +++ b/src/hyperion/external_interaction/nexus/write_nexus.py @@ -9,7 +9,6 @@ from pathlib import Path from typing import Optional -import numpy as np from dodal.utils import get_beamline_name from nexgen.nxs_utils import Attenuator, Beam, Detector, Goniometer, Source from nexgen.nxs_write.NXmxWriter import NXmxFileWriter @@ -78,15 +77,18 @@ def __init__( self.omega_start, self.scan_points, chi=chi ) - def create_nexus_file(self, bit_depth: DTypeLike = np.uint16): + def create_nexus_file(self, bit_depth: DTypeLike): """ - Creates a nexus file based on the parameters supplied when this obect was + Creates a nexus file based on the parameters supplied when this object was initialised. """ start_time, est_end_time = get_start_and_predicted_end_time( self.detector.exp_time * self.full_num_of_images ) + assert self.beam is not None + assert self.attenuator is not None + vds_shape = self.data_shape for filename in [self.nexus_file, self.master_file]: diff --git a/tests/unit_tests/external_interaction/nexus/test_write_nexus.py b/tests/unit_tests/external_interaction/nexus/test_write_nexus.py index b87e0637b..591f2ad14 100644 --- a/tests/unit_tests/external_interaction/nexus/test_write_nexus.py +++ b/tests/unit_tests/external_interaction/nexus/test_write_nexus.py @@ -118,7 +118,7 @@ def test_given_dummy_data_then_datafile_written_correctly( ): nexus_writer_1, nexus_writer_2 = dummy_nexus_writers grid_scan_params: GridScanParams = test_fgs_params.experiment_params - nexus_writer_1.create_nexus_file() + nexus_writer_1.create_nexus_file(np.uint16) for filename in [nexus_writer_1.nexus_file, nexus_writer_1.master_file]: with h5py.File(filename, "r") as written_nexus_file: @@ -170,7 +170,7 @@ def test_given_dummy_data_then_datafile_written_correctly( assert_data_edge_at(nexus_writer_1.nexus_file, 799) assert_end_data_correct(nexus_writer_1) - nexus_writer_2.create_nexus_file() + nexus_writer_2.create_nexus_file(np.uint16) for filename in [nexus_writer_2.nexus_file, nexus_writer_2.master_file]: with h5py.File(filename, "r") as written_nexus_file: @@ -281,8 +281,8 @@ def check_validity_through_zocalo(nexus_writers: tuple[NexusWriter, NexusWriter] nexus_writer_1, nexus_writer_2 = nexus_writers - nexus_writer_1.create_nexus_file() - nexus_writer_2.create_nexus_file() + nexus_writer_1.create_nexus_file(np.uint16) + nexus_writer_2.create_nexus_file(np.uint16) for filename in [nexus_writer_1.nexus_file, nexus_writer_1.master_file]: with h5py.File(filename, "r") as written_nexus_file: @@ -317,8 +317,8 @@ def test_given_some_datafiles_outside_of_VDS_range_THEN_they_are_not_in_nexus_fi ): nexus_writer_1, nexus_writer_2 = dummy_nexus_writers_with_more_images - nexus_writer_1.create_nexus_file() - nexus_writer_2.create_nexus_file() + nexus_writer_1.create_nexus_file(np.uint16) + nexus_writer_2.create_nexus_file(np.uint16) for filename in [nexus_writer_1.nexus_file, nexus_writer_1.master_file]: with h5py.File(filename, "r") as written_nexus_file: @@ -343,8 +343,8 @@ def test_given_data_files_not_yet_written_when_nexus_files_created_then_nexus_fi nexus_writer_1, nexus_writer_2, ): - nexus_writer_1.create_nexus_file() - nexus_writer_2.create_nexus_file() + nexus_writer_1.create_nexus_file(np.uint16) + nexus_writer_2.create_nexus_file(np.uint16) for filename in [ nexus_writer_1.nexus_file, From 9614e0e949f0f1f1cf4930502442c965ce74f5d5 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Wed, 10 Apr 2024 17:09:35 +0100 Subject: [PATCH 3/3] o(#1245) Appease pyright --- .../callbacks/xray_centre/test_nexus_handler.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/external_interaction/callbacks/xray_centre/test_nexus_handler.py b/tests/unit_tests/external_interaction/callbacks/xray_centre/test_nexus_handler.py index 57be8805d..6078125ee 100644 --- a/tests/unit_tests/external_interaction/callbacks/xray_centre/test_nexus_handler.py +++ b/tests/unit_tests/external_interaction/callbacks/xray_centre/test_nexus_handler.py @@ -78,8 +78,12 @@ def test_given_different_bit_depths_then_writers_created_wth_correct_VDS_size( assert nexus_handler.nexus_writer_1 is not None assert nexus_handler.nexus_writer_2 is not None - nexus_handler.nexus_writer_1.create_nexus_file.assert_called_once_with(vds_type) - nexus_handler.nexus_writer_2.create_nexus_file.assert_called_once_with(vds_type) + nexus_handler.nexus_writer_1.create_nexus_file.assert_called_once_with( # type:ignore + vds_type + ) + nexus_handler.nexus_writer_2.create_nexus_file.assert_called_once_with( # type:ignore + vds_type + ) @patch("hyperion.external_interaction.callbacks.xray_centre.nexus_callback.NexusWriter")