From 801a531128329df71bdc3127384450dd531d00ee Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Tue, 30 Jul 2024 09:01:33 +0100 Subject: [PATCH] (#842) Move the ispyb activation so that messages can be logged from pin tip detection --- .../grid_detect_then_xray_centre_plan.py | 19 +- .../pin_centre_then_xray_centre_plan.py | 30 +-- .../test_ispyb_dev_connection.py | 2 +- tests/unit_tests/conftest.py | 57 ++++++ .../test_grid_detect_then_xray_centre_plan.py | 187 +++++++++++++++--- .../test_pin_centre_then_xray_centre_plan.py | 42 ++++ .../callbacks/conftest.py | 48 +---- 7 files changed, 283 insertions(+), 102 deletions(-) diff --git a/src/hyperion/experiment_plans/grid_detect_then_xray_centre_plan.py b/src/hyperion/experiment_plans/grid_detect_then_xray_centre_plan.py index 88910bc9e..0ed219f3e 100644 --- a/src/hyperion/experiment_plans/grid_detect_then_xray_centre_plan.py +++ b/src/hyperion/experiment_plans/grid_detect_then_xray_centre_plan.py @@ -108,16 +108,6 @@ def detect_grid_and_do_gridscan( composite: GridDetectThenXRayCentreComposite, parameters: GridScanWithEdgeDetect, oav_params: OAVParameters, -): - yield from ispyb_activation_wrapper( - _detect_grid_and_do_gridscan(composite, parameters, oav_params), parameters - ) - - -def _detect_grid_and_do_gridscan( - composite: GridDetectThenXRayCentreComposite, - parameters: GridScanWithEdgeDetect, - oav_params: OAVParameters, ): assert composite.aperture_scatterguard.aperture_positions is not None @@ -202,10 +192,13 @@ def grid_detect_then_xray_centre( oav_params = OAVParameters("xrayCentring", oav_config) - plan_to_perform = detect_grid_and_do_gridscan( - composite, + plan_to_perform = ispyb_activation_wrapper( + detect_grid_and_do_gridscan( + composite, + parameters, + oav_params, + ), parameters, - oav_params, ) return start_preparing_data_collection_then_do_plan( diff --git a/src/hyperion/experiment_plans/pin_centre_then_xray_centre_plan.py b/src/hyperion/experiment_plans/pin_centre_then_xray_centre_plan.py index d0c769aea..15a5ff68e 100644 --- a/src/hyperion/experiment_plans/pin_centre_then_xray_centre_plan.py +++ b/src/hyperion/experiment_plans/pin_centre_then_xray_centre_plan.py @@ -17,6 +17,9 @@ PinTipCentringComposite, pin_tip_centre_plan, ) +from hyperion.external_interaction.callbacks.xray_centre.ispyb_callback import ( + ispyb_activation_wrapper, +) from hyperion.log import LOGGER from hyperion.parameters.constants import CONST from hyperion.parameters.gridscan import ( @@ -61,21 +64,24 @@ def pin_centre_then_xray_centre_plan( pin_tip_detection=composite.pin_tip_detection, ) - yield from pin_tip_centre_plan( - pin_tip_centring_composite, - parameters.tip_offset_um, - oav_config_file, - ) + def _pin_centre_then_xray_centre_plan(): + yield from pin_tip_centre_plan( + pin_tip_centring_composite, + parameters.tip_offset_um, + oav_config_file, + ) - grid_detect_params = create_parameters_for_grid_detection(parameters) + grid_detect_params = create_parameters_for_grid_detection(parameters) - oav_params = OAVParameters("xrayCentring", oav_config_file) + oav_params = OAVParameters("xrayCentring", oav_config_file) - yield from detect_grid_and_do_gridscan( - composite, - grid_detect_params, - oav_params, - ) + yield from detect_grid_and_do_gridscan( + composite, + grid_detect_params, + oav_params, + ) + + yield from ispyb_activation_wrapper(_pin_centre_then_xray_centre_plan(), parameters) def pin_tip_centre_then_xray_centre( diff --git a/tests/system_tests/external_interaction/test_ispyb_dev_connection.py b/tests/system_tests/external_interaction/test_ispyb_dev_connection.py index 01c57a171..97ab98451 100644 --- a/tests/system_tests/external_interaction/test_ispyb_dev_connection.py +++ b/tests/system_tests/external_interaction/test_ispyb_dev_connection.py @@ -784,7 +784,7 @@ def test_ispyb_deposition_in_rotation_plan( assert dcid is not None assert ( fetch_comment(dcid) - == "Sample position: (1.0, 2.0, 3.0) test Aperture: Small. " + == "Sample position (µm): (1000, 2000, 3000) test Aperture: Small. " ) expected_values = EXPECTED_DATACOLLECTION_FOR_ROTATION | { diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index ad994c979..904f230c1 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -2,6 +2,9 @@ from unittest.mock import patch import pytest +from event_model import Event, EventDescriptor + +from hyperion.parameters.constants import CONST BANNED_PATHS = [Path("/dls"), Path("/dls_sw")] @@ -21,3 +24,57 @@ def patched_open(*args, **kwargs): with patch("builtins.open", side_effect=patched_open): yield [] + + +class OavGridSnapshotTestEvents: + test_descriptor_document_oav_snapshot: EventDescriptor = { + "uid": "b5ba4aec-de49-4970-81a4-b4a847391d34", + "run_start": "d8bee3ee-f614-4e7a-a516-25d6b9e87ef3", + "name": CONST.DESCRIPTORS.OAV_GRID_SNAPSHOT_TRIGGERED, + } # type: ignore + test_event_document_oav_snapshot_xy: Event = { + "descriptor": "b5ba4aec-de49-4970-81a4-b4a847391d34", + "time": 1666604299.828203, + "timestamps": {}, + "seq_num": 1, + "uid": "29033ecf-e052-43dd-98af-c7cdd62e8174", + "data": { + "oav_grid_snapshot_top_left_x": 50, + "oav_grid_snapshot_top_left_y": 100, + "oav_grid_snapshot_num_boxes_x": 40, + "oav_grid_snapshot_num_boxes_y": 20, + "oav_grid_snapshot_microns_per_pixel_x": 1.25, + "oav_grid_snapshot_microns_per_pixel_y": 1.5, + "oav_grid_snapshot_box_width": 0.1 * 1000 / 1.25, # size in pixels + "oav_grid_snapshot_last_path_full_overlay": "test_1_y", + "oav_grid_snapshot_last_path_outer": "test_2_y", + "oav_grid_snapshot_last_saved_path": "test_3_y", + "smargon-omega": 0, + "smargon-x": 0, + "smargon-y": 0, + "smargon-z": 0, + }, + } + test_event_document_oav_snapshot_xz: Event = { + "descriptor": "b5ba4aec-de49-4970-81a4-b4a847391d34", + "time": 1666604299.828203, + "timestamps": {}, + "seq_num": 1, + "uid": "29033ecf-e052-43dd-98af-c7cdd62e8174", + "data": { + "oav_grid_snapshot_top_left_x": 50, + "oav_grid_snapshot_top_left_y": 0, + "oav_grid_snapshot_num_boxes_x": 40, + "oav_grid_snapshot_num_boxes_y": 10, + "oav_grid_snapshot_box_width": 0.1 * 1000 / 1.25, # size in pixels + "oav_grid_snapshot_last_path_full_overlay": "test_1_z", + "oav_grid_snapshot_last_path_outer": "test_2_z", + "oav_grid_snapshot_last_saved_path": "test_3_z", + "oav_grid_snapshot_microns_per_pixel_x": 1.25, + "oav_grid_snapshot_microns_per_pixel_y": 1.5, + "smargon-omega": -90, + "smargon-x": 0, + "smargon-y": 0, + "smargon-z": 0, + }, + } diff --git a/tests/unit_tests/experiment_plans/test_grid_detect_then_xray_centre_plan.py b/tests/unit_tests/experiment_plans/test_grid_detect_then_xray_centre_plan.py index fa9a3ee18..765ce299e 100644 --- a/tests/unit_tests/experiment_plans/test_grid_detect_then_xray_centre_plan.py +++ b/tests/unit_tests/experiment_plans/test_grid_detect_then_xray_centre_plan.py @@ -3,7 +3,9 @@ import bluesky.plan_stubs as bps import pytest +from bluesky import Msg from bluesky.run_engine import RunEngine +from bluesky.simulators import RunEngineSimulator, assert_message_and_return_remaining from dodal.beamlines import i03 from dodal.devices.backlight import BacklightPosition from dodal.devices.eiger import EigerDetector @@ -17,9 +19,14 @@ detect_grid_and_do_gridscan, grid_detect_then_xray_centre, ) +from hyperion.external_interaction.callbacks.xray_centre.ispyb_callback import ( + ispyb_activation_wrapper, +) from hyperion.parameters.constants import CONST from hyperion.parameters.gridscan import GridScanWithEdgeDetect, ThreeDGridScan +from ..conftest import OavGridSnapshotTestEvents + def _fake_grid_detection( devices: OavGridDetectionComposite, @@ -82,6 +89,20 @@ def test_full_grid_scan(test_fgs_params, test_config_files): assert isinstance(plan, Generator) +@pytest.fixture +def grid_detect_devices_with_oav_config_params( + grid_detect_devices: GridDetectThenXRayCentreComposite, test_config_files +) -> GridDetectThenXRayCentreComposite: + grid_detect_devices.oav.parameters = OAVConfigParams( + test_config_files["zoom_params_file"], test_config_files["display_config"] + ) + grid_detect_devices.oav.parameters.micronsPerXPixel = 0.806 + grid_detect_devices.oav.parameters.micronsPerYPixel = 0.806 + grid_detect_devices.oav.parameters.beam_centre_i = 549 + grid_detect_devices.oav.parameters.beam_centre_j = 347 + return grid_detect_devices + + @patch( "hyperion.experiment_plans.grid_detect_then_xray_centre_plan.grid_detection_plan", autospec=True, @@ -93,32 +114,33 @@ def test_full_grid_scan(test_fgs_params, test_config_files): async def test_detect_grid_and_do_gridscan( mock_flyscan_xray_centre_plan: MagicMock, mock_grid_detection_plan: MagicMock, - grid_detect_devices: GridDetectThenXRayCentreComposite, + grid_detect_devices_with_oav_config_params: GridDetectThenXRayCentreComposite, RE: RunEngine, smargon: Smargon, test_full_grid_scan_params: GridScanWithEdgeDetect, test_config_files: Dict, ): mock_grid_detection_plan.side_effect = _fake_grid_detection - grid_detect_devices.oav.parameters = OAVConfigParams( - test_config_files["zoom_params_file"], test_config_files["display_config"] + assert ( + grid_detect_devices_with_oav_config_params.aperture_scatterguard.aperture_positions + is not None ) - grid_detect_devices.oav.parameters.micronsPerXPixel = 0.806 - grid_detect_devices.oav.parameters.micronsPerYPixel = 0.806 - grid_detect_devices.oav.parameters.beam_centre_i = 549 - grid_detect_devices.oav.parameters.beam_centre_j = 347 - assert grid_detect_devices.aperture_scatterguard.aperture_positions is not None with patch.object( - grid_detect_devices.aperture_scatterguard, "set", MagicMock() + grid_detect_devices_with_oav_config_params.aperture_scatterguard, + "set", + MagicMock(), ) as mock_aperture_scatterguard: RE( - detect_grid_and_do_gridscan( - grid_detect_devices, - parameters=test_full_grid_scan_params, - oav_params=OAVParameters( - "xrayCentring", test_config_files["oav_config_json"] + ispyb_activation_wrapper( + detect_grid_and_do_gridscan( + grid_detect_devices_with_oav_config_params, + parameters=test_full_grid_scan_params, + oav_params=OAVParameters( + "xrayCentring", test_config_files["oav_config_json"] + ), ), + test_full_grid_scan_params, ) ) # Verify we called the grid detection plan @@ -126,13 +148,13 @@ async def test_detect_grid_and_do_gridscan( # Check backlight was moved OUT assert ( - await grid_detect_devices.backlight.position.get_value() + await grid_detect_devices_with_oav_config_params.backlight.position.get_value() == BacklightPosition.OUT ) # Check aperture was changed to SMALL mock_aperture_scatterguard.assert_called_once_with( - grid_detect_devices.aperture_scatterguard.aperture_positions.SMALL + grid_detect_devices_with_oav_config_params.aperture_scatterguard.aperture_positions.SMALL ) # Check we called out to underlying fast grid scan plan @@ -151,7 +173,7 @@ def test_when_full_grid_scan_run_then_parameters_sent_to_fgs_as_expected( mock_flyscan_xray_centre_plan: MagicMock, mock_grid_detection_plan: MagicMock, eiger: EigerDetector, - grid_detect_devices: GridDetectThenXRayCentreComposite, + grid_detect_devices_with_oav_config_params: GridDetectThenXRayCentreComposite, RE: RunEngine, test_full_grid_scan_params: GridScanWithEdgeDetect, test_config_files: Dict, @@ -161,22 +183,19 @@ def test_when_full_grid_scan_run_then_parameters_sent_to_fgs_as_expected( mock_grid_detection_plan.side_effect = _fake_grid_detection - grid_detect_devices.oav.parameters = OAVConfigParams( - test_config_files["zoom_params_file"], test_config_files["display_config"] - ) - grid_detect_devices.oav.parameters.micronsPerXPixel = 0.806 - grid_detect_devices.oav.parameters.micronsPerYPixel = 0.806 - grid_detect_devices.oav.parameters.beam_centre_i = 549 - grid_detect_devices.oav.parameters.beam_centre_j = 347 - with patch.object(eiger.do_arm, "set", MagicMock()), patch.object( - grid_detect_devices.aperture_scatterguard, "set", MagicMock() + grid_detect_devices_with_oav_config_params.aperture_scatterguard, + "set", + MagicMock(), ): RE( - detect_grid_and_do_gridscan( - grid_detect_devices, - parameters=test_full_grid_scan_params, - oav_params=oav_params, + ispyb_activation_wrapper( + detect_grid_and_do_gridscan( + grid_detect_devices_with_oav_config_params, + parameters=test_full_grid_scan_params, + oav_params=oav_params, + ), + test_full_grid_scan_params, ) ) @@ -189,3 +208,111 @@ def test_when_full_grid_scan_run_then_parameters_sent_to_fgs_as_expected( # Parameters can be serialized params.json() + + +@patch( + "hyperion.experiment_plans.grid_detect_then_xray_centre_plan.grid_detection_plan", + autospec=True, +) +@patch( + "hyperion.experiment_plans.grid_detect_then_xray_centre_plan.flyscan_xray_centre", + autospec=True, +) +def test_detect_grid_and_do_gridscan_does_not_activate_ispyb_callback( + mock_flyscan_xray_centre, + mock_grid_detection_plan, + grid_detect_devices_with_oav_config_params: GridDetectThenXRayCentreComposite, + sim_run_engine: RunEngineSimulator, + test_full_grid_scan_params: GridScanWithEdgeDetect, + test_config_files, +): + mock_grid_detection_plan.return_value = iter([Msg("save_oav_grids")]) + sim_run_engine.add_handler_for_callback_subscribes() + sim_run_engine.add_callback_handler_for_multiple( + "save_oav_grids", + [ + [ + ( + "descriptor", + OavGridSnapshotTestEvents.test_descriptor_document_oav_snapshot, # type: ignore + ), + ( + "event", + OavGridSnapshotTestEvents.test_event_document_oav_snapshot_xy, # type: ignore + ), + ( + "event", + OavGridSnapshotTestEvents.test_event_document_oav_snapshot_xz, # type: ignore + ), + ] + ], + ) + + msgs = sim_run_engine.simulate_plan( + detect_grid_and_do_gridscan( + grid_detect_devices_with_oav_config_params, + test_full_grid_scan_params, + OAVParameters("xrayCentring", test_config_files["oav_config_json"]), + ) + ) + + activations = [ + msg + for msg in msgs + if msg.command == "open_run" + and "GridscanISPyBCallback" in msg.kwargs["activate_callbacks"] + ] + assert not activations + + +@patch( + "hyperion.experiment_plans.grid_detect_then_xray_centre_plan.grid_detection_plan", + autospec=True, +) +@patch( + "hyperion.experiment_plans.grid_detect_then_xray_centre_plan.flyscan_xray_centre", + autospec=True, +) +def test_grid_detect_then_xray_centre_activates_ispyb_callback( + mock_flyscan_xray_centre, + mock_grid_detection_plan, + sim_run_engine: RunEngineSimulator, + grid_detect_devices_with_oav_config_params: GridDetectThenXRayCentreComposite, + test_full_grid_scan_params: GridScanWithEdgeDetect, + test_config_files, +): + mock_grid_detection_plan.return_value = iter([Msg("save_oav_grids")]) + + sim_run_engine.add_handler_for_callback_subscribes() + sim_run_engine.add_callback_handler_for_multiple( + "save_oav_grids", + [ + [ + ( + "descriptor", + OavGridSnapshotTestEvents.test_descriptor_document_oav_snapshot, # type: ignore + ), + ( + "event", + OavGridSnapshotTestEvents.test_event_document_oav_snapshot_xy, # type: ignore + ), + ( + "event", + OavGridSnapshotTestEvents.test_event_document_oav_snapshot_xz, # type: ignore + ), + ] + ], + ) + msgs = sim_run_engine.simulate_plan( + grid_detect_then_xray_centre( + grid_detect_devices_with_oav_config_params, + test_full_grid_scan_params, + test_config_files["oav_config_json"], + ) + ) + + assert_message_and_return_remaining( + msgs, + lambda msg: msg.command == "open_run" + and "GridscanISPyBCallback" in msg.kwargs["activate_callbacks"], + ) diff --git a/tests/unit_tests/experiment_plans/test_pin_centre_then_xray_centre_plan.py b/tests/unit_tests/experiment_plans/test_pin_centre_then_xray_centre_plan.py index 7ff6571a7..744b55d3f 100644 --- a/tests/unit_tests/experiment_plans/test_pin_centre_then_xray_centre_plan.py +++ b/tests/unit_tests/experiment_plans/test_pin_centre_then_xray_centre_plan.py @@ -147,3 +147,45 @@ def add_handlers_to_simulate_detector_motion(msg: Msg): lambda msg: msg.command == "open_run" and msg.kwargs["subplan_name"] == "do_fgs", ) + + +@patch( + "hyperion.experiment_plans.pin_centre_then_xray_centre_plan.pin_tip_centre_plan", + autospec=True, +) +@patch( + "hyperion.experiment_plans.pin_centre_then_xray_centre_plan.detect_grid_and_do_gridscan", + autospec=True, +) +def test_pin_centre_then_xray_centre_plan_activates_ispyb_callback_before_pin_tip_centre_plan( + mock_detect_grid_and_do_gridscan, + mock_pin_tip_centre_plan, + sim_run_engine: RunEngineSimulator, + test_pin_centre_then_xray_centre_params: PinTipCentreThenXrayCentre, + test_config_files, +): + mock_detect_grid_and_do_gridscan.return_value = iter( + [Msg("detect_grid_and_do_gridscan")] + ) + mock_pin_tip_centre_plan.return_value = iter([Msg("pin_tip_centre_plan")]) + + msgs = sim_run_engine.simulate_plan( + pin_centre_then_xray_centre_plan( + MagicMock(), + test_pin_centre_then_xray_centre_params, + test_config_files["oav_config_json"], + ) + ) + + msgs = assert_message_and_return_remaining( + msgs, + lambda msg: msg.command == "open_run" + and "GridscanISPyBCallback" in msg.kwargs["activate_callbacks"], + ) + msgs = assert_message_and_return_remaining( + msgs, lambda msg: msg.command == "pin_tip_centre_plan" + ) + msgs = assert_message_and_return_remaining( + msgs, lambda msg: msg.command == "detect_grid_and_do_gridscan" + ) + assert_message_and_return_remaining(msgs, lambda msg: msg.command == "close_run") diff --git a/tests/unit_tests/external_interaction/callbacks/conftest.py b/tests/unit_tests/external_interaction/callbacks/conftest.py index 761f1f0b6..ab934f77e 100644 --- a/tests/unit_tests/external_interaction/callbacks/conftest.py +++ b/tests/unit_tests/external_interaction/callbacks/conftest.py @@ -8,6 +8,7 @@ from tests.conftest import create_dummy_scan_spec from ....conftest import default_raw_params, raw_params_from_file +from ...conftest import OavGridSnapshotTestEvents def dummy_params(): @@ -32,7 +33,7 @@ def test_rotation_start_outer_document(dummy_rotation_params): } -class TestData: +class TestData(OavGridSnapshotTestEvents): DUMMY_TIME_STRING: str = "1970-01-01 00:00:00" GOOD_ISPYB_RUN_STATUS: str = "DataCollection Successful" BAD_ISPYB_RUN_STATUS: str = "DataCollection Unsuccessful" @@ -129,11 +130,6 @@ class TestData: "zocalo_environment": "dev_artemis", "scan_points": create_dummy_scan_spec(10, 20, 30), } - test_descriptor_document_oav_snapshot: EventDescriptor = { - "uid": "b5ba4aec-de49-4970-81a4-b4a847391d34", - "run_start": "d8bee3ee-f614-4e7a-a516-25d6b9e87ef3", - "name": CONST.DESCRIPTORS.OAV_GRID_SNAPSHOT_TRIGGERED, - } # type: ignore test_descriptor_document_oav_rotation_snapshot: EventDescriptor = { "uid": "c7d698ce-6d49-4c56-967e-7d081f964573", "run_start": "d8bee3ee-f614-4e7a-a516-25d6b9e87ef3", @@ -162,46 +158,6 @@ class TestData: "uid": "32d7c25c-c310-4292-ac78-36ce6509be3d", "data": {"oav_snapshot_last_saved_path": "snapshot_0"}, } - test_event_document_oav_snapshot_xy: Event = { - "descriptor": "b5ba4aec-de49-4970-81a4-b4a847391d34", - "time": 1666604299.828203, - "timestamps": {}, - "seq_num": 1, - "uid": "29033ecf-e052-43dd-98af-c7cdd62e8174", - "data": { - "oav_grid_snapshot_top_left_x": 50, - "oav_grid_snapshot_top_left_y": 100, - "oav_grid_snapshot_num_boxes_x": 40, - "oav_grid_snapshot_num_boxes_y": 20, - "oav_grid_snapshot_microns_per_pixel_x": 1.25, - "oav_grid_snapshot_microns_per_pixel_y": 1.5, - "oav_grid_snapshot_box_width": 0.1 * 1000 / 1.25, # size in pixels - "oav_grid_snapshot_last_path_full_overlay": "test_1_y", - "oav_grid_snapshot_last_path_outer": "test_2_y", - "oav_grid_snapshot_last_saved_path": "test_3_y", - "smargon-omega": 0, - }, - } - test_event_document_oav_snapshot_xz: Event = { - "descriptor": "b5ba4aec-de49-4970-81a4-b4a847391d34", - "time": 1666604299.828203, - "timestamps": {}, - "seq_num": 1, - "uid": "29033ecf-e052-43dd-98af-c7cdd62e8174", - "data": { - "oav_grid_snapshot_top_left_x": 50, - "oav_grid_snapshot_top_left_y": 0, - "oav_grid_snapshot_num_boxes_x": 40, - "oav_grid_snapshot_num_boxes_y": 10, - "oav_grid_snapshot_box_width": 0.1 * 1000 / 1.25, # size in pixels - "oav_grid_snapshot_last_path_full_overlay": "test_1_z", - "oav_grid_snapshot_last_path_outer": "test_2_z", - "oav_grid_snapshot_last_saved_path": "test_3_z", - "oav_grid_snapshot_microns_per_pixel_x": 1.25, - "oav_grid_snapshot_microns_per_pixel_y": 1.5, - "smargon-omega": -90, - }, - } test_event_document_pre_data_collection: Event = { "descriptor": "bd45c2e5-2b85-4280-95d7-a9a15800a78b", "time": 1666604299.828203,