Merge pull request #766 from DiamondLightSource/759_fix_full_grid_sca…

…n_issues

Fix parameter issues in full_grid_scan

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,
 )
@@ -127,9 +125,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 = (
@@ -145,7 +143,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(
@@ -156,7 +153,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(

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,
@@ -17,6 +18,9 @@
     start_arming_then_do_grid,
     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,
 )
@@ -75,22 +79,22 @@ 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)
 
 
-@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 +103,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(
         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,
@@ -125,7 +123,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
@@ -139,7 +137,69 @@ 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,
+                aperture_scatterguard=aperture_scatterguard,
+                detector_motion=detector_motion,
+                oav_params=OAVParameters("xrayCentring", **test_config_files),
+            )
+        )
+
+        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",
+        ]
 
 
 @patch("artemis.experiment_plans.full_grid_scan.grid_detection_plan")

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)