(#1327) ispyb omega_start, axis_start now read from hardware for grid…

…scans (#1445)

* (#1327) ispyb omega_start, axis_start now read from hardware for gridscans

* (#1327) Response to PR comments

* (#1327) refactor ispyb_callback classes as per PR discussion

src/hyperion/external_interaction/callbacks/common/ispyb_mapping.py

@@ -40,7 +40,6 @@ def populate_remaining_data_collection_info(
     data_collection_info.parent_id = data_collection_group_id
     data_collection_info.sample_id = params.sample_id
     data_collection_info.detector_id = I03_EIGER_DETECTOR
-    data_collection_info.axis_start = data_collection_info.omega_start
     data_collection_info.comments = comment
     data_collection_info.detector_distance = params.detector_params.detector_distance
     data_collection_info.exp_time = params.detector_params.exposure_time

src/hyperion/external_interaction/callbacks/ispyb_callback_base.py

@@ -12,16 +12,11 @@
 from hyperion.external_interaction.callbacks.plan_reactive_callback import (
     PlanReactiveCallback,
 )
-from hyperion.external_interaction.callbacks.xray_centre.ispyb_mapping import (
-    construct_comment_for_gridscan,
-)
 from hyperion.external_interaction.ispyb.data_model import (
-    DataCollectionGridInfo,
     DataCollectionInfo,
     DataCollectionPositionInfo,
     ScanDataInfo,
 )
-from hyperion.external_interaction.ispyb.ispyb_dataclass import Orientation
 from hyperion.external_interaction.ispyb.ispyb_store import (
     IspybIds,
     StoreInIspyb,
@@ -86,17 +81,13 @@ def activity_gated_event(self, doc: Event) -> Event:
         event_descriptor = self.descriptors.get(doc["descriptor"])
         if event_descriptor is None:
             ISPYB_LOGGER.warning(
-                f"Ispyb handler {self} recieved event doc {format_doc_for_log(doc)} and "
+                f"Ispyb handler {self} received event doc {format_doc_for_log(doc)} and "
                 "has no corresponding descriptor record"
             )
             return doc
         match event_descriptor.get("name"):
             case CONST.DESCRIPTORS.ISPYB_HARDWARE_READ:
                 scan_data_infos = self._handle_ispyb_hardware_read(doc)
-            case CONST.DESCRIPTORS.OAV_ROTATION_SNAPSHOT_TRIGGERED:
-                scan_data_infos = self._handle_oav_rotation_snapshot_triggered(doc)
-            case CONST.DESCRIPTORS.OAV_GRID_SNAPSHOT_TRIGGERED:
-                scan_data_infos = self._handle_oav_grid_snapshot_triggered(doc)
             case CONST.DESCRIPTORS.ISPYB_TRANSMISSION_FLUX_READ:
                 scan_data_infos = self._handle_ispyb_transmission_flux_read(doc)
             case _:
@@ -137,69 +128,6 @@ def _handle_ispyb_hardware_read(self, doc) -> Sequence[ScanDataInfo]:
         ISPYB_LOGGER.info("Updating ispyb data collection after hardware read.")
         return scan_data_infos
 
-    def _handle_oav_rotation_snapshot_triggered(self, doc) -> Sequence[ScanDataInfo]:
-        assert self.ispyb_ids.data_collection_ids, "No current data collection"
-        assert self.params, "ISPyB handler didn't recieve parameters!"
-        data = doc["data"]
-        self._oav_snapshot_event_idx += 1
-        data_collection_info = DataCollectionInfo(
-            **{
-                f"xtal_snapshot{self._oav_snapshot_event_idx}": data.get(
-                    "oav_snapshot_last_saved_path"
-                )
-            }
-        )
-        scan_data_info = ScanDataInfo(
-            data_collection_id=self.ispyb_ids.data_collection_ids[-1],
-            data_collection_info=data_collection_info,
-        )
-        return [scan_data_info]
-
-    def _handle_oav_grid_snapshot_triggered(self, doc) -> Sequence[ScanDataInfo]:
-        assert self.ispyb_ids.data_collection_ids, "No current data collection"
-        assert self.params, "ISPyB handler didn't recieve parameters!"
-        data = doc["data"]
-        data_collection_id = None
-        data_collection_info = DataCollectionInfo(
-            xtal_snapshot1=data.get("oav_grid_snapshot_last_path_full_overlay"),
-            xtal_snapshot2=data.get("oav_grid_snapshot_last_path_outer"),
-            xtal_snapshot3=data.get("oav_grid_snapshot_last_saved_path"),
-            n_images=(
-                data["oav_grid_snapshot_num_boxes_x"]
-                * data["oav_grid_snapshot_num_boxes_y"]
-            ),
-        )
-        microns_per_pixel_x = data["oav_grid_snapshot_microns_per_pixel_x"]
-        microns_per_pixel_y = data["oav_grid_snapshot_microns_per_pixel_y"]
-        data_collection_grid_info = DataCollectionGridInfo(
-            dx_in_mm=data["oav_grid_snapshot_box_width"] * microns_per_pixel_x / 1000,
-            dy_in_mm=data["oav_grid_snapshot_box_width"] * microns_per_pixel_y / 1000,
-            steps_x=data["oav_grid_snapshot_num_boxes_x"],
-            steps_y=data["oav_grid_snapshot_num_boxes_y"],
-            microns_per_pixel_x=microns_per_pixel_x,
-            microns_per_pixel_y=microns_per_pixel_y,
-            snapshot_offset_x_pixel=int(data["oav_grid_snapshot_top_left_x"]),
-            snapshot_offset_y_pixel=int(data["oav_grid_snapshot_top_left_y"]),
-            orientation=Orientation.HORIZONTAL,
-            snaked=True,
-        )
-        data_collection_info.comments = construct_comment_for_gridscan(
-            data_collection_grid_info
-        )
-        if len(self.ispyb_ids.data_collection_ids) > self._oav_snapshot_event_idx:
-            data_collection_id = self.ispyb_ids.data_collection_ids[
-                self._oav_snapshot_event_idx
-            ]
-
-        scan_data_info = ScanDataInfo(
-            data_collection_info=data_collection_info,
-            data_collection_id=data_collection_id,
-            data_collection_grid_info=data_collection_grid_info,
-        )
-        ISPYB_LOGGER.info("Updating ispyb data collection after oav snapshot.")
-        self._oav_snapshot_event_idx += 1
-        return [scan_data_info]
-
     def _handle_ispyb_transmission_flux_read(self, doc) -> Sequence[ScanDataInfo]:
         assert self.params
         hwscan_data_collection_info = DataCollectionInfo(
@@ -238,7 +166,7 @@ def activity_gated_stop(self, doc: RunStop) -> RunStop:
         uid to use this method!"""
         assert isinstance(
             self.ispyb, StoreInIspyb
-        ), "ISPyB handler recieved stop document, but deposition object doesn't exist!"
+        ), "ISPyB handler received stop document, but deposition object doesn't exist!"
         ISPYB_LOGGER.debug("ISPyB handler received stop document.")
         exit_status = (
             doc.get("exit_status") or "Exit status not available in stop document!"

src/hyperion/external_interaction/callbacks/rotation/ispyb_callback.py

@@ -60,7 +60,7 @@ def __init__(
     def activity_gated_start(self, doc: RunStart):
         if doc.get("subplan_name") == CONST.PLAN.ROTATION_OUTER:
             ISPYB_LOGGER.info(
-                "ISPyB callback recieved start document with experiment parameters."
+                "ISPyB callback received start document with experiment parameters."
             )
             self.params = RotationScan.from_json(doc.get("hyperion_parameters"))
             dcgid = (
@@ -144,8 +144,34 @@ def _handle_ispyb_hardware_read(self, doc: Event):
     def activity_gated_event(self, doc: Event):
         doc = super().activity_gated_event(doc)
         set_dcgid_tag(self.ispyb_ids.data_collection_group_id)
+
+        descriptor_name = self.descriptors[doc["descriptor"]].get("name")
+        if descriptor_name == CONST.DESCRIPTORS.OAV_ROTATION_SNAPSHOT_TRIGGERED:
+            scan_data_infos = self._handle_oav_rotation_snapshot_triggered(doc)
+            self.ispyb_ids = self.ispyb.update_deposition(
+                self.ispyb_ids, scan_data_infos
+            )
+
         return doc
 
+    def _handle_oav_rotation_snapshot_triggered(self, doc) -> Sequence[ScanDataInfo]:
+        assert self.ispyb_ids.data_collection_ids, "No current data collection"
+        assert self.params, "ISPyB handler didn't receive parameters!"
+        data = doc["data"]
+        self._oav_snapshot_event_idx += 1
+        data_collection_info = DataCollectionInfo(
+            **{
+                f"xtal_snapshot{self._oav_snapshot_event_idx}": data.get(
+                    "oav_snapshot_last_saved_path"
+                )
+            }
+        )
+        scan_data_info = ScanDataInfo(
+            data_collection_id=self.ispyb_ids.data_collection_ids[-1],
+            data_collection_info=data_collection_info,
+        )
+        return [scan_data_info]
+
     def activity_gated_stop(self, doc: RunStop) -> RunStop:
         if doc.get("run_start") == self.uid_to_finalize_on:
             self.uid_to_finalize_on = None

src/hyperion/external_interaction/callbacks/rotation/ispyb_mapping.py

@@ -11,6 +11,7 @@ def populate_data_collection_info_for_rotation(params: RotationScan):
         data_collection_number=params.detector_params.run_number,  # type:ignore # the validator always makes this int
         n_images=params.num_images,
         axis_range=params.rotation_increment_deg,
+        axis_start=params.omega_start_deg,
         axis_end=(params.omega_start_deg + params.scan_width_deg),
         kappa_start=params.kappa_start_deg,
     )

src/hyperion/external_interaction/callbacks/xray_centre/ispyb_callback.py

@@ -18,25 +18,27 @@
 )
 from hyperion.external_interaction.callbacks.logging_callback import format_doc_for_log
 from hyperion.external_interaction.callbacks.xray_centre.ispyb_mapping import (
+    construct_comment_for_gridscan,
     populate_xy_data_collection_info,
     populate_xz_data_collection_info,
 )
 from hyperion.external_interaction.exceptions import ISPyBDepositionNotMade
 from hyperion.external_interaction.ispyb.data_model import (
+    DataCollectionGridInfo,
     DataCollectionInfo,
     DataCollectionPositionInfo,
     ScanDataInfo,
 )
+from hyperion.external_interaction.ispyb.ispyb_dataclass import Orientation
 from hyperion.external_interaction.ispyb.ispyb_store import (
     IspybIds,
     StoreInIspyb,
 )
 from hyperion.log import ISPYB_LOGGER, set_dcgid_tag
+from hyperion.parameters.components import DiffractionExperimentWithSample
 from hyperion.parameters.constants import CONST
 from hyperion.parameters.gridscan import (
     GridCommon,
-    GridScanWithEdgeDetect,
-    ThreeDGridScan,
 )
 
 if TYPE_CHECKING:
@@ -86,7 +88,7 @@ def activity_gated_start(self, doc: RunStart):
         if doc.get("subplan_name") == CONST.PLAN.GRID_DETECT_AND_DO_GRIDSCAN:
             self.uid_to_finalize_on = doc.get("uid")
             ISPYB_LOGGER.info(
-                "ISPyB callback recieved start document with experiment parameters and "
+                "ISPyB callback received start document with experiment parameters and "
                 f"uid: {self.uid_to_finalize_on}"
             )
             self.params = GridCommon.from_json(
@@ -125,49 +127,112 @@ def activity_gated_start(self, doc: RunStart):
     def activity_gated_event(self, doc: Event):
         doc = super().activity_gated_event(doc)
 
-        event_descriptor = self.descriptors[doc["descriptor"]]
-        if event_descriptor.get("name") == ZOCALO_READING_PLAN_NAME:
-            crystal_summary = ""
-            if self._processing_start_time is not None:
-                proc_time = time() - self._processing_start_time
-                crystal_summary = f"Zocalo processing took {proc_time:.2f} s. "
-
-            bboxes: List[np.ndarray] = []
-            ISPYB_LOGGER.info(
-                f"Amending comment based on Zocalo reading doc: {format_doc_for_log(doc)}"
-            )
-            raw_results = doc["data"]["zocalo-results"]
-            if len(raw_results) > 0:
-                for n, res in enumerate(raw_results):
-                    bb = res["bounding_box"]
-                    diff = np.array(bb[1]) - np.array(bb[0])
-                    bboxes.append(diff)
-
-                    nicely_formatted_com = [
-                        f"{np.round(com, 2)}" for com in res["centre_of_mass"]
-                    ]
-                    crystal_summary += (
-                        f"Crystal {n + 1}: "
-                        f"Strength {res['total_count']}; "
-                        f"Position (grid boxes) {nicely_formatted_com}; "
-                        f"Size (grid boxes) {bboxes[n]}; "
-                    )
-            else:
-                crystal_summary += "Zocalo found no crystals in this gridscan."
-            assert (
-                self.ispyb_ids.data_collection_ids
-            ), "No data collection to add results to"
-            self.ispyb.append_to_comment(
-                self.ispyb_ids.data_collection_ids[0], crystal_summary
+        descriptor_name = self.descriptors[doc["descriptor"]].get("name")
+        if descriptor_name == ZOCALO_READING_PLAN_NAME:
+            self._handle_zocalo_read_event(doc)
+        elif descriptor_name == CONST.DESCRIPTORS.OAV_GRID_SNAPSHOT_TRIGGERED:
+            scan_data_infos = self._handle_oav_grid_snapshot_triggered(doc)
+            self.ispyb_ids = self.ispyb.update_deposition(
+                self.ispyb_ids, scan_data_infos
             )
 
         return doc
 
+    def _handle_zocalo_read_event(self, doc):
+        crystal_summary = ""
+        if self._processing_start_time is not None:
+            proc_time = time() - self._processing_start_time
+            crystal_summary = f"Zocalo processing took {proc_time:.2f} s. "
+        bboxes: List[np.ndarray] = []
+        ISPYB_LOGGER.info(
+            f"Amending comment based on Zocalo reading doc: {format_doc_for_log(doc)}"
+        )
+        raw_results = doc["data"]["zocalo-results"]
+        if len(raw_results) > 0:
+            for n, res in enumerate(raw_results):
+                bb = res["bounding_box"]
+                diff = np.array(bb[1]) - np.array(bb[0])
+                bboxes.append(diff)
+
+                nicely_formatted_com = [
+                    f"{np.round(com, 2)}" for com in res["centre_of_mass"]
+                ]
+                crystal_summary += (
+                    f"Crystal {n + 1}: "
+                    f"Strength {res['total_count']}; "
+                    f"Position (grid boxes) {nicely_formatted_com}; "
+                    f"Size (grid boxes) {bboxes[n]}; "
+                )
+        else:
+            crystal_summary += "Zocalo found no crystals in this gridscan."
+        assert (
+            self.ispyb_ids.data_collection_ids
+        ), "No data collection to add results to"
+        self.ispyb.append_to_comment(
+            self.ispyb_ids.data_collection_ids[0], crystal_summary
+        )
+
+    def _handle_oav_grid_snapshot_triggered(self, doc) -> Sequence[ScanDataInfo]:
+        assert self.ispyb_ids.data_collection_ids, "No current data collection"
+        assert self.params, "ISPyB handler didn't receive parameters!"
+        data = doc["data"]
+        data_collection_id = None
+        data_collection_info = DataCollectionInfo(
+            xtal_snapshot1=data.get("oav_grid_snapshot_last_path_full_overlay"),
+            xtal_snapshot2=data.get("oav_grid_snapshot_last_path_outer"),
+            xtal_snapshot3=data.get("oav_grid_snapshot_last_saved_path"),
+            n_images=(
+                data["oav_grid_snapshot_num_boxes_x"]
+                * data["oav_grid_snapshot_num_boxes_y"]
+            ),
+        )
+        microns_per_pixel_x = data["oav_grid_snapshot_microns_per_pixel_x"]
+        microns_per_pixel_y = data["oav_grid_snapshot_microns_per_pixel_y"]
+        data_collection_grid_info = DataCollectionGridInfo(
+            dx_in_mm=data["oav_grid_snapshot_box_width"] * microns_per_pixel_x / 1000,
+            dy_in_mm=data["oav_grid_snapshot_box_width"] * microns_per_pixel_y / 1000,
+            steps_x=data["oav_grid_snapshot_num_boxes_x"],
+            steps_y=data["oav_grid_snapshot_num_boxes_y"],
+            microns_per_pixel_x=microns_per_pixel_x,
+            microns_per_pixel_y=microns_per_pixel_y,
+            snapshot_offset_x_pixel=int(data["oav_grid_snapshot_top_left_x"]),
+            snapshot_offset_y_pixel=int(data["oav_grid_snapshot_top_left_y"]),
+            orientation=Orientation.HORIZONTAL,
+            snaked=True,
+        )
+        data_collection_info.comments = construct_comment_for_gridscan(
+            data_collection_grid_info
+        )
+        if len(self.ispyb_ids.data_collection_ids) > self._oav_snapshot_event_idx:
+            data_collection_id = self.ispyb_ids.data_collection_ids[
+                self._oav_snapshot_event_idx
+            ]
+        self._populate_axis_info(data_collection_info, doc["data"]["smargon_omega"])
+
+        scan_data_info = ScanDataInfo(
+            data_collection_info=data_collection_info,
+            data_collection_id=data_collection_id,
+            data_collection_grid_info=data_collection_grid_info,
+        )
+        ISPYB_LOGGER.info("Updating ispyb data collection after oav snapshot.")
+        self._oav_snapshot_event_idx += 1
+        return [scan_data_info]
+
+    def _populate_axis_info(
+        self, data_collection_info: DataCollectionInfo, omega_start: float | None
+    ):
+        if omega_start is not None:
+            omega_in_gda_space = -omega_start
+            data_collection_info.omega_start = omega_in_gda_space
+            data_collection_info.axis_start = omega_in_gda_space
+            data_collection_info.axis_end = omega_in_gda_space
+            data_collection_info.axis_range = 0
+
     def populate_info_for_update(
         self,
         event_sourced_data_collection_info: DataCollectionInfo,
         event_sourced_position_info: Optional[DataCollectionPositionInfo],
-        params: ThreeDGridScan | GridScanWithEdgeDetect,
+        params: DiffractionExperimentWithSample,
     ) -> Sequence[ScanDataInfo]:
         assert (
             self.ispyb_ids.data_collection_ids

src/hyperion/external_interaction/callbacks/xray_centre/ispyb_mapping.py

@@ -15,23 +15,16 @@ def populate_xz_data_collection_info(detector_params: DetectorParams):
         detector_params.omega_start is not None
         and detector_params.run_number is not None
     ), "StoreGridscanInIspyb failed to get parameters"
-    omega_start = detector_params.omega_start + 90
     run_number = detector_params.run_number + 1
     info = DataCollectionInfo(
-        omega_start=omega_start,
         data_collection_number=run_number,
-        axis_range=0,
-        axis_end=omega_start,
     )
     return info
 
 
 def populate_xy_data_collection_info(detector_params: DetectorParams):
     return DataCollectionInfo(
-        omega_start=detector_params.omega_start,
         data_collection_number=detector_params.run_number,
-        axis_range=0,
-        axis_end=detector_params.omega_start,
     )
 
 

tests/unit_tests/experiment_plans/test_rotation_scan_plan.py

@@ -8,7 +8,6 @@
 import pytest
 from bluesky.run_engine import RunEngine
 from dodal.devices.aperturescatterguard import ApertureScatterguard
-from dodal.devices.smargon import Smargon
 from dodal.devices.synchrotron import SynchrotronMode
 from dodal.devices.zebra import Zebra
 from ophyd.status import Status