From fa7239dcd1f5094cfc55f3d2348d91ee93a79d00 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Fri, 22 Mar 2024 10:13:25 +0000 Subject: [PATCH] (#1217) * Remove upsert_dc_grid calls from hardware read ispyb updates * Add units to GridScanInfo fields * Move grid comment updates from hardware read to oav snapshot event --- .../grid_detect_then_xray_centre_plan.py | 1 + .../wait_for_robot_load_then_centre_plan.py | 2 + .../callbacks/common/ispyb_mapping.py | 8 +- .../callbacks/ispyb_callback_base.py | 91 +++- .../callbacks/oav_snapshot_callback.py | 1 + .../callbacks/rotation/ispyb_callback.py | 4 + .../callbacks/xray_centre/ispyb_callback.py | 78 +-- .../callbacks/xray_centre/ispyb_mapping.py | 35 +- .../external_interaction/ispyb/ispyb_store.py | 64 ++- .../test_ispyb_dev_connection.py | 7 +- .../test_grid_detection_plan.py | 10 + .../callbacks/conftest.py | 22 +- .../callbacks/rotation/test_ispyb_callback.py | 2 +- .../xray_centre/test_ispyb_callback.py | 456 +++++++++--------- .../xray_centre/test_ispyb_mapping.py | 37 +- .../external_interaction/ispyb/conftest.py | 7 +- .../ispyb/test_gridscan_ispyb_store_2d.py | 2 +- .../ispyb/test_gridscan_ispyb_store_3d.py | 3 +- 18 files changed, 441 insertions(+), 389 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 e2eb2bafb..34143e0bc 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 @@ -199,6 +199,7 @@ def run_grid_detection_plan( ) # Hack because the callback returns the list in inverted order + # TODO 1217 REMOVE THIS parameters.hyperion_params.ispyb_params.xtal_snapshots_omega_start = ( oav_callback.snapshot_filenames[0][::-1] ) diff --git a/src/hyperion/experiment_plans/wait_for_robot_load_then_centre_plan.py b/src/hyperion/experiment_plans/wait_for_robot_load_then_centre_plan.py index fbba74559..eb5dc99a2 100644 --- a/src/hyperion/experiment_plans/wait_for_robot_load_then_centre_plan.py +++ b/src/hyperion/experiment_plans/wait_for_robot_load_then_centre_plan.py @@ -121,6 +121,8 @@ def wait_for_robot_load_then_centre_plan( yield from wait_for_smargon_not_disabled(composite.smargon) + # XXX 1278 this effectively casts between unrelated types which doesn't have all + # attributes needed for downstream e.g. grid_width_microns params_json = json.loads(parameters.json()) pin_centre_params = PinCentreThenXrayCentreInternalParameters(**params_json) diff --git a/src/hyperion/external_interaction/callbacks/common/ispyb_mapping.py b/src/hyperion/external_interaction/callbacks/common/ispyb_mapping.py index 5a0614dd6..f8186aec4 100644 --- a/src/hyperion/external_interaction/callbacks/common/ispyb_mapping.py +++ b/src/hyperion/external_interaction/callbacks/common/ispyb_mapping.py @@ -121,8 +121,8 @@ def get_xtal_snapshots(ispyb_params): @dataclass class GridScanInfo: - upper_left: Union[list[int], ndarray] # TODO REMOVE THIS - # upper_left_x: float - # upper_left_y: float + upper_left_px: Union[list[int], ndarray] + x_steps: int y_steps: int - y_step_size: float + x_step_size_mm: float + y_step_size_mm: float diff --git a/src/hyperion/external_interaction/callbacks/ispyb_callback_base.py b/src/hyperion/external_interaction/callbacks/ispyb_callback_base.py index 4ac319b17..37dd45099 100644 --- a/src/hyperion/external_interaction/callbacks/ispyb_callback_base.py +++ b/src/hyperion/external_interaction/callbacks/ispyb_callback_base.py @@ -4,16 +4,24 @@ from collections.abc import Sequence from typing import TYPE_CHECKING, Any, Callable, Dict, Optional, TypeVar +import numpy + from hyperion.external_interaction.callbacks.common.ispyb_mapping import ( + GridScanInfo, populate_data_collection_group, ) 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, ScanDataInfo, ) +from hyperion.external_interaction.ispyb.ispyb_dataclass import Orientation from hyperion.external_interaction.ispyb.ispyb_store import ( IspybIds, StoreInIspyb, @@ -46,7 +54,8 @@ def __init__( for self.ispyb_ids.""" ISPYB_LOGGER.debug("Initialising ISPyB callback") super().__init__(log=ISPYB_LOGGER, emit=emit) - self._event_driven_data_collection_info: Optional[DataCollectionInfo] = None + self._oav_snapshot_event_idx: Optional[int] = None + self._hwscan_data_collection_info: Optional[DataCollectionInfo] = None self._sample_barcode: Optional[str] = None self.params: GridscanInternalParameters | RotationInternalParameters | None = ( None @@ -68,7 +77,8 @@ def __init__( self.log = ISPYB_LOGGER def activity_gated_start(self, doc: RunStart): - self._event_driven_data_collection_info = DataCollectionInfo() + self._oav_snapshot_event_idx = 0 + self._hwscan_data_collection_info = DataCollectionInfo() self._sample_barcode = None return self._tag_doc(doc) @@ -99,7 +109,7 @@ def activity_gated_event(self, doc: Event) -> Event: self._handle_ispyb_transmission_flux_read(doc) scan_data_infos = self.populate_info_for_update( - self._event_driven_data_collection_info, self.params + self._hwscan_data_collection_info, self.params ) ISPYB_LOGGER.info("Updating ispyb entry.") self.ispyb_ids = self.update_deposition( @@ -111,44 +121,99 @@ def activity_gated_event(self, doc: Event) -> Event: return self._tag_doc(doc) def _handle_ispyb_hardware_read(self, doc): - assert self._event_driven_data_collection_info + assert self._hwscan_data_collection_info ISPYB_LOGGER.info("ISPyB handler received event from read hardware") - self._event_driven_data_collection_info.undulator_gap1 = doc["data"][ + self._hwscan_data_collection_info.undulator_gap1 = doc["data"][ "undulator_current_gap" ] - self._event_driven_data_collection_info.synchrotron_mode = doc["data"][ + self._hwscan_data_collection_info.synchrotron_mode = doc["data"][ "synchrotron_machine_status_synchrotron_mode" ] - self._event_driven_data_collection_info.slitgap_horizontal = doc["data"][ + self._hwscan_data_collection_info.slitgap_horizontal = doc["data"][ "s4_slit_gaps_xgap" ] - self._event_driven_data_collection_info.slitgap_vertical = doc["data"][ + self._hwscan_data_collection_info.slitgap_vertical = doc["data"][ "s4_slit_gaps_ygap" ] self._sample_barcode = doc["data"]["robot-barcode"] def _handle_oav_snapshot_triggered(self, doc): + assert self.ispyb_ids.data_collection_ids, "No current data collection" + data = doc["data"] + data_collection_id = None + data_collection_info = DataCollectionInfo() + data_collection_info.n_images = ( + data["oav_snapshot_num_boxes_x"] * data["oav_snapshot_num_boxes_y"] + ) + grid_scan_info = GridScanInfo( + upper_left_px=numpy.array( + [data["oav_snapshot_top_left_x"], data["oav_snapshot_top_left_y"]] + ), + x_steps=data["oav_snapshot_num_boxes_x"], + y_steps=data["oav_snapshot_num_boxes_y"], + # convert pixels back to mm + x_step_size_mm=data["oav_snapshot_box_width"] + * self.params.hyperion_params.ispyb_params.microns_per_pixel_x + / 1000, + y_step_size_mm=data["oav_snapshot_box_width"] + * self.params.hyperion_params.ispyb_params.microns_per_pixel_y + / 1000, + ) + data_collection_info.comments = construct_comment_for_gridscan( + self.params.hyperion_params.ispyb_params, grid_scan_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 + ] + + # TODO 1217 populate xtal_snapshot1/2/3 + data_collection_grid_info = DataCollectionGridInfo( + dx_in_mm=data["oav_snapshot_box_width"] + * self.params.hyperion_params.ispyb_params.microns_per_pixel_x + / 1000, + dy_in_mm=data["oav_snapshot_box_width"] + * self.params.hyperion_params.ispyb_params.microns_per_pixel_y + / 1000, + steps_x=data["oav_snapshot_num_boxes_x"], + steps_y=data["oav_snapshot_num_boxes_y"], + microns_per_pixel_x=self.params.hyperion_params.ispyb_params.microns_per_pixel_x, + microns_per_pixel_y=self.params.hyperion_params.ispyb_params.microns_per_pixel_y, + snapshot_offset_x_pixel=data["oav_snapshot_top_left_x"], + snapshot_offset_y_pixel=data["oav_snapshot_top_left_y"], + orientation=Orientation.HORIZONTAL, + snaked=True, + ) + scan_data_info = ScanDataInfo( + data_collection_info=data_collection_info, + data_collection_id=data_collection_id, + data_collection_grid_info=data_collection_grid_info, + ) + self.ispyb_ids = self.ispyb.update_deposition( + self.ispyb_ids, None, [scan_data_info] + ) + self._oav_snapshot_event_idx += 1 pass def _handle_ispyb_transmission_flux_read(self, doc): - assert self._event_driven_data_collection_info + assert self._hwscan_data_collection_info if doc["data"]["attenuator_actual_transmission"]: # Ispyb wants the transmission in a percentage, we use fractions - self._event_driven_data_collection_info.transmission = ( + self._hwscan_data_collection_info.transmission = ( doc["data"]["attenuator_actual_transmission"] * 100 ) # TODO 1173 Remove this once nexus_utils no longer needs it self.params.hyperion_params.ispyb_params.transmission_fraction = doc[ "data" ]["attenuator_actual_transmission"] - self._event_driven_data_collection_info.flux = doc["data"]["flux_flux_reading"] + self._hwscan_data_collection_info.flux = doc["data"]["flux_flux_reading"] # TODO 1173 Remove this once nexus_utils no longer needs it self.params.hyperion_params.ispyb_params.flux = ( - self._event_driven_data_collection_info.flux + self._hwscan_data_collection_info.flux ) if doc["data"]["dcm_energy_in_kev"]: energy_ev = doc["data"]["dcm_energy_in_kev"] * 1000 - self._event_driven_data_collection_info.wavelength = convert_eV_to_angstrom( + self._hwscan_data_collection_info.wavelength = convert_eV_to_angstrom( energy_ev ) # TODO 1173 Remove this once nexus_utils no longer needs wavelength_angstroms diff --git a/src/hyperion/external_interaction/callbacks/oav_snapshot_callback.py b/src/hyperion/external_interaction/callbacks/oav_snapshot_callback.py index 1786d17a3..5613e4fb2 100644 --- a/src/hyperion/external_interaction/callbacks/oav_snapshot_callback.py +++ b/src/hyperion/external_interaction/callbacks/oav_snapshot_callback.py @@ -18,6 +18,7 @@ def event(self, doc): ] ) + # TODO 1217 REMOVE THIS self.out_upper_left.append( [data.get("oav_snapshot_top_left_x"), data.get("oav_snapshot_top_left_y")] ) diff --git a/src/hyperion/external_interaction/callbacks/rotation/ispyb_callback.py b/src/hyperion/external_interaction/callbacks/rotation/ispyb_callback.py index 526a1c6db..7c45ddc11 100644 --- a/src/hyperion/external_interaction/callbacks/rotation/ispyb_callback.py +++ b/src/hyperion/external_interaction/callbacks/rotation/ispyb_callback.py @@ -131,6 +131,9 @@ def activity_gated_start(self, doc: RunStart): def populate_info_for_update( self, event_sourced_data_collection_info: DataCollectionInfo, params ) -> Sequence[ScanDataInfo]: + assert ( + self.ispyb_ids.data_collection_ids + ), "Expect an existing DataCollection to update" params = cast(RotationInternalParameters, params) initial_collection_info = populate_data_collection_info_for_rotation( params.hyperion_params.ispyb_params, @@ -157,6 +160,7 @@ def populate_info_for_update( data_collection_position_info=populate_data_collection_position_info( params.hyperion_params.ispyb_params ), + data_collection_id=self.ispyb_ids.data_collection_ids[0], ) ] diff --git a/src/hyperion/external_interaction/callbacks/xray_centre/ispyb_callback.py b/src/hyperion/external_interaction/callbacks/xray_centre/ispyb_callback.py index 08c74da65..57ca1dbcc 100644 --- a/src/hyperion/external_interaction/callbacks/xray_centre/ispyb_callback.py +++ b/src/hyperion/external_interaction/callbacks/xray_centre/ispyb_callback.py @@ -11,7 +11,6 @@ from dodal.devices.zocalo.zocalo_results import ZOCALO_READING_PLAN_NAME from hyperion.external_interaction.callbacks.common.ispyb_mapping import ( - GridScanInfo, populate_data_collection_group, populate_data_collection_position_info, populate_remaining_data_collection_info, @@ -20,8 +19,6 @@ BaseISPyBCallback, ) from hyperion.external_interaction.callbacks.xray_centre.ispyb_mapping import ( - construct_comment_for_gridscan, - populate_data_collection_grid_info, populate_xy_data_collection_info, populate_xz_data_collection_info, ) @@ -116,26 +113,12 @@ def activity_gated_start(self, doc: RunStart): self.params.hyperion_params.detector_params, self.params.hyperion_params.ispyb_params, ) - grid_scan_info = GridScanInfo( - self.params.hyperion_params.ispyb_params.upper_left, - self.params.experiment_params.y_steps, - self.params.experiment_params.y_step_size, - ) - - def constructor(): - return construct_comment_for_gridscan( - self.params, - self.params.hyperion_params.ispyb_params, - grid_scan_info, - ) scan_data_info = ScanDataInfo( data_collection_info=populate_remaining_data_collection_info( - constructor, + lambda: None, None, populate_xy_data_collection_info( - grid_scan_info, - self.params, self.params.hyperion_params.ispyb_params, self.params.hyperion_params.detector_params, ), @@ -192,40 +175,14 @@ def populate_info_for_update( self, event_sourced_data_collection_info: DataCollectionInfo, params ) -> Sequence[ScanDataInfo]: params = cast(GridscanInternalParameters, params) - grid_scan_info = GridScanInfo( - [ - int(params.hyperion_params.ispyb_params.upper_left[0]), - int(params.hyperion_params.ispyb_params.upper_left[1]), - ], - params.experiment_params.y_steps, - params.experiment_params.y_step_size, - ) xy_scan_data_info = self.populate_xy_scan_data_info( - params, event_sourced_data_collection_info, grid_scan_info - ) - xy_scan_data_info.data_collection_grid_info = ( - populate_data_collection_grid_info( - params, grid_scan_info, params.hyperion_params.ispyb_params - ) + params, event_sourced_data_collection_info ) scan_data_infos = [xy_scan_data_info] if self.is_3d_gridscan(): - xz_grid_scan_info = GridScanInfo( - [ - int(params.hyperion_params.ispyb_params.upper_left[0]), - int(params.hyperion_params.ispyb_params.upper_left[2]), - ], - params.experiment_params.z_steps, - params.experiment_params.z_step_size, - ) xz_scan_data_info = self.populate_xz_scan_data_info( - params, event_sourced_data_collection_info, xz_grid_scan_info - ) - xz_scan_data_info.data_collection_grid_info = ( - populate_data_collection_grid_info( - params, xz_grid_scan_info, params.hyperion_params.ispyb_params - ) + params, event_sourced_data_collection_info ) scan_data_infos.append(xz_scan_data_info) return scan_data_infos @@ -234,11 +191,11 @@ def populate_xy_scan_data_info( self, params, event_sourced_data_collection_info: DataCollectionInfo, - grid_scan_info: GridScanInfo, ): + assert ( + self.ispyb_ids.data_collection_ids + ), "Expect at least one valid data collection to record scan data" xy_data_collection_info = populate_xy_data_collection_info( - grid_scan_info, - params, params.hyperion_params.ispyb_params, params.hyperion_params.detector_params, ) @@ -252,13 +209,8 @@ def populate_xy_scan_data_info( }, ) - def comment_constructor(): - return construct_comment_for_gridscan( - params, params.hyperion_params.ispyb_params, grid_scan_info - ) - xy_data_collection_info = populate_remaining_data_collection_info( - comment_constructor, + lambda: None, self.ispyb_ids.data_collection_group_id, xy_data_collection_info, params.hyperion_params.detector_params, @@ -270,16 +222,15 @@ def comment_constructor(): data_collection_position_info=populate_data_collection_position_info( params.hyperion_params.ispyb_params ), + data_collection_id=self.ispyb_ids.data_collection_ids[0], ) def populate_xz_scan_data_info( self, params, event_sourced_data_collection_info: DataCollectionInfo, - xz_grid_scan_info: GridScanInfo, ): xz_data_collection_info = populate_xz_data_collection_info( - xz_grid_scan_info, params, params.hyperion_params.ispyb_params, params.hyperion_params.detector_params, @@ -293,23 +244,24 @@ def populate_xz_scan_data_info( }, ) - def xz_comment_constructor(): - return construct_comment_for_gridscan( - params, params.hyperion_params.ispyb_params, xz_grid_scan_info - ) - xz_data_collection_info = populate_remaining_data_collection_info( - xz_comment_constructor, + lambda: None, self.ispyb_ids.data_collection_group_id, xz_data_collection_info, params.hyperion_params.detector_params, params.hyperion_params.ispyb_params, ) + data_collection_id = ( + self.ispyb_ids.data_collection_ids[1] + if len(self.ispyb_ids.data_collection_ids) > 1 + else None + ) return ScanDataInfo( data_collection_info=xz_data_collection_info, data_collection_position_info=populate_data_collection_position_info( params.hyperion_params.ispyb_params ), + data_collection_id=data_collection_id, ) def activity_gated_stop(self, doc: RunStop) -> Optional[RunStop]: diff --git a/src/hyperion/external_interaction/callbacks/xray_centre/ispyb_mapping.py b/src/hyperion/external_interaction/callbacks/xray_centre/ispyb_mapping.py index 89157fad3..4a0ef4046 100644 --- a/src/hyperion/external_interaction/callbacks/xray_centre/ispyb_mapping.py +++ b/src/hyperion/external_interaction/callbacks/xray_centre/ispyb_mapping.py @@ -11,7 +11,6 @@ def populate_xz_data_collection_info( - grid_scan_info: GridScanInfo, full_params, ispyb_params, detector_params, @@ -28,7 +27,6 @@ def populate_xz_data_collection_info( info = DataCollectionInfo( omega_start=omega_start, data_collection_number=run_number, - n_images=full_params.experiment_params.x_steps * grid_scan_info.y_steps, axis_range=0, axis_end=omega_start, ) @@ -38,13 +36,10 @@ def populate_xz_data_collection_info( return info -def populate_xy_data_collection_info( - grid_scan_info: GridScanInfo, full_params, ispyb_params, detector_params -): +def populate_xy_data_collection_info(ispyb_params, detector_params): info = DataCollectionInfo( omega_start=detector_params.omega_start, data_collection_number=detector_params.run_number, - n_images=full_params.experiment_params.x_steps * grid_scan_info.y_steps, axis_range=0, axis_end=detector_params.omega_start, ) @@ -55,29 +50,27 @@ def populate_xy_data_collection_info( return info -def construct_comment_for_gridscan(full_params, ispyb_params, grid_scan_info) -> str: +def construct_comment_for_gridscan(ispyb_params, grid_scan_info: GridScanInfo) -> str: assert ( - ispyb_params is not None - and full_params is not None - and grid_scan_info is not None + ispyb_params is not None and grid_scan_info is not None ), "StoreGridScanInIspyb failed to get parameters" bottom_right = oav_utils.bottom_right_from_top_left( - grid_scan_info.upper_left, # type: ignore - full_params.experiment_params.x_steps, + grid_scan_info.upper_left_px, # type: ignore + grid_scan_info.x_steps, grid_scan_info.y_steps, - full_params.experiment_params.x_step_size, - grid_scan_info.y_step_size, + grid_scan_info.x_step_size_mm, + grid_scan_info.y_step_size_mm, ispyb_params.microns_per_pixel_x, ispyb_params.microns_per_pixel_y, ) return ( "Hyperion: Xray centring - Diffraction grid scan of " - f"{full_params.experiment_params.x_steps} by " + f"{grid_scan_info.x_steps} by " f"{grid_scan_info.y_steps} images in " - f"{(full_params.experiment_params.x_step_size * 1e3):.1f} um by " - f"{(grid_scan_info.y_step_size * 1e3):.1f} um steps. " - f"Top left (px): [{int(grid_scan_info.upper_left[0])},{int(grid_scan_info.upper_left[1])}], " + f"{(grid_scan_info.x_step_size_mm * 1e3):.1f} um by " + f"{(grid_scan_info.y_step_size_mm * 1e3):.1f} um steps. " + f"Top left (px): [{int(grid_scan_info.upper_left_px[0])},{int(grid_scan_info.upper_left_px[1])}], " f"bottom right (px): [{bottom_right[0]},{bottom_right[1]}]." ) @@ -87,13 +80,13 @@ def populate_data_collection_grid_info(full_params, grid_scan_info, ispyb_params assert full_params is not None dc_grid_info = DataCollectionGridInfo( dx_in_mm=full_params.experiment_params.x_step_size, - dy_in_mm=grid_scan_info.y_step_size, + dy_in_mm=grid_scan_info.y_step_size_mm, steps_x=full_params.experiment_params.x_steps, steps_y=grid_scan_info.y_steps, microns_per_pixel_x=ispyb_params.microns_per_pixel_x, # cast coordinates from numpy int64 to avoid mysql type conversion issues - snapshot_offset_x_pixel=int(grid_scan_info.upper_left[0]), - snapshot_offset_y_pixel=int(grid_scan_info.upper_left[1]), + snapshot_offset_x_pixel=int(grid_scan_info.upper_left_px[0]), + snapshot_offset_y_pixel=int(grid_scan_info.upper_left_px[1]), microns_per_pixel_y=ispyb_params.microns_per_pixel_y, orientation=Orientation.HORIZONTAL, snaked=True, diff --git a/src/hyperion/external_interaction/ispyb/ispyb_store.py b/src/hyperion/external_interaction/ispyb/ispyb_store.py index 64dfca6b0..a9342bf41 100755 --- a/src/hyperion/external_interaction/ispyb/ispyb_store.py +++ b/src/hyperion/external_interaction/ispyb/ispyb_store.py @@ -2,7 +2,6 @@ from abc import ABC from dataclasses import asdict -from itertools import zip_longest from typing import TYPE_CHECKING, Optional, Sequence, Tuple import ispyb @@ -13,6 +12,7 @@ from pydantic import BaseModel from hyperion.external_interaction.ispyb.data_model import ( + DataCollectionGridInfo, DataCollectionGroupInfo, DataCollectionInfo, ExperimentType, @@ -66,36 +66,46 @@ def begin_deposition( def update_deposition( self, ispyb_ids, - data_collection_group_info: DataCollectionGroupInfo, + data_collection_group_info: Optional[DataCollectionGroupInfo], scan_data_infos: Sequence[ScanDataInfo], - ): + ) -> IspybIds: assert ( ispyb_ids.data_collection_group_id ), "Attempted to store scan data without a collection group" assert ( ispyb_ids.data_collection_ids ), "Attempted to store scan data without a collection" + print(f"OBJECT PASSED IN IS {ispyb_ids}") return self._begin_or_update_deposition( ispyb_ids, data_collection_group_info, scan_data_infos ) def _begin_or_update_deposition( - self, ispyb_ids, data_collection_group_info, scan_data_infos - ): + self, + ispyb_ids, + data_collection_group_info: Optional[DataCollectionGroupInfo], + scan_data_infos, + ) -> IspybIds: + print(f"OBJECT RECEIVED IS {ispyb_ids}") with ispyb.open(self.ISPYB_CONFIG_PATH) as conn: assert conn is not None, "Failed to connect to ISPyB" - - ispyb_ids.data_collection_group_id = ( - self._store_data_collection_group_table( - conn, data_collection_group_info, ispyb_ids.data_collection_group_id + if data_collection_group_info: + ispyb_ids.data_collection_group_id = ( + self._store_data_collection_group_table( + conn, + data_collection_group_info, + ispyb_ids.data_collection_group_id, + ) ) - ) + else: + assert ( + ispyb_ids.data_collection_group_id + ), "Attempt to update data collection without a data collection group ID" grid_ids = [] - data_collection_ids_out = [] - for scan_data_info, data_collection_id in zip_longest( - scan_data_infos, ispyb_ids.data_collection_ids - ): + data_collection_ids_out = list(ispyb_ids.data_collection_ids) + for scan_data_info in scan_data_infos: + data_collection_id = scan_data_info.data_collection_id if ( scan_data_info.data_collection_info and not scan_data_info.data_collection_info.parent_id @@ -104,10 +114,14 @@ def _begin_or_update_deposition( ispyb_ids.data_collection_group_id ) - data_collection_id, grid_id = self._store_single_scan_data( + new_data_collection_id, grid_id = self._store_single_scan_data( conn, scan_data_info, data_collection_id ) - data_collection_ids_out.append(data_collection_id) + print( + f"OLD_DC_ID = {data_collection_id} NEW DCID = {new_data_collection_id}" + ) + if not data_collection_id: + data_collection_ids_out.append(new_data_collection_id) if grid_id: grid_ids.append(grid_id) ispyb_ids = IspybIds( @@ -115,6 +129,7 @@ def _begin_or_update_deposition( grid_ids=tuple(grid_ids), data_collection_group_id=ispyb_ids.data_collection_group_id, ) + print(f"NEW ISPYBIDS={ispyb_ids}") return ispyb_ids def end_deposition(self, ispyb_ids: IspybIds, success: str, reason: str): @@ -214,9 +229,11 @@ def _store_data_collection_table( def _store_single_scan_data( self, conn, scan_data_info, data_collection_id=None ) -> Tuple[int, Optional[int]]: + print(f"DCID IN IS {data_collection_id}") data_collection_id = self._store_data_collection_table( conn, data_collection_id, scan_data_info.data_collection_info ) + print(f"DCID OUT IS {data_collection_id}") if scan_data_info.data_collection_position_info: self._store_position_table( @@ -232,10 +249,14 @@ def _store_single_scan_data( data_collection_id, scan_data_info.data_collection_grid_info, ) + print(f"DCID OUT2 IS {data_collection_id}") return data_collection_id, grid_id def _store_grid_info_table( - self, conn: Connector, ispyb_data_collection_id: int, dc_grid_info + self, + conn: Connector, + ispyb_data_collection_id: int, + dc_grid_info: DataCollectionGridInfo, ) -> int: mx_acquisition: MXAcquisition = conn.mx_acquisition params = mx_acquisition.get_dc_grid_params() @@ -251,10 +272,11 @@ def _fill_common_data_collection_params( if data_collection_id: params["id"] = data_collection_id - assert data_collection_info.visit_string - params["visit_id"] = get_session_id_from_visit( - conn, data_collection_info.visit_string - ) + if data_collection_info.visit_string: + # This is only needed for populating the DataCollectionGroup + params["visit_id"] = get_session_id_from_visit( + conn, data_collection_info.visit_string + ) params |= { k: v for k, v in asdict(data_collection_info).items() if k != "visit_string" } 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 76904e074..0293ac660 100644 --- a/tests/system_tests/external_interaction/test_ispyb_dev_connection.py +++ b/tests/system_tests/external_interaction/test_ispyb_dev_connection.py @@ -66,14 +66,12 @@ def dummy_scan_data_info_for_begin(dummy_params): dummy_params.experiment_params.y_step_size, ) info = populate_xy_data_collection_info( - grid_scan_info, - dummy_params, dummy_params.hyperion_params.ispyb_params, dummy_params.hyperion_params.detector_params, ) info = populate_remaining_data_collection_info( lambda: construct_comment_for_gridscan( - dummy_params, dummy_params.hyperion_params.ispyb_params, grid_scan_info + dummy_params.hyperion_params.ispyb_params, grid_scan_info ), None, info, @@ -118,7 +116,6 @@ def scan_data_infos_for_update_3d( dummy_params.experiment_params.z_step_size, ) xz_data_collection_info = populate_xz_data_collection_info( - xz_grid_scan_info, dummy_params, dummy_params.hyperion_params.ispyb_params, dummy_params.hyperion_params.detector_params, @@ -126,7 +123,7 @@ def scan_data_infos_for_update_3d( def comment_constructor(): return construct_comment_for_gridscan( - dummy_params, dummy_params.hyperion_params.ispyb_params, xz_grid_scan_info + dummy_params.hyperion_params.ispyb_params, xz_grid_scan_info ) xz_data_collection_info = populate_remaining_data_collection_info( diff --git a/tests/unit_tests/experiment_plans/test_grid_detection_plan.py b/tests/unit_tests/experiment_plans/test_grid_detection_plan.py index 48b8dbc36..89e142e8d 100644 --- a/tests/unit_tests/experiment_plans/test_grid_detection_plan.py +++ b/tests/unit_tests/experiment_plans/test_grid_detection_plan.py @@ -274,6 +274,16 @@ def decorated(): "oav_snapshot_box_width": 16, }, ) + assert_event( + cb.activity_gated_event.mock_calls[1], + { + "oav_snapshot_top_left_x": 8, + "oav_snapshot_top_left_y": 2, + "oav_snapshot_num_boxes_x": 8, + "oav_snapshot_num_boxes_y": 1, + "oav_snapshot_box_width": 16, + }, + ) @patch("dodal.beamlines.beamline_utils.active_device_is_same_type", lambda a, b: True) diff --git a/tests/unit_tests/external_interaction/callbacks/conftest.py b/tests/unit_tests/external_interaction/callbacks/conftest.py index e75ae58f0..4f5d22034 100644 --- a/tests/unit_tests/external_interaction/callbacks/conftest.py +++ b/tests/unit_tests/external_interaction/callbacks/conftest.py @@ -129,7 +129,7 @@ class TestData: } test_descriptor_document_oav_snapshot: EventDescriptor = { "uid": "b5ba4aec-de49-4970-81a4-b4a847391d34", - "run_start": "?", + "run_start": "d8bee3ee-f614-4e7a-a516-25d6b9e87ef3", "name": CONST.DESCRIPTORS.OAV_SNAPSHOT_TRIGGERED, } # type: ignore test_descriptor_document_pre_data_collection: EventDescriptor = { @@ -147,9 +147,25 @@ class TestData: "run_start": "d8bee3ee-f614-4e7a-a516-25d6b9e87ef3", "name": CONST.DESCRIPTORS.ZOCALO_HW_READ, } # type: ignore - test_event_document_oav_snapshot: Event = { + test_event_document_oav_snapshot_xy: Event = { "descriptor": "b5ba4aec-de49-4970-81a4-b4a847391d34", - "data": {}, + "data": { + "oav_snapshot_top_left_x": 50, + "oav_snapshot_top_left_y": 100, + "oav_snapshot_num_boxes_x": 40, + "oav_snapshot_num_boxes_y": 20, + "oav_snapshot_box_width": 0.1 * 1000 / 1.25, # size in pixels + }, + } + test_event_document_oav_snapshot_xz: Event = { + "descriptor": "b5ba4aec-de49-4970-81a4-b4a847391d34", + "data": { + "oav_snapshot_top_left_x": 50, + "oav_snapshot_top_left_y": 0, + "oav_snapshot_num_boxes_x": 40, + "oav_snapshot_num_boxes_y": 10, + "oav_snapshot_box_width": 0.1 * 1000 / 1.25, # size in pixels + }, } test_event_document_pre_data_collection: Event = { "descriptor": "bd45c2e5-2b85-4280-95d7-a9a15800a78b", diff --git a/tests/unit_tests/external_interaction/callbacks/rotation/test_ispyb_callback.py b/tests/unit_tests/external_interaction/callbacks/rotation/test_ispyb_callback.py index 8b18765d8..ffb530f98 100644 --- a/tests/unit_tests/external_interaction/callbacks/rotation/test_ispyb_callback.py +++ b/tests/unit_tests/external_interaction/callbacks/rotation/test_ispyb_callback.py @@ -82,7 +82,7 @@ def test_activity_gated_start(mock_ispyb_conn, test_rotation_start_outer_documen "hyperion.external_interaction.callbacks.common.ispyb_mapping.get_current_time_string", new=MagicMock(return_value=EXPECTED_START_TIME), ) -def test_activity_gated_event( +def test_hardware_and_flux_read_events( mock_ispyb_conn, dummy_rotation_params, test_rotation_start_outer_document ): callback = RotationISPyBCallback() diff --git a/tests/unit_tests/external_interaction/callbacks/xray_centre/test_ispyb_callback.py b/tests/unit_tests/external_interaction/callbacks/xray_centre/test_ispyb_callback.py index 73af6913f..9a69f3aae 100644 --- a/tests/unit_tests/external_interaction/callbacks/xray_centre/test_ispyb_callback.py +++ b/tests/unit_tests/external_interaction/callbacks/xray_centre/test_ispyb_callback.py @@ -27,9 +27,6 @@ "focal_spot_size_at_sampley": 0.0, "beamsize_at_samplex": 0.1, "beamsize_at_sampley": 0.1, - "comments": "Hyperion: Xray centring - Diffraction grid scan of 40 by 20 " - "images in 100.0 um by 100.0 um steps. Top left (px): [50,100], " - "bottom right (px): [3250,1700].", "data_collection_number": 0, "detector_distance": 100.0, "exp_time": 0.1, @@ -51,7 +48,6 @@ "undulator_gap1": None, "starttime": EXPECTED_START_TIME, "filetemplate": "file_name_0_master.h5", - "nimages": 40 * 20, } EXPECTED_DATA_COLLECTION_2D = { @@ -66,9 +62,6 @@ "focal_spot_size_at_sampley": 0.0, "beamsize_at_samplex": 0.1, "beamsize_at_sampley": 0.1, - "comments": "Hyperion: Xray centring - Diffraction grid scan of 40 by 20 " - "images in 100.0 um by 100.0 um steps. Top left (px): [50,100], " - "bottom right (px): [450,300].", "data_collection_number": 0, "detector_distance": 100.0, "exp_time": 0.1, @@ -90,7 +83,6 @@ "undulator_gap1": None, "starttime": EXPECTED_START_TIME, "filetemplate": "file_name_0_master.h5", - "nimages": 40 * 20, } @@ -98,228 +90,242 @@ "hyperion.external_interaction.callbacks.common.ispyb_mapping.get_current_time_string", new=MagicMock(return_value=EXPECTED_START_TIME), ) -def test_activity_gated_start_2d(mock_ispyb_conn): - callback = GridscanISPyBCallback() - callback.activity_gated_start( - TestData.test_gridscan2d_start_document # pyright: ignore - ) - mx_acq = mx_acquisition_from_conn(mock_ispyb_conn) - assert_upsert_call_with( - mx_acq.upsert_data_collection_group.mock_calls[0], # pyright: ignore - mx_acq.get_data_collection_group_params(), - { - "parentid": TEST_SESSION_ID, - "experimenttype": "mesh", - "sampleid": TEST_SAMPLE_ID, - }, - ) - assert_upsert_call_with( - mx_acq.upsert_data_collection.mock_calls[0], - mx_acq.get_data_collection_params(), - EXPECTED_DATA_COLLECTION_2D, - ) - mx_acq.upsert_data_collection.update_dc_position.assert_not_called() - mx_acq.upsert_data_collection.upsert_dc_grid.assert_not_called() +class TestXrayCentreISPyBCallback: + def test_activity_gated_start_2d(self, mock_ispyb_conn): + callback = GridscanISPyBCallback() + callback.activity_gated_start( + TestData.test_gridscan2d_start_document # pyright: ignore + ) + mx_acq = mx_acquisition_from_conn(mock_ispyb_conn) + assert_upsert_call_with( + mx_acq.upsert_data_collection_group.mock_calls[0], # pyright: ignore + mx_acq.get_data_collection_group_params(), + { + "parentid": TEST_SESSION_ID, + "experimenttype": "mesh", + "sampleid": TEST_SAMPLE_ID, + }, + ) + assert_upsert_call_with( + mx_acq.upsert_data_collection.mock_calls[0], + mx_acq.get_data_collection_params(), + EXPECTED_DATA_COLLECTION_2D, + ) + mx_acq.upsert_data_collection.update_dc_position.assert_not_called() + mx_acq.upsert_data_collection.upsert_dc_grid.assert_not_called() + def test_hardware_and_flux_read_events_2d(self, mock_ispyb_conn): + callback = GridscanISPyBCallback() + callback.activity_gated_start( + TestData.test_gridscan2d_start_document # pyright: ignore + ) + mx_acq = mx_acquisition_from_conn(mock_ispyb_conn) + mx_acq.upsert_data_collection_group.reset_mock() + mx_acq.upsert_data_collection.reset_mock() + callback.activity_gated_descriptor( + TestData.test_descriptor_document_pre_data_collection + ) + callback.activity_gated_event(TestData.test_event_document_pre_data_collection) + callback.activity_gated_descriptor( + TestData.test_descriptor_document_during_data_collection + ) + callback.activity_gated_event( + TestData.test_event_document_during_data_collection + ) -@patch( - "hyperion.external_interaction.callbacks.common.ispyb_mapping.get_current_time_string", - new=MagicMock(return_value=EXPECTED_START_TIME), -) -def test_activity_gated_event_2d(mock_ispyb_conn): - callback = GridscanISPyBCallback() - callback.activity_gated_start( - TestData.test_gridscan2d_start_document # pyright: ignore - ) - mx_acq = mx_acquisition_from_conn(mock_ispyb_conn) - mx_acq.upsert_data_collection_group.reset_mock() - mx_acq.upsert_data_collection.reset_mock() - callback.activity_gated_descriptor( - TestData.test_descriptor_document_pre_data_collection - ) - callback.activity_gated_event(TestData.test_event_document_pre_data_collection) - callback.activity_gated_descriptor( - TestData.test_descriptor_document_during_data_collection - ) - callback.activity_gated_event(TestData.test_event_document_during_data_collection) + assert_upsert_call_with( + mx_acq.upsert_data_collection_group.mock_calls[0], # pyright: ignore + mx_acq.get_data_collection_group_params(), + { + "id": TEST_DATA_COLLECTION_GROUP_ID, + "parentid": TEST_SESSION_ID, + "experimenttype": "mesh", + "sampleid": TEST_SAMPLE_ID, + "sample_barcode": "BARCODE", # deferred + }, + ) + assert_upsert_call_with( + mx_acq.upsert_data_collection.mock_calls[0], + mx_acq.get_data_collection_params(), + EXPECTED_DATA_COLLECTION_2D + | { + "id": TEST_DATA_COLLECTION_IDS[0], + "slitgaphorizontal": 0.1234, + "slitgapvertical": 0.2345, + "synchrotronmode": "test", + "undulatorgap1": 1.234, + "wavelength": 1.1164718451643736, + "transmission": 100, + "flux": 10, + }, + ) + assert_upsert_call_with( + mx_acq.update_dc_position.mock_calls[0], + mx_acq.get_dc_position_params(), + { + "id": TEST_DATA_COLLECTION_IDS[0], + "pos_x": 0, + "pos_y": 0, + "pos_z": 0, + }, + ) + mx_acq.upsert_dc_grid.assert_not_called() - assert_upsert_call_with( - mx_acq.upsert_data_collection_group.mock_calls[0], # pyright: ignore - mx_acq.get_data_collection_group_params(), - { - "id": TEST_DATA_COLLECTION_GROUP_ID, - "parentid": TEST_SESSION_ID, - "experimenttype": "mesh", - "sampleid": TEST_SAMPLE_ID, - "sample_barcode": "BARCODE", # deferred - }, - ) - assert_upsert_call_with( - mx_acq.upsert_data_collection.mock_calls[0], - mx_acq.get_data_collection_params(), - EXPECTED_DATA_COLLECTION_2D - | { - "id": TEST_DATA_COLLECTION_IDS[0], - "slitgaphorizontal": 0.1234, - "slitgapvertical": 0.2345, - "synchrotronmode": "test", - "undulatorgap1": 1.234, - "wavelength": 1.1164718451643736, - "transmission": 100, - "flux": 10, - }, - ) - assert_upsert_call_with( - mx_acq.update_dc_position.mock_calls[0], - mx_acq.get_dc_position_params(), - { - "id": TEST_DATA_COLLECTION_IDS[0], - "pos_x": 0, - "pos_y": 0, - "pos_z": 0, - }, - ) - assert_upsert_call_with( - mx_acq.upsert_dc_grid.mock_calls[0], - mx_acq.get_dc_grid_params(), - { - "parentid": TEST_DATA_COLLECTION_IDS[0], - "dxinmm": 0.1, - "dyinmm": 0.1, - "stepsx": 40, - "stepsy": 20, - "micronsperpixelx": 10, - "micronsperpixely": 10, - "snapshotoffsetxpixel": 50, - "snapshotoffsetypixel": 100, - "orientation": "horizontal", - "snaked": True, - }, - ) + def test_activity_gated_start_3d(self, mock_ispyb_conn): + callback = GridscanISPyBCallback() + callback.activity_gated_start(TestData.test_start_document) # pyright: ignore + mx_acq = mx_acquisition_from_conn(mock_ispyb_conn) + assert_upsert_call_with( + mx_acq.upsert_data_collection_group.mock_calls[0], # pyright: ignore + mx_acq.get_data_collection_group_params(), + { + "parentid": TEST_SESSION_ID, + "experimenttype": "Mesh3D", + "sampleid": TEST_SAMPLE_ID, + }, + ) + assert_upsert_call_with( + mx_acq.upsert_data_collection.mock_calls[0], + mx_acq.get_data_collection_params(), + EXPECTED_DATA_COLLECTION_3D, + ) + mx_acq.upsert_data_collection.update_dc_position.assert_not_called() + mx_acq.upsert_data_collection.upsert_dc_grid.assert_not_called() + def test_hardware_and_flux_read_events_3d(self, mock_ispyb_conn): + callback = GridscanISPyBCallback() + callback.activity_gated_start(TestData.test_start_document) # pyright: ignore + mx_acq = mx_acquisition_from_conn(mock_ispyb_conn) + mx_acq.upsert_data_collection_group.reset_mock() + mx_acq.upsert_data_collection.reset_mock() + callback.activity_gated_descriptor( + TestData.test_descriptor_document_pre_data_collection + ) + callback.activity_gated_event(TestData.test_event_document_pre_data_collection) + callback.activity_gated_descriptor( + TestData.test_descriptor_document_during_data_collection + ) + callback.activity_gated_event( + TestData.test_event_document_during_data_collection + ) -@patch( - "hyperion.external_interaction.callbacks.common.ispyb_mapping.get_current_time_string", - new=MagicMock(return_value=EXPECTED_START_TIME), -) -def test_activity_gated_start_3d(mock_ispyb_conn): - callback = GridscanISPyBCallback() - callback.activity_gated_start(TestData.test_start_document) # pyright: ignore - mx_acq = mx_acquisition_from_conn(mock_ispyb_conn) - assert_upsert_call_with( - mx_acq.upsert_data_collection_group.mock_calls[0], # pyright: ignore - mx_acq.get_data_collection_group_params(), - { - "parentid": TEST_SESSION_ID, - "experimenttype": "Mesh3D", - "sampleid": TEST_SAMPLE_ID, - }, - ) - assert_upsert_call_with( - mx_acq.upsert_data_collection.mock_calls[0], - mx_acq.get_data_collection_params(), - EXPECTED_DATA_COLLECTION_3D, - ) - mx_acq.upsert_data_collection.update_dc_position.assert_not_called() - mx_acq.upsert_data_collection.upsert_dc_grid.assert_not_called() + assert_upsert_call_with( + mx_acq.upsert_data_collection_group.mock_calls[0], # pyright: ignore + mx_acq.get_data_collection_group_params(), + { + "id": TEST_DATA_COLLECTION_GROUP_ID, + "parentid": TEST_SESSION_ID, + "experimenttype": "Mesh3D", + "sampleid": TEST_SAMPLE_ID, + "sample_barcode": "BARCODE", # deferred + }, + ) + assert_upsert_call_with( + mx_acq.upsert_data_collection.mock_calls[0], + mx_acq.get_data_collection_params(), + EXPECTED_DATA_COLLECTION_3D + | { + "id": TEST_DATA_COLLECTION_IDS[0], + "slitgaphorizontal": 0.1234, + "slitgapvertical": 0.2345, + "synchrotronmode": "test", + "undulatorgap1": 1.234, + "wavelength": 1.1164718451643736, + "transmission": 100, + "flux": 10, + }, + ) + assert_upsert_call_with( + mx_acq.update_dc_position.mock_calls[0], + mx_acq.get_dc_position_params(), + { + "id": TEST_DATA_COLLECTION_IDS[0], + "pos_x": 0, + "pos_y": 0, + "pos_z": 0, + }, + ) + assert_upsert_call_with( + mx_acq.update_dc_position.mock_calls[1], + mx_acq.get_dc_position_params(), + { + "id": TEST_DATA_COLLECTION_IDS[1], + "pos_x": 0, + "pos_y": 0, + "pos_z": 0, + }, + ) + mx_acq.upsert_dc_grid.assert_not_called() + def test_activity_gated_event_oav_snapshot_triggered(self, mock_ispyb_conn): + callback = GridscanISPyBCallback() + callback.activity_gated_start(TestData.test_start_document) # pyright: ignore + mx_acq = mx_acquisition_from_conn(mock_ispyb_conn) + mx_acq.upsert_data_collection.assert_called_once() + mx_acq.upsert_data_collection_group.reset_mock() + mx_acq.upsert_data_collection.reset_mock() -@patch( - "hyperion.external_interaction.callbacks.common.ispyb_mapping.get_current_time_string", - new=MagicMock(return_value=EXPECTED_START_TIME), -) -def test_activity_gated_event_3d(mock_ispyb_conn): - callback = GridscanISPyBCallback() - callback.activity_gated_start(TestData.test_start_document) # pyright: ignore - mx_acq = mx_acquisition_from_conn(mock_ispyb_conn) - mx_acq.upsert_data_collection_group.reset_mock() - mx_acq.upsert_data_collection.reset_mock() - callback.activity_gated_descriptor( - TestData.test_descriptor_document_pre_data_collection - ) - callback.activity_gated_event(TestData.test_event_document_pre_data_collection) - callback.activity_gated_descriptor( - TestData.test_descriptor_document_during_data_collection - ) - callback.activity_gated_event(TestData.test_event_document_during_data_collection) + callback.activity_gated_descriptor( + TestData.test_descriptor_document_oav_snapshot + ) + callback.activity_gated_event(TestData.test_event_document_oav_snapshot_xy) + callback.activity_gated_event(TestData.test_event_document_oav_snapshot_xz) - assert_upsert_call_with( - mx_acq.upsert_data_collection_group.mock_calls[0], # pyright: ignore - mx_acq.get_data_collection_group_params(), - { - "id": TEST_DATA_COLLECTION_GROUP_ID, - "parentid": TEST_SESSION_ID, - "experimenttype": "Mesh3D", - "sampleid": TEST_SAMPLE_ID, - "sample_barcode": "BARCODE", # deferred - }, - ) - assert_upsert_call_with( - mx_acq.upsert_data_collection.mock_calls[0], - mx_acq.get_data_collection_params(), - EXPECTED_DATA_COLLECTION_3D - | { - "id": TEST_DATA_COLLECTION_IDS[0], - "slitgaphorizontal": 0.1234, - "slitgapvertical": 0.2345, - "synchrotronmode": "test", - "undulatorgap1": 1.234, - "wavelength": 1.1164718451643736, - "transmission": 100, - "flux": 10, - }, - ) - assert_upsert_call_with( - mx_acq.update_dc_position.mock_calls[0], - mx_acq.get_dc_position_params(), - { - "id": TEST_DATA_COLLECTION_IDS[0], - "pos_x": 0, - "pos_y": 0, - "pos_z": 0, - }, - ) - assert_upsert_call_with( - mx_acq.upsert_dc_grid.mock_calls[0], - mx_acq.get_dc_grid_params(), - { - "parentid": TEST_DATA_COLLECTION_IDS[0], - "dxinmm": 0.1, - "dyinmm": 0.1, - "stepsx": 40, - "stepsy": 20, - "micronsperpixelx": 1.25, - "micronsperpixely": 1.25, - "snapshotoffsetxpixel": 50, - "snapshotoffsetypixel": 100, - "orientation": "horizontal", - "snaked": True, - }, - ) - assert_upsert_call_with( - mx_acq.update_dc_position.mock_calls[1], - mx_acq.get_dc_position_params(), - { - "id": TEST_DATA_COLLECTION_IDS[1], - "pos_x": 0, - "pos_y": 0, - "pos_z": 0, - }, - ) - assert_upsert_call_with( - mx_acq.upsert_dc_grid.mock_calls[1], - mx_acq.get_dc_grid_params(), - { - "parentid": TEST_DATA_COLLECTION_IDS[1], - "dxinmm": 0.1, - "dyinmm": 0.1, - "stepsx": 40, - "stepsy": 10, - "micronsperpixelx": 1.25, - "micronsperpixely": 1.25, - "snapshotoffsetxpixel": 50, - "snapshotoffsetypixel": 0, - "orientation": "horizontal", - "snaked": True, - }, - ) + mx_acq.upsert_data_collection_group.assert_not_called() + assert_upsert_call_with( + mx_acq.upsert_data_collection.mock_calls[0], + mx_acq.get_data_collection_params(), + { + "id": TEST_DATA_COLLECTION_IDS[0], + "parentid": TEST_DATA_COLLECTION_GROUP_ID, + "nimages": 40 * 20, + "comments": "Hyperion: Xray centring - Diffraction grid scan of 40 by 20 " + "images in 100.0 um by 100.0 um steps. Top left (px): [50,100], " + "bottom right (px): [3250,1700].", + }, + ) + assert_upsert_call_with( + mx_acq.upsert_data_collection.mock_calls[1], + mx_acq.get_data_collection_params(), + { + "parentid": TEST_DATA_COLLECTION_GROUP_ID, + "nimages": 40 * 10, + "comments": "Hyperion: Xray centring - Diffraction grid scan of 40 by 10 " + "images in 100.0 um by 100.0 um steps. Top left (px): [50,0], " + "bottom right (px): [3250,800].", + }, + ) + assert_upsert_call_with( + mx_acq.upsert_dc_grid.mock_calls[0], + mx_acq.get_dc_grid_params(), + { + "parentid": TEST_DATA_COLLECTION_IDS[0], + "dxinmm": 0.1, + "dyinmm": 0.1, + "stepsx": 40, + "stepsy": 20, + "micronsperpixelx": 1.25, + "micronsperpixely": 1.25, + "snapshotoffsetxpixel": 50, + "snapshotoffsetypixel": 100, + "orientation": "horizontal", + "snaked": True, + }, + ) + assert_upsert_call_with( + mx_acq.upsert_dc_grid.mock_calls[1], + mx_acq.get_dc_grid_params(), + { + "parentid": TEST_DATA_COLLECTION_IDS[1], + "dxinmm": 0.1, + "dyinmm": 0.1, + "stepsx": 40, + "stepsy": 10, + "micronsperpixelx": 1.25, + "micronsperpixely": 1.25, + "snapshotoffsetxpixel": 50, + "snapshotoffsetypixel": 0, + "orientation": "horizontal", + "snaked": True, + }, + ) diff --git a/tests/unit_tests/external_interaction/callbacks/xray_centre/test_ispyb_mapping.py b/tests/unit_tests/external_interaction/callbacks/xray_centre/test_ispyb_mapping.py index d26191940..96feb3668 100644 --- a/tests/unit_tests/external_interaction/callbacks/xray_centre/test_ispyb_mapping.py +++ b/tests/unit_tests/external_interaction/callbacks/xray_centre/test_ispyb_mapping.py @@ -6,7 +6,6 @@ from hyperion.external_interaction.callbacks.common.ispyb_mapping import GridScanInfo from hyperion.external_interaction.callbacks.xray_centre.ispyb_mapping import ( construct_comment_for_gridscan, - populate_xy_data_collection_info, ) from hyperion.parameters.plan_specific.gridscan_internal_params import ( GridscanInternalParameters, @@ -29,27 +28,6 @@ def dummy_params(): return dummy_params -def test_given_x_and_y_steps_different_from_total_images_when_grid_scan_stored_then_num_images_correct( - dummy_params: GridscanInternalParameters, -): - expected_number_of_steps = 200 * 3 - dummy_params.experiment_params.x_steps = 200 - dummy_params.experiment_params.y_steps = 3 - grid_scan_info = GridScanInfo( - dummy_params.hyperion_params.ispyb_params.upper_left, - 3, - dummy_params.experiment_params.y_step_size, - ) - actual = populate_xy_data_collection_info( - grid_scan_info, - dummy_params, - dummy_params.hyperion_params.ispyb_params, - dummy_params.hyperion_params.detector_params, - ) - - assert actual.n_images == expected_number_of_steps - - @patch("ispyb.open", autospec=True) def test_ispyb_deposition_rounds_position_to_int( mock_ispyb_conn: MagicMock, @@ -58,9 +36,10 @@ def test_ispyb_deposition_rounds_position_to_int( dummy_params.hyperion_params.ispyb_params.upper_left = np.array([0.01, 100, 50]) assert construct_comment_for_gridscan( - dummy_params, dummy_params.hyperion_params.ispyb_params, - GridScanInfo(dummy_params.hyperion_params.ispyb_params.upper_left, 20, 0.1), + GridScanInfo( + dummy_params.hyperion_params.ispyb_params.upper_left, 40, 20, 0.1, 0.1 + ), ) == ( "Hyperion: Xray centring - Diffraction grid scan of 40 by 20 images " "in 100.0 um by 100.0 um steps. Top left (px): [0,100], bottom right (px): [3200,1700]." @@ -89,8 +68,6 @@ def test_ispyb_deposition_rounds_box_size_int( raw, rounded, ): - dummy_params.experiment_params.x_steps = 0 - dummy_params.experiment_params.x_step_size = raw grid_scan_info = GridScanInfo( [ 0, @@ -98,13 +75,13 @@ def test_ispyb_deposition_rounds_box_size_int( 0, ], 0, + 0, + raw, raw, ) - bottom_right_from_top_left.return_value = grid_scan_info.upper_left + bottom_right_from_top_left.return_value = grid_scan_info.upper_left_px - assert construct_comment_for_gridscan( - dummy_params, MagicMock(), grid_scan_info - ) == ( + assert construct_comment_for_gridscan(MagicMock(), grid_scan_info) == ( "Hyperion: Xray centring - Diffraction grid scan of 0 by 0 images in " f"{rounded} um by {rounded} um steps. Top left (px): [0,0], bottom right (px): [0,0]." ) diff --git a/tests/unit_tests/external_interaction/ispyb/conftest.py b/tests/unit_tests/external_interaction/ispyb/conftest.py index 531a38195..d08452a13 100644 --- a/tests/unit_tests/external_interaction/ispyb/conftest.py +++ b/tests/unit_tests/external_interaction/ispyb/conftest.py @@ -7,6 +7,7 @@ DataCollectionGridInfo, DataCollectionPositionInfo, ExperimentType, + ScanDataInfo, ) from hyperion.external_interaction.ispyb.ispyb_dataclass import Orientation from hyperion.external_interaction.ispyb.ispyb_store import StoreInIspyb @@ -17,6 +18,7 @@ from ..conftest import ( TEST_DATA_COLLECTION_GROUP_ID, + TEST_DATA_COLLECTION_IDS, TEST_SAMPLE_ID, default_raw_params, ) @@ -51,7 +53,9 @@ def dummy_2d_gridscan_ispyb(): @pytest.fixture -def scan_xy_data_info_for_update(scan_data_info_for_begin): +def scan_xy_data_info_for_update( + scan_data_info_for_begin: ScanDataInfo, +) -> ScanDataInfo: scan_data_info_for_update = deepcopy(scan_data_info_for_begin) scan_data_info_for_update.data_collection_info.parent_id = ( TEST_DATA_COLLECTION_GROUP_ID @@ -73,4 +77,5 @@ def scan_xy_data_info_for_update(scan_data_info_for_begin): scan_data_info_for_update.data_collection_position_info = ( DataCollectionPositionInfo(0, 0, 0) ) + scan_data_info_for_update.data_collection_id = TEST_DATA_COLLECTION_IDS[0] return scan_data_info_for_update diff --git a/tests/unit_tests/external_interaction/ispyb/test_gridscan_ispyb_store_2d.py b/tests/unit_tests/external_interaction/ispyb/test_gridscan_ispyb_store_2d.py index 35e6f0353..07a71041b 100644 --- a/tests/unit_tests/external_interaction/ispyb/test_gridscan_ispyb_store_2d.py +++ b/tests/unit_tests/external_interaction/ispyb/test_gridscan_ispyb_store_2d.py @@ -128,7 +128,7 @@ def dummy_collection_group_info(): "hyperion.external_interaction.callbacks.common.ispyb_mapping.get_current_time_string", new=MagicMock(return_value=EXPECTED_START_TIME), ) -def scan_data_info_for_begin(): +def scan_data_info_for_begin() -> ScanDataInfo: return ScanDataInfo( data_collection_info=DataCollectionInfo( omega_start=0.0, diff --git a/tests/unit_tests/external_interaction/ispyb/test_gridscan_ispyb_store_3d.py b/tests/unit_tests/external_interaction/ispyb/test_gridscan_ispyb_store_3d.py index 07d0e7af1..58f914797 100644 --- a/tests/unit_tests/external_interaction/ispyb/test_gridscan_ispyb_store_3d.py +++ b/tests/unit_tests/external_interaction/ispyb/test_gridscan_ispyb_store_3d.py @@ -132,7 +132,7 @@ def scan_data_infos_for_update(): undulator_gap1=1.0, start_time=EXPECTED_START_TIME, ), - data_collection_id=None, + data_collection_id=TEST_DATA_COLLECTION_IDS[0], data_collection_position_info=DataCollectionPositionInfo( pos_x=0, pos_y=0, pos_z=0 ), @@ -255,6 +255,7 @@ def test_store_3d_grid_scan( assert ispyb_ids == IspybIds( data_collection_ids=(TEST_DATA_COLLECTION_IDS[0],), data_collection_group_id=TEST_DATA_COLLECTION_GROUP_ID, + data_collection_id=TEST_DATA_COLLECTION_IDS[0], ) assert dummy_3d_gridscan_ispyb.update_deposition(