From 99053aa5d1521cda3539033ae4775f6afc11a6a7 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Thu, 29 Jun 2023 18:01:21 +0100 Subject: [PATCH 1/2] (#759) Fix parameter issues in full_grid_scan --- .../experiment_plans/full_grid_scan.py | 13 +-- .../tests/test_full_grid_scan_plan.py | 93 ++++++++++++++++--- .../tests/test_fgs_internal_parameters.py | 1 + 3 files changed, 85 insertions(+), 22 deletions(-) diff --git a/src/artemis/experiment_plans/full_grid_scan.py b/src/artemis/experiment_plans/full_grid_scan.py index 0f3f12138..dab55673a 100644 --- a/src/artemis/experiment_plans/full_grid_scan.py +++ b/src/artemis/experiment_plans/full_grid_scan.py @@ -2,6 +2,7 @@ from typing import TYPE_CHECKING, Callable +import numpy as np from bluesky import plan_stubs as bps from bluesky import preprocessors as bpp from dodal.beamlines import i03 @@ -19,9 +20,6 @@ create_devices as oav_create_devices, ) from artemis.experiment_plans.oav_grid_detection_plan import grid_detection_plan -from artemis.external_interaction.callbacks.fgs.fgs_callback_collection import ( - FGSCallbackCollection, -) from artemis.external_interaction.callbacks.oav_snapshot_callback import ( OavSnapshotCallback, ) @@ -108,9 +106,9 @@ def run_grid_detection_plan( ) # Hack because GDA only passes 3 values to ispyb - out_upper_left = oav_callback.out_upper_left[0] + [ - oav_callback.out_upper_left[1][1] - ] + out_upper_left = np.array( + oav_callback.out_upper_left[0] + [oav_callback.out_upper_left[1][1]] + ) # Hack because the callback returns the list in inverted order parameters.artemis_params.ispyb_params.xtal_snapshots_omega_start = ( @@ -126,7 +124,6 @@ def run_grid_detection_plan( parameters.artemis_params.detector_params.num_triggers = fgs_params.get_num_images() LOGGER.info(f"Parameters for FGS: {parameters}") - subscriptions = FGSCallbackCollection.from_params(parameters) yield from bps.abs_set(backlight.pos, Backlight.OUT) LOGGER.info( @@ -137,7 +134,7 @@ def run_grid_detection_plan( ) yield from wait_for_det_to_finish_moving(detector_motion) - yield from fgs_get_plan(parameters, subscriptions) + yield from fgs_get_plan(parameters) def get_plan( diff --git a/src/artemis/experiment_plans/tests/test_full_grid_scan_plan.py b/src/artemis/experiment_plans/tests/test_full_grid_scan_plan.py index 6a0c410a8..7ec94fcb6 100644 --- a/src/artemis/experiment_plans/tests/test_full_grid_scan_plan.py +++ b/src/artemis/experiment_plans/tests/test_full_grid_scan_plan.py @@ -9,6 +9,7 @@ from dodal.devices.detector_motion import DetectorMotion from dodal.devices.eiger import EigerDetector from dodal.devices.oav.oav_parameters import OAVParameters +from numpy.testing import assert_array_equal from artemis.experiment_plans.full_grid_scan import ( create_devices, @@ -16,6 +17,9 @@ get_plan, wait_for_det_to_finish_moving, ) +from artemis.external_interaction.callbacks.oav_snapshot_callback import ( + OavSnapshotCallback, +) from artemis.parameters.plan_specific.grid_scan_with_edge_detect_params import ( GridScanWithEdgeDetectInternalParameters, ) @@ -84,12 +88,15 @@ def test_get_plan(test_fgs_params, test_config_files, mock_subscriptions): assert isinstance(plan, Generator) -@patch("artemis.experiment_plans.full_grid_scan.wait_for_det_to_finish_moving") -@patch("artemis.experiment_plans.full_grid_scan.grid_detection_plan") -@patch("artemis.experiment_plans.full_grid_scan.fgs_get_plan") -@patch("artemis.experiment_plans.full_grid_scan.OavSnapshotCallback") +@patch( + "artemis.experiment_plans.full_grid_scan.wait_for_det_to_finish_moving", + autospec=True, +) +@patch("artemis.experiment_plans.full_grid_scan.grid_detection_plan", autospec=True) +@patch("artemis.experiment_plans.full_grid_scan.fgs_get_plan", autospec=True) +@patch("artemis.experiment_plans.full_grid_scan.OavSnapshotCallback", autospec=True) def test_detect_grid_and_do_gridscan( - mock_oav_callback: MagicMock, + mock_oav_callback_init: MagicMock, mock_fast_grid_scan_plan: MagicMock, mock_grid_detection_plan: MagicMock, mock_wait_for_detector: MagicMock, @@ -99,19 +106,13 @@ def test_detect_grid_and_do_gridscan( aperture_scatterguard: ApertureScatterguard, RE: RunEngine, test_full_grid_scan_params: GridScanWithEdgeDetectInternalParameters, - mock_subscriptions: MagicMock, test_config_files: Dict, ): - mock_oav_callback.snapshot_filenames = [[], []] - mock_oav_callback.out_upper_left = [[1, 1], [1, 1]] mock_grid_detection_plan.side_effect = _fake_grid_detection with patch.object(eiger.do_arm, "set", MagicMock()) as mock_eiger_set, patch.object( aperture_scatterguard, "set", MagicMock() - ) as mock_aperture_scatterguard, patch( - "artemis.external_interaction.callbacks.fgs.fgs_callback_collection.FGSCallbackCollection.from_params", - return_value=mock_subscriptions, - ): + ) as mock_aperture_scatterguard: RE( detect_grid_and_do_gridscan( parameters=test_full_grid_scan_params, @@ -131,7 +132,7 @@ def test_detect_grid_and_do_gridscan( mock_grid_detection_plan.assert_called_once() # Verify callback to oav snaposhot was called - mock_oav_callback.assert_called_once() + mock_oav_callback_init.assert_called_once() # Check backlight was moved OUT assert backlight.pos.get() == Backlight.OUT @@ -145,4 +146,68 @@ def test_detect_grid_and_do_gridscan( mock_wait_for_detector.assert_called_once() # Check we called out to underlying fast grid scan plan - mock_fast_grid_scan_plan.assert_called_once_with(ANY, mock_subscriptions) + mock_fast_grid_scan_plan.assert_called_once_with(ANY) + + +@patch( + "artemis.experiment_plans.full_grid_scan.wait_for_det_to_finish_moving", + autospec=True, +) +@patch("artemis.experiment_plans.full_grid_scan.grid_detection_plan", autospec=True) +@patch("artemis.experiment_plans.full_grid_scan.fgs_get_plan", autospec=True) +@patch("artemis.experiment_plans.full_grid_scan.OavSnapshotCallback", autospec=True) +def test_when_full_grid_scan_run_then_parameters_sent_to_fgs_as_expected( + mock_oav_callback_init: MagicMock, + mock_fast_grid_scan_plan: MagicMock, + mock_grid_detection_plan: MagicMock, + _: MagicMock, + eiger: EigerDetector, + backlight: Backlight, + detector_motion: DetectorMotion, + aperture_scatterguard: ApertureScatterguard, + RE: RunEngine, + test_full_grid_scan_params: GridScanWithEdgeDetectInternalParameters, + test_config_files: Dict, +): + mock_oav_callback = OavSnapshotCallback() + mock_oav_callback.snapshot_filenames = [["a", "b", "c"], ["d", "e", "f"]] + mock_oav_callback.out_upper_left = [[1, 2], [1, 3]] + + mock_oav_callback_init.return_value = mock_oav_callback + + mock_grid_detection_plan.side_effect = _fake_grid_detection + + with patch.object(eiger.do_arm, "set", MagicMock()), patch.object( + aperture_scatterguard, "set", MagicMock() + ): + RE( + detect_grid_and_do_gridscan( + parameters=test_full_grid_scan_params, + backlight=backlight, + eiger=eiger, + aperture_scatterguard=aperture_scatterguard, + detector_motion=detector_motion, + oav_params=OAVParameters("xrayCentring", **test_config_files), + experiment_params=test_full_grid_scan_params.experiment_params, + ) + ) + + params: GridScanWithEdgeDetectInternalParameters = ( + mock_fast_grid_scan_plan.call_args[0][0] + ) + + # Parameters can be serialized + params.json() + + ispyb_params = params.artemis_params.ispyb_params + assert_array_equal(ispyb_params.upper_left, [1, 2, 3]) + assert ispyb_params.xtal_snapshots_omega_start == [ + "c", + "b", + "a", + ] + assert ispyb_params.xtal_snapshots_omega_end == [ + "f", + "e", + "d", + ] diff --git a/src/artemis/parameters/plan_specific/tests/test_fgs_internal_parameters.py b/src/artemis/parameters/plan_specific/tests/test_fgs_internal_parameters.py index 342264422..f41a7b9e9 100644 --- a/src/artemis/parameters/plan_specific/tests/test_fgs_internal_parameters.py +++ b/src/artemis/parameters/plan_specific/tests/test_fgs_internal_parameters.py @@ -11,6 +11,7 @@ def test_FGS_parameters_load_from_file(): "src/artemis/parameters/tests/test_data/good_test_parameters.json" ) internal_parameters = FGSInternalParameters(**params) + internal_parameters.json() assert isinstance(internal_parameters.experiment_params, GridScanParams) From 183bf0c0daa3aa8b138e5b8739c1682ef27433d8 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Thu, 29 Jun 2023 18:06:55 +0100 Subject: [PATCH 2/2] (#759) Fix broken test --- .../experiment_plans/tests/test_full_grid_scan_plan.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/artemis/experiment_plans/tests/test_full_grid_scan_plan.py b/src/artemis/experiment_plans/tests/test_full_grid_scan_plan.py index 7ec94fcb6..f8d65c740 100644 --- a/src/artemis/experiment_plans/tests/test_full_grid_scan_plan.py +++ b/src/artemis/experiment_plans/tests/test_full_grid_scan_plan.py @@ -78,11 +78,8 @@ def test_wait_for_detector(RE): RE(wait_for_det_to_finish_moving(d_m, 0.5)) -def test_get_plan(test_fgs_params, test_config_files, mock_subscriptions): - with patch("artemis.experiment_plans.full_grid_scan.i03"), patch( - "artemis.experiment_plans.full_grid_scan.FGSCallbackCollection.from_params", - lambda _: mock_subscriptions, - ): +def test_get_plan(test_fgs_params, test_config_files): + with patch("artemis.experiment_plans.full_grid_scan.i03"): plan = get_plan(test_fgs_params, test_config_files) assert isinstance(plan, Generator)