Merge pull request #1249 from DiamondLightSource/1227_filter_negative…

…s_from_edge_detection

Fix occassional very large grids

src/hyperion/experiment_plans/oav_grid_detection_plan.py

@@ -2,7 +2,7 @@
 
 import dataclasses
 import math
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Tuple
 
 import bluesky.plan_stubs as bps
 import bluesky.preprocessors as bpp
@@ -11,10 +11,10 @@
 from dodal.devices.backlight import Backlight
 from dodal.devices.oav.oav_detector import OAV
 from dodal.devices.oav.pin_image_recognition import PinTipDetection
+from dodal.devices.oav.pin_image_recognition.utils import NONE_VALUE
 from dodal.devices.smargon import Smargon
 
 from hyperion.device_setup_plans.setup_oav import (
-    get_move_required_so_that_beam_is_at_pixel,
     pre_centring_setup_oav,
     wait_for_tip_to_be_found,
 )
@@ -40,6 +40,23 @@ def create_devices(context: BlueskyContext) -> OavGridDetectionComposite:
     return device_composite_from_context(context, OavGridDetectionComposite)
 
 
+def get_min_and_max_y_of_pin(
+    top: np.ndarray, bottom: np.ndarray, full_image_height_px: int
+) -> Tuple[int, int]:
+    """Gives the minimum and maximum y that would cover the whole pin.
+
+    First filters out where no edge was found or the edge covers the full image.
+    If this results in no edges found then returns a min/max that covers the full image
+    """
+    filtered_top = top[np.where((top != 0) & (top != NONE_VALUE))]
+    min_y = min(filtered_top) if len(filtered_top) else 0
+    filtered_bottom = bottom[
+        np.where((bottom != full_image_height_px) & (bottom != NONE_VALUE))
+    ]
+    max_y = max(filtered_bottom) if len(filtered_bottom) else full_image_height_px
+    return min_y, max_y
+
+
 @bpp.run_decorator()
 def grid_detection_plan(
     composite: OavGridDetectionComposite,
@@ -54,8 +71,8 @@ def grid_detection_plan(
     encompass the whole of the sample as it appears in the OAV.
 
     Args:
-        parameters (OAVParamaters): Object containing paramters for setting up the OAV
-        out_parameters (GridScanParams): The returned parameters for the gridscan
+        composite (OavGridDetectionComposite): Composite containing devices for doing a grid detection.
+        parameters (OAVParameters): Object containing parameters for setting up the OAV
         snapshot_template (str): A template for the name of the snapshots, expected to be filled in with an angle
         snapshot_dir (str): The location to save snapshots
         grid_width_microns (int): The width of the grid to scan in microns
@@ -74,9 +91,6 @@ def grid_detection_plan(
 
     LOGGER.info("OAV Centring: Camera set up")
 
-    start_positions = []
-    box_numbers = []
-
     assert isinstance(oav.parameters.micronsPerXPixel, float)
     box_size_x_pixels = box_size_um / oav.parameters.micronsPerXPixel
     assert isinstance(oav.parameters.micronsPerYPixel, float)
@@ -108,16 +122,10 @@ def grid_detection_plan(
         LOGGER.info(f"OAV Edge detection top: {list(top_edge)}")
         LOGGER.info(f"OAV Edge detection bottom: {list(bottom_edge)}")
 
-        # the edge detection line can jump to the edge of the image sometimes, filter
-        # those points out, and if empty after filter use the whole image
-        filtered_top = list(top_edge[top_edge != 0]) or [0]
-        filtered_bottom = list(bottom_edge[bottom_edge != full_image_height_px]) or [
-            full_image_height_px
-        ]
-        LOGGER.info(f"OAV Edge detection filtered top: {filtered_top}")
-        LOGGER.info(f"OAV Edge detection filtered bottom: {filtered_bottom}")
-        min_y = min(filtered_top)
-        max_y = max(filtered_bottom)
+        min_y, max_y = get_min_and_max_y_of_pin(
+            top_edge, bottom_edge, full_image_height_px
+        )
+
         grid_height_px = max_y - min_y
 
         y_steps: int = math.ceil(grid_height_px / box_size_y_pixels)
@@ -131,23 +139,19 @@ def grid_detection_plan(
             y_steps += 1
             min_y -= box_size_y_pixels / 2
             max_y += box_size_y_pixels / 2
-            grid_height_px += 1
+            grid_height_px += box_size_y_pixels
 
         LOGGER.info(f"Drawing snapshot {grid_width_pixels} by {grid_height_px}")
 
-        boxes = (
-            math.ceil(grid_width_pixels / box_size_x_pixels),
-            y_steps,
-        )
-        box_numbers.append(boxes)
+        x_steps = math.ceil(grid_width_pixels / box_size_x_pixels)
 
         upper_left = (tip_x_px, min_y)
 
         yield from bps.abs_set(oav.snapshot.top_left_x, upper_left[0])
         yield from bps.abs_set(oav.snapshot.top_left_y, upper_left[1])
         yield from bps.abs_set(oav.snapshot.box_width, box_size_x_pixels)
-        yield from bps.abs_set(oav.snapshot.num_boxes_x, boxes[0])
-        yield from bps.abs_set(oav.snapshot.num_boxes_y, boxes[1])
+        yield from bps.abs_set(oav.snapshot.num_boxes_x, x_steps)
+        yield from bps.abs_set(oav.snapshot.num_boxes_y, y_steps)
 
         snapshot_filename = snapshot_template.format(angle=abs(angle))
 
@@ -161,23 +165,6 @@ def grid_detection_plan(
         yield from bps.read(smargon)
         yield from bps.save()
 
-        # The first frame is taken at the centre of the first box
-        centre_of_first_box = (
-            int(upper_left[0] + box_size_x_pixels / 2),
-            int(upper_left[1] + box_size_y_pixels / 2),
-        )
-
-        position = yield from get_move_required_so_that_beam_is_at_pixel(
-            smargon, centre_of_first_box, oav.parameters
+        LOGGER.info(
+            f"Grid calculated at {angle}: {x_steps}px by {y_steps}px starting at {upper_left}px"
         )
-        start_positions.append(position)
-
-    LOGGER.info(
-        f"Calculated start position {start_positions[0][0], start_positions[0][1], start_positions[1][2]}"
-    )
-
-    LOGGER.info(
-        f"Calculated number of steps {box_numbers[0][0], box_numbers[0][1], box_numbers[1][1]}"
-    )
-
-    LOGGER.info(f"Step sizes: {box_size_um, box_size_um, box_size_um}")

tests/unit_tests/experiment_plans/test_grid_detection_plan.py

@@ -10,12 +10,13 @@
 from dodal.devices.oav.oav_detector import OAVConfigParams
 from dodal.devices.oav.oav_parameters import OAVParameters
 from dodal.devices.oav.pin_image_recognition import PinTipDetection
-from dodal.devices.oav.pin_image_recognition.utils import SampleLocation
+from dodal.devices.oav.pin_image_recognition.utils import NONE_VALUE, SampleLocation
 from dodal.devices.smargon import Smargon
 
 from hyperion.exceptions import WarningException
 from hyperion.experiment_plans.oav_grid_detection_plan import (
     OavGridDetectionComposite,
+    get_min_and_max_y_of_pin,
     grid_detection_plan,
 )
 from hyperion.external_interaction.callbacks.grid_detection_callback import (
@@ -360,3 +361,51 @@ def record_set(msg: Msg):
 
     assert abs_sets["snapshot.top_left_y"][0] == expected_min_y
     assert abs_sets["snapshot.num_boxes_y"][0] == expected_y_steps
+
+
+@pytest.mark.parametrize(
+    "top, bottom, expected_min, expected_max",
+    [
+        (np.array([1, 2, 5]), np.array([8, 9, 40]), 1, 40),
+        (np.array([9, 6, 10]), np.array([152, 985, 72]), 6, 985),
+        (np.array([5, 1]), np.array([999, 1056, 896, 10]), 1, 1056),
+    ],
+)
+def test_given_array_with_valid_top_and_bottom_then_min_and_max_as_expected(
+    top, bottom, expected_min, expected_max
+):
+    min_y, max_y = get_min_and_max_y_of_pin(top, bottom, 100)
+    assert min_y == expected_min
+    assert max_y == expected_max
+
+
+@pytest.mark.parametrize(
+    "top, bottom, expected_min, expected_max",
+    [
+        (np.array([1, 2, NONE_VALUE]), np.array([8, 9, 40]), 1, 40),
+        (np.array([6, NONE_VALUE, 10]), np.array([152, 985, NONE_VALUE]), 6, 985),
+        (np.array([1, 5]), np.array([999, 1056, NONE_VALUE, 10]), 1, 1056),
+    ],
+)
+def test_given_array_with_some_invalid_top_and_bottom_sections_then_min_and_max_as_expected(
+    top, bottom, expected_min, expected_max
+):
+    min_y, max_y = get_min_and_max_y_of_pin(top, bottom, 100)
+    assert min_y == expected_min
+    assert max_y == expected_max
+
+
+@pytest.mark.parametrize(
+    "top, bottom, expected_min, expected_max",
+    [
+        (np.array([NONE_VALUE, 0, NONE_VALUE]), np.array([100, NONE_VALUE]), 0, 100),
+        (np.array([NONE_VALUE, NONE_VALUE]), np.array([100, NONE_VALUE]), 0, 100),
+        (np.array([0, NONE_VALUE]), np.array([NONE_VALUE]), 0, 100),
+    ],
+)
+def test_given_array_with_all_invalid_top_and_bottom_sections_then_min_and_max_is_full_image(
+    top, bottom, expected_min, expected_max
+):
+    min_y, max_y = get_min_and_max_y_of_pin(top, bottom, 100)
+    assert min_y == expected_min
+    assert max_y == expected_max