diff --git a/.gitignore b/.gitignore index 7becea822..19bbd143c 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ __pycache__/ # Output *.png +!docs/*.png # Distribution / packaging .Python diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 1d021baa4..dc89911c1 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -14,11 +14,10 @@ repos: - id: no-images name: Check for image files entry: > - Images for documentation should go into the documentation repository - https://github.com/dials/dials.github.io + Images for documentation should go into the docs folder language: fail files: '.*\.png$' - + exclude: '^docs/' - id: ruff name: Run ruff stages: [commit] diff --git a/docs/param_hierarchy-Hyperion_Parameter_Model.png b/docs/param_hierarchy-Hyperion_Parameter_Model.png new file mode 100644 index 000000000..c2f277010 Binary files /dev/null and b/docs/param_hierarchy-Hyperion_Parameter_Model.png differ diff --git a/docs/param_hierarchy.puml b/docs/param_hierarchy.puml new file mode 100644 index 000000000..9646ae6e8 --- /dev/null +++ b/docs/param_hierarchy.puml @@ -0,0 +1,66 @@ +@startuml +'https://plantuml.com/class-diagram +title Hyperion Parameter Model + +abstract class BaseModel + +package Mixins { + class WithSample + class WithScan + class WithOavCentring + class WithSnapshot + class OptionalXyzStarts + class XyzStarts + class OptionalGonioAngleStarts + class SplitScan +} + +class HyperionParameters +note bottom: Base class for all experiment parameter models + +package Experiments { + class DiffractionExperiment + class DiffractionExperimentWithSample + class GridCommon + class GridScanWithEdgeDetect + class PinTipCentreThenXrayCentre + class RotationScan + class RobotLoadThenCentre + class SpecifiedGridScan + class ThreeDGridScan +} +class TemporaryIspybExtras +note bottom: To be removed + + +BaseModel <|-- HyperionParameters +BaseModel <|-- SplitScan +BaseModel <|-- OptionalGonioAngleStarts +BaseModel <|-- OptionalXyzStarts +BaseModel <|-- TemporaryIspybExtras +BaseModel <|-- WithOavCentring +BaseModel <|-- WithSnapshot +BaseModel <|-- WithSample +BaseModel <|-- WithScan +BaseModel <|-- XyzStarts + +RotationScan *-- TemporaryIspybExtras +HyperionParameters <|-- DiffractionExperiment +WithSnapshot <|-- DiffractionExperiment +DiffractionExperiment <|-- DiffractionExperimentWithSample +WithSample <|-- DiffractionExperimentWithSample +DiffractionExperimentWithSample <|-- GridCommon +GridCommon <|-- GridScanWithEdgeDetect +GridCommon <|-- PinTipCentreThenXrayCentre +GridCommon <|-- RobotLoadThenCentre +GridCommon <|-- SpecifiedGridScan +WithScan <|-- SpecifiedGridScan +SpecifiedGridScan <|-- ThreeDGridScan +SplitScan <|-- ThreeDGridScan +WithOavCentring <|-- GridCommon +DiffractionExperimentWithSample <|-- RotationScan +OptionalXyzStarts <|-- RotationScan +XyzStarts <|-- SpecifiedGridScan +OptionalGonioAngleStarts <|-- GridCommon +OptionalGonioAngleStarts <|-- RotationScan +@enduml \ No newline at end of file diff --git a/src/hyperion/device_setup_plans/setup_oav.py b/src/hyperion/device_setup_plans/setup_oav.py index c4b59b91e..a1cee8c90 100644 --- a/src/hyperion/device_setup_plans/setup_oav.py +++ b/src/hyperion/device_setup_plans/setup_oav.py @@ -54,21 +54,12 @@ def setup_pin_tip_detection_params( ) -def pre_centring_setup_oav( - oav: OAV, - parameters: OAVParameters, - pin_tip_detection_device: PinTipDetection, -): - """ - Setup OAV PVs with required values. - """ +def setup_general_oav_params(oav: OAV, parameters: OAVParameters): yield from set_using_group(oav.cam.color_mode, ColorMode.RGB1) yield from set_using_group(oav.cam.acquire_period, parameters.acquire_period) yield from set_using_group(oav.cam.acquire_time, parameters.exposure) yield from set_using_group(oav.cam.gain, parameters.gain) - yield from setup_pin_tip_detection_params(pin_tip_detection_device, parameters) - zoom_level_str = f"{float(parameters.zoom)}x" if zoom_level_str not in oav.zoom_controller.allowed_zoom_levels: raise OAVError_ZoomLevelNotFound( @@ -81,6 +72,17 @@ def pre_centring_setup_oav( wait=True, ) + +def pre_centring_setup_oav( + oav: OAV, + parameters: OAVParameters, + pin_tip_detection_device: PinTipDetection, +): + """ + Setup OAV PVs with required values. + """ + yield from setup_general_oav_params(oav, parameters) + yield from setup_pin_tip_detection_params(pin_tip_detection_device, parameters) yield from bps.wait(oav_group) """ diff --git a/src/hyperion/experiment_plans/oav_grid_detection_plan.py b/src/hyperion/experiment_plans/oav_grid_detection_plan.py index 8103fc30d..839c50415 100644 --- a/src/hyperion/experiment_plans/oav_grid_detection_plan.py +++ b/src/hyperion/experiment_plans/oav_grid_detection_plan.py @@ -159,7 +159,7 @@ def grid_detection_plan( yield from bps.abs_set(oav.grid_snapshot.filename, snapshot_filename) yield from bps.abs_set(oav.grid_snapshot.directory, snapshot_dir) yield from bps.trigger(oav.grid_snapshot, wait=True) - yield from bps.create(CONST.DESCRIPTORS.OAV_SNAPSHOT_TRIGGERED) + yield from bps.create(CONST.DESCRIPTORS.OAV_GRID_SNAPSHOT_TRIGGERED) yield from bps.read(oav.grid_snapshot) yield from bps.read(smargon) diff --git a/src/hyperion/experiment_plans/oav_snapshot_plan.py b/src/hyperion/experiment_plans/oav_snapshot_plan.py new file mode 100644 index 000000000..0a6167a38 --- /dev/null +++ b/src/hyperion/experiment_plans/oav_snapshot_plan.py @@ -0,0 +1,64 @@ +import dataclasses +from datetime import datetime + +from blueapi.core import BlueskyContext, MsgGenerator +from bluesky import plan_stubs as bps +from dodal.devices.oav.oav_detector import OAV +from dodal.devices.oav.oav_parameters import OAVParameters +from dodal.devices.smargon import Smargon + +from hyperion.device_setup_plans.setup_oav import setup_general_oav_params +from hyperion.parameters.components import WithSnapshot +from hyperion.parameters.constants import DocDescriptorNames +from hyperion.utils.context import device_composite_from_context + +OAV_SNAPSHOT_GROUP = "oav_snapshot_group" + + +@dataclasses.dataclass +class OavSnapshotComposite: + smargon: Smargon + oav: OAV + + +def create_devices(context: BlueskyContext) -> OavSnapshotComposite: + return device_composite_from_context(context, OavSnapshotComposite) # type: ignore + + +def _setup_oav( + composite: OavSnapshotComposite, + parameters: WithSnapshot, + oav_parameters: OAVParameters, +): + yield from setup_general_oav_params(composite.oav, oav_parameters) + yield from bps.abs_set( + composite.oav.snapshot.directory, str(parameters.snapshot_directory) + ) + + +def _take_oav_snapshot( + composite: OavSnapshotComposite, parameters: WithSnapshot, omega: float +): + yield from bps.abs_set(composite.smargon.omega, omega, group=OAV_SNAPSHOT_GROUP) + time_now = datetime.now() + filename = f"{time_now.strftime('%H%M%S')}_oav_snapshot_{omega:.0f}" + yield from bps.abs_set(composite.oav.snapshot.filename, filename) + yield from bps.trigger(composite.oav.snapshot, group=OAV_SNAPSHOT_GROUP) + yield from bps.wait(group=OAV_SNAPSHOT_GROUP) + yield from bps.create(DocDescriptorNames.OAV_ROTATION_SNAPSHOT_TRIGGERED) + yield from bps.read(composite.oav.snapshot) + yield from bps.save() + + +def oav_snapshot_plan( + composite: OavSnapshotComposite, + parameters: WithSnapshot, + oav_parameters: OAVParameters, + wait: bool = True, +) -> MsgGenerator: + omegas = parameters.snapshot_omegas_deg + if not omegas: + return + yield from _setup_oav(composite, parameters, oav_parameters) + for omega in omegas: + yield from _take_oav_snapshot(composite, parameters, omega) diff --git a/src/hyperion/external_interaction/callbacks/common/ispyb_mapping.py b/src/hyperion/external_interaction/callbacks/common/ispyb_mapping.py index 9241d56f2..57cfb5199 100644 --- a/src/hyperion/external_interaction/callbacks/common/ispyb_mapping.py +++ b/src/hyperion/external_interaction/callbacks/common/ispyb_mapping.py @@ -18,7 +18,6 @@ VISIT_PATH_REGEX, get_current_time_string, ) -from hyperion.log import ISPYB_LOGGER from hyperion.parameters.components import DiffractionExperimentWithSample @@ -87,15 +86,3 @@ def get_visit_string(ispyb_params: IspybParams, detector_params: DetectorParams) f"Visit not found from {ispyb_params.visit_path} or {detector_params.directory}" ) return visit_path_match - - -def get_xtal_snapshots(ispyb_params): - if ispyb_params.xtal_snapshots_omega_start: - xtal_snapshots = ispyb_params.xtal_snapshots_omega_start[:3] - ISPYB_LOGGER.info( - f"Using rotation scan snapshots {xtal_snapshots} for ISPyB deposition" - ) - else: - ISPYB_LOGGER.warning("No xtal snapshot paths sent to ISPyB!") - xtal_snapshots = [] - return xtal_snapshots + [None] * (3 - len(xtal_snapshots)) diff --git a/src/hyperion/external_interaction/callbacks/ispyb_callback_base.py b/src/hyperion/external_interaction/callbacks/ispyb_callback_base.py index 9129aa6d5..473f4ef6b 100644 --- a/src/hyperion/external_interaction/callbacks/ispyb_callback_base.py +++ b/src/hyperion/external_interaction/callbacks/ispyb_callback_base.py @@ -93,8 +93,10 @@ def activity_gated_event(self, doc: Event) -> Event: match event_descriptor.get("name"): case CONST.DESCRIPTORS.ISPYB_HARDWARE_READ: scan_data_infos = self._handle_ispyb_hardware_read(doc) - case CONST.DESCRIPTORS.OAV_SNAPSHOT_TRIGGERED: - scan_data_infos = self._handle_oav_snapshot_triggered(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 _: @@ -135,7 +137,25 @@ 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_snapshot_triggered(self, doc) -> Sequence[ScanDataInfo]: + 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"] diff --git a/src/hyperion/external_interaction/callbacks/rotation/ispyb_mapping.py b/src/hyperion/external_interaction/callbacks/rotation/ispyb_mapping.py index 0a84c3bb7..4d5e0760a 100644 --- a/src/hyperion/external_interaction/callbacks/rotation/ispyb_mapping.py +++ b/src/hyperion/external_interaction/callbacks/rotation/ispyb_mapping.py @@ -1,9 +1,7 @@ from __future__ import annotations -from hyperion.external_interaction.callbacks.common.ispyb_mapping import ( - get_xtal_snapshots, -) from hyperion.external_interaction.ispyb.data_model import DataCollectionInfo +from hyperion.log import ISPYB_LOGGER from hyperion.parameters.rotation import RotationScan @@ -16,7 +14,22 @@ def populate_data_collection_info_for_rotation(params: RotationScan): axis_end=(params.omega_start_deg + params.scan_width_deg), kappa_start=params.kappa_start_deg, ) - (info.xtal_snapshot1, info.xtal_snapshot2, info.xtal_snapshot3) = ( - get_xtal_snapshots(params.ispyb_params) - ) + ( + info.xtal_snapshot1, + info.xtal_snapshot2, + info.xtal_snapshot3, + info.xtal_snapshot4, + ) = get_xtal_snapshots(params.ispyb_params) return info + + +def get_xtal_snapshots(ispyb_params): + if ispyb_params.xtal_snapshots_omega_start: + xtal_snapshots = ispyb_params.xtal_snapshots_omega_start[:4] + ISPYB_LOGGER.info( + f"Using rotation scan snapshots {xtal_snapshots} for ISPyB deposition" + ) + else: + ISPYB_LOGGER.warning("No xtal snapshot paths sent to ISPyB!") + xtal_snapshots = [] + return xtal_snapshots + [None] * (4 - len(xtal_snapshots)) diff --git a/src/hyperion/external_interaction/ispyb/data_model.py b/src/hyperion/external_interaction/ispyb/data_model.py index e898d0860..f6904df34 100644 --- a/src/hyperion/external_interaction/ispyb/data_model.py +++ b/src/hyperion/external_interaction/ispyb/data_model.py @@ -19,6 +19,7 @@ class DataCollectionInfo: xtal_snapshot1: Optional[str] = None xtal_snapshot2: Optional[str] = None xtal_snapshot3: Optional[str] = None + xtal_snapshot4: Optional[str] = None n_images: Optional[int] = None axis_range: Optional[float] = None diff --git a/src/hyperion/external_interaction/ispyb/ispyb_dataclass.py b/src/hyperion/external_interaction/ispyb/ispyb_dataclass.py index 9fbd472e9..1119b85d8 100644 --- a/src/hyperion/external_interaction/ispyb/ispyb_dataclass.py +++ b/src/hyperion/external_interaction/ispyb/ispyb_dataclass.py @@ -8,8 +8,6 @@ "sample_id": None, "visit_path": "", "position": None, - "xtal_snapshots_omega_start": ["test_1_y", "test_2_y", "test_3_y"], - "xtal_snapshots_omega_end": ["test_1_z", "test_2_z", "test_3_z"], "comment": "Descriptive comment.", } @@ -22,7 +20,6 @@ class IspybParams(BaseModel): # Optional from GDA as populated by Ophyd xtal_snapshots_omega_start: Optional[list[str]] = None - xtal_snapshots_omega_end: Optional[list[str]] = None ispyb_experiment_type: Optional[str] = None class Config: diff --git a/src/hyperion/parameters/components.py b/src/hyperion/parameters/components.py index 557d746fe..d0efd8ed1 100644 --- a/src/hyperion/parameters/components.py +++ b/src/hyperion/parameters/components.py @@ -138,7 +138,12 @@ def from_json(cls, input: str | None, *, allow_extras: bool = False): return params -class DiffractionExperiment(HyperionParameters): +class WithSnapshot(BaseModel): + snapshot_directory: Path + snapshot_omegas_deg: list[float] | None + + +class DiffractionExperiment(HyperionParameters, WithSnapshot): """For all experiments which use beam""" visit: str = Field(min_length=1) @@ -160,7 +165,6 @@ class DiffractionExperiment(HyperionParameters): selected_aperture: AperturePositionGDANames | None = Field(default=None) ispyb_experiment_type: IspybExperimentType storage_directory: str - snapshot_directory: Path @root_validator(pre=True) def validate_snapshot_directory(cls, values): @@ -260,5 +264,3 @@ class Config: extra = Extra.forbid xtal_snapshots_omega_start: list[str] | None = None - xtal_snapshots_omega_end: list[str] | None = None - xtal_snapshots: list[str] | None = None diff --git a/src/hyperion/parameters/constants.py b/src/hyperion/parameters/constants.py index 2d3c8e3ab..b631f557e 100644 --- a/src/hyperion/parameters/constants.py +++ b/src/hyperion/parameters/constants.py @@ -39,7 +39,8 @@ class DocDescriptorNames: # Robot load event descriptor ROBOT_LOAD = "robot_load" # For callbacks to use - OAV_SNAPSHOT_TRIGGERED = "snapshot_to_ispyb" + OAV_ROTATION_SNAPSHOT_TRIGGERED = "rotation_snapshot_triggered" + OAV_GRID_SNAPSHOT_TRIGGERED = "snapshot_to_ispyb" NEXUS_READ = "nexus_read_plan" ISPYB_HARDWARE_READ = "ispyb_reading_hardware" ISPYB_TRANSMISSION_FLUX_READ = "ispyb_update_transmission_flux" diff --git a/src/hyperion/parameters/gridscan.py b/src/hyperion/parameters/gridscan.py index ad5bed030..ddfaa11bf 100644 --- a/src/hyperion/parameters/gridscan.py +++ b/src/hyperion/parameters/gridscan.py @@ -23,7 +23,6 @@ IspybExperimentType, OptionalGonioAngleStarts, SplitScan, - TemporaryIspybExtras, WithOavCentring, WithScan, XyzStarts, @@ -50,8 +49,6 @@ class GridCommon( selected_aperture: AperturePositionGDANames | None = Field( default=AperturePositionGDANames.SMALL_APERTURE ) - # field rather than inherited to make it easier to track when it can be removed: - ispyb_extras: TemporaryIspybExtras @property def ispyb_params(self): @@ -59,9 +56,6 @@ def ispyb_params(self): visit_path=str(self.visit_directory), comment=self.comment, sample_id=self.sample_id, - xtal_snapshots_omega_start=self.ispyb_extras.xtal_snapshots_omega_start - or [], - xtal_snapshots_omega_end=self.ispyb_extras.xtal_snapshots_omega_end or [], ispyb_experiment_type=self.ispyb_experiment_type, ) diff --git a/src/hyperion/parameters/rotation.py b/src/hyperion/parameters/rotation.py index 93f3331d1..ecad11d03 100644 --- a/src/hyperion/parameters/rotation.py +++ b/src/hyperion/parameters/rotation.py @@ -43,7 +43,7 @@ class RotationScan( default=IspybExperimentType.ROTATION ) transmission_frac: float - ispyb_extras: TemporaryIspybExtras + ispyb_extras: TemporaryIspybExtras | None @property def detector_params(self): @@ -80,8 +80,11 @@ def ispyb_params(self): # pyright: ignore return RotationIspybParams( visit_path=str(self.visit_directory), comment=self.comment, - xtal_snapshots_omega_start=self.ispyb_extras.xtal_snapshots_omega_start, - xtal_snapshots_omega_end=self.ispyb_extras.xtal_snapshots_omega_end, + xtal_snapshots_omega_start=( + self.ispyb_extras.xtal_snapshots_omega_start + if self.ispyb_extras + else [] + ), ispyb_experiment_type=self.ispyb_experiment_type, ) diff --git a/tests/system_tests/external_interaction/callbacks/test_external_callbacks.py b/tests/system_tests/external_interaction/callbacks/test_external_callbacks.py index e7b5719d9..ad5a2ac5e 100644 --- a/tests/system_tests/external_interaction/callbacks/test_external_callbacks.py +++ b/tests/system_tests/external_interaction/callbacks/test_external_callbacks.py @@ -18,6 +18,7 @@ from bluesky.callbacks.zmq import Publisher from bluesky.run_engine import RunEngine from dodal.devices.zocalo import ZocaloResults +from ophyd_async.core import set_mock_value from zmq.utils.monitor import recv_monitor_message from hyperion.experiment_plans.flyscan_xray_centre_plan import ( @@ -153,9 +154,7 @@ async def test_external_callbacks_handle_gridscan_ispyb_and_zocalo( RE = RE_with_external_callbacks test_fgs_params.zocalo_environment = "dev_artemis" - fake_fgs_composite.aperture_scatterguard.aperture.z.user_setpoint.sim_put( # type: ignore - 2 - ) + set_mock_value(fake_fgs_composite.aperture_scatterguard.aperture.z.user_setpoint, 2) fake_fgs_composite.eiger.unstage = MagicMock(return_value=done_status) # type: ignore fake_fgs_composite.smargon.stub_offsets.set = MagicMock(return_value=done_status) # type: ignore fake_fgs_composite.zocalo = zocalo_device 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 5026fb462..ecf273dc2 100644 --- a/tests/system_tests/external_interaction/test_ispyb_dev_connection.py +++ b/tests/system_tests/external_interaction/test_ispyb_dev_connection.py @@ -831,6 +831,10 @@ def test_ispyb_deposition_in_rotation_plan( "synchrotronMode": test_synchrotron_mode.value, "slitGapHorizontal": test_slit_gap_horiz, "slitGapVertical": test_slit_gap_vert, + "xtalSnapshotFullPath1": "/tmp/snapshot1.png", + "xtalSnapshotFullPath2": "/tmp/snapshot2.png", + "xtalSnapshotFullPath3": "/tmp/snapshot3.png", + "xtalSnapshotFullPath4": "/tmp/snapshot4.png", } compare_actual_and_expected(dcid, EXPECTED_VALUES, fetch_datacollection_attribute) diff --git a/tests/test_data/parameter_json_files/good_test_grid_with_edge_detect_parameters.json b/tests/test_data/parameter_json_files/good_test_grid_with_edge_detect_parameters.json index e6492a1d8..ac78e41bb 100644 --- a/tests/test_data/parameter_json_files/good_test_grid_with_edge_detect_parameters.json +++ b/tests/test_data/parameter_json_files/good_test_grid_with_edge_detect_parameters.json @@ -15,17 +15,5 @@ "grid_width_um": 290.6, "oav_centring_file": "tests/test_data/test_OAVCentring.json", "transmission_frac": 1.0, - "visit": "cm31105-4", - "ispyb_extras": { - "xtal_snapshots_omega_start": [ - "c", - "b", - "a" - ], - "xtal_snapshots_omega_end": [ - "f", - "e", - "d" - ] - } + "visit": "cm31105-4" } \ No newline at end of file diff --git a/tests/test_data/parameter_json_files/good_test_parameters.json b/tests/test_data/parameter_json_files/good_test_parameters.json index 1920c6042..cdb3cf4bd 100644 --- a/tests/test_data/parameter_json_files/good_test_parameters.json +++ b/tests/test_data/parameter_json_files/good_test_parameters.json @@ -25,22 +25,5 @@ "y2_start_um": 0.0, "z_start_um": 0.0, "z2_start_um": 0.0, - "storage_directory": "/tmp/dls/i03/data/2024/cm31105-4/xraycentring/123456/", - "ispyb_extras": { - "xtal_snapshots_omega_start": [ - "test_1_y", - "test_2_y", - "test_3_y" - ], - "xtal_snapshots_omega_end": [ - "test_1_z", - "test_2_z", - "test_3_z" - ], - "xtal_snapshots": [ - "test_1", - "test_2", - "test_3" - ] - } + "storage_directory": "/tmp/dls/i03/data/2024/cm31105-4/xraycentring/123456/" } \ No newline at end of file diff --git a/tests/test_data/parameter_json_files/good_test_pin_centre_then_xray_centre_parameters.json b/tests/test_data/parameter_json_files/good_test_pin_centre_then_xray_centre_parameters.json index e0c67f70d..20b4080e5 100644 --- a/tests/test_data/parameter_json_files/good_test_pin_centre_then_xray_centre_parameters.json +++ b/tests/test_data/parameter_json_files/good_test_pin_centre_then_xray_centre_parameters.json @@ -16,7 +16,5 @@ "grid_width_um": 290.6, "oav_centring_file": "tests/test_data/test_OAVCentring.json", "transmission_frac": 1.0, - "visit": "cm31105-4", - "ispyb_extras": { - } + "visit": "cm31105-4" } \ No newline at end of file diff --git a/tests/test_data/parameter_json_files/good_test_robot_load_params.json b/tests/test_data/parameter_json_files/good_test_robot_load_params.json index 93e121f2f..afaeaaee6 100644 --- a/tests/test_data/parameter_json_files/good_test_robot_load_params.json +++ b/tests/test_data/parameter_json_files/good_test_robot_load_params.json @@ -17,7 +17,5 @@ "sample_id": 12345, "sample_puck": 40, "sample_pin": 3, - "comment": "Descriptive comment.", - "ispyb_extras": { - } + "comment": "Descriptive comment." } \ No newline at end of file diff --git a/tests/test_data/parameter_json_files/good_test_rotation_scan_parameters.json b/tests/test_data/parameter_json_files/good_test_rotation_scan_parameters.json index 5dbe14c28..67d1193af 100644 --- a/tests/test_data/parameter_json_files/good_test_rotation_scan_parameters.json +++ b/tests/test_data/parameter_json_files/good_test_rotation_scan_parameters.json @@ -30,16 +30,6 @@ "test_1_y", "test_2_y", "test_3_y" - ], - "xtal_snapshots_omega_end": [ - "test_1_z", - "test_2_z", - "test_3_z" - ], - "xtal_snapshots": [ - "test_1", - "test_2", - "test_3" ] } } \ No newline at end of file diff --git a/tests/test_data/parameter_json_files/good_test_rotation_scan_parameters_nomove.json b/tests/test_data/parameter_json_files/good_test_rotation_scan_parameters_nomove.json index 5b762988c..8f9983f7c 100644 --- a/tests/test_data/parameter_json_files/good_test_rotation_scan_parameters_nomove.json +++ b/tests/test_data/parameter_json_files/good_test_rotation_scan_parameters_nomove.json @@ -25,16 +25,6 @@ "test_1_y", "test_2_y", "test_3_y" - ], - "xtal_snapshots_omega_end": [ - "test_1_z", - "test_2_z", - "test_3_z" - ], - "xtal_snapshots": [ - "test_1", - "test_2", - "test_3" ] } } \ No newline at end of file diff --git a/tests/test_data/parameter_json_files/ispyb_gridscan_system_test_parameters.json b/tests/test_data/parameter_json_files/ispyb_gridscan_system_test_parameters.json index c93653c42..1c3424890 100644 --- a/tests/test_data/parameter_json_files/ispyb_gridscan_system_test_parameters.json +++ b/tests/test_data/parameter_json_files/ispyb_gridscan_system_test_parameters.json @@ -16,17 +16,5 @@ "oav_centring_file": "tests/test_data/test_OAVCentring.json", "transmission_frac": 0.49118, "visit": "cm31105-4", - "demand_energy_ev": 12700, - "ispyb_extras": { - "xtal_snapshots_omega_start": [ - "test_1_y", - "test_2_y", - "test_3_y" - ], - "xtal_snapshots_omega_end": [ - "test_1_z", - "test_2_z", - "test_3_z" - ] - } + "demand_energy_ev": 12700 } \ No newline at end of file diff --git a/tests/test_data/parameter_json_files/test_gridscan_param_defaults.json b/tests/test_data/parameter_json_files/test_gridscan_param_defaults.json index cf5ca9e83..23ec0d023 100644 --- a/tests/test_data/parameter_json_files/test_gridscan_param_defaults.json +++ b/tests/test_data/parameter_json_files/test_gridscan_param_defaults.json @@ -27,17 +27,5 @@ "detector_distance_mm": 100.0, "omega_start_deg": 0.0, "exposure_time_s": 0.1, - "set_stub_offsets": true, - "ispyb_extras": { - "xtal_snapshots_omega_start": [ - "test_1_y", - "test_2_y", - "test_3_y" - ], - "xtal_snapshots_omega_end": [ - "test_1_z", - "test_2_z", - "test_3_z" - ] - } + "set_stub_offsets": true } \ No newline at end of file diff --git a/tests/test_data/parameter_json_files/test_oav_snapshot_params.json b/tests/test_data/parameter_json_files/test_oav_snapshot_params.json new file mode 100644 index 000000000..ad653819c --- /dev/null +++ b/tests/test_data/parameter_json_files/test_oav_snapshot_params.json @@ -0,0 +1,5 @@ +{ + "parameter_model_version": "5.0.0", + "snapshot_directory": "/tmp/my_snapshots", + "snapshot_omegas_deg": [0, 90, 180, 270] +} \ No newline at end of file diff --git a/tests/unit_tests/experiment_plans/test_grid_detect_then_xray_centre_plan.py b/tests/unit_tests/experiment_plans/test_grid_detect_then_xray_centre_plan.py index ed4837ec8..fa9a3ee18 100644 --- a/tests/unit_tests/experiment_plans/test_grid_detect_then_xray_centre_plan.py +++ b/tests/unit_tests/experiment_plans/test_grid_detect_then_xray_centre_plan.py @@ -34,7 +34,7 @@ def _fake_grid_detection( # first grid detection: x * y oav.grid_snapshot.num_boxes_x.put(10) oav.grid_snapshot.num_boxes_y.put(4) - yield from bps.create(CONST.DESCRIPTORS.OAV_SNAPSHOT_TRIGGERED) + yield from bps.create(CONST.DESCRIPTORS.OAV_GRID_SNAPSHOT_TRIGGERED) yield from bps.read(oav.grid_snapshot) yield from bps.read(devices.smargon) yield from bps.save() @@ -42,7 +42,7 @@ def _fake_grid_detection( # second grid detection: x * z, so num_boxes_y refers to smargon z oav.grid_snapshot.num_boxes_x.put(10) oav.grid_snapshot.num_boxes_y.put(1) - yield from bps.create(CONST.DESCRIPTORS.OAV_SNAPSHOT_TRIGGERED) + yield from bps.create(CONST.DESCRIPTORS.OAV_GRID_SNAPSHOT_TRIGGERED) yield from bps.read(oav.grid_snapshot) yield from bps.read(devices.smargon) yield from bps.save() diff --git a/tests/unit_tests/experiment_plans/test_oav_snapshot_plan.py b/tests/unit_tests/experiment_plans/test_oav_snapshot_plan.py new file mode 100644 index 000000000..407b6af5b --- /dev/null +++ b/tests/unit_tests/experiment_plans/test_oav_snapshot_plan.py @@ -0,0 +1,124 @@ +from datetime import datetime +from unittest.mock import patch + +import pytest +from dodal.devices.oav.oav_parameters import OAVParameters +from dodal.devices.oav.utils import ColorMode + +from hyperion.experiment_plans.oav_snapshot_plan import ( + OAV_SNAPSHOT_GROUP, + OavSnapshotComposite, + oav_snapshot_plan, +) +from hyperion.parameters.components import WithSnapshot +from hyperion.parameters.constants import DocDescriptorNames + +from ...conftest import raw_params_from_file + + +@pytest.fixture +def oav_snapshot_params(): + return WithSnapshot( + **raw_params_from_file( + "tests/test_data/parameter_json_files/test_oav_snapshot_params.json" + ) + ) + + +@pytest.fixture +def oav_snapshot_composite(smargon, oav): + oav.zoom_controller.fvst.sim_put("5.0x") + return OavSnapshotComposite(smargon=smargon, oav=oav) + + +@patch("hyperion.experiment_plans.oav_snapshot_plan.datetime", spec=datetime) +def test_oav_snapshot_plan_issues_rotations_and_generates_events( + mock_datetime, oav_snapshot_params, oav_snapshot_composite, sim_run_engine +): + mock_datetime.now.return_value = datetime.fromisoformat("2024-06-07T10:06:23") + msgs = sim_run_engine.simulate_plan( + oav_snapshot_plan( + oav_snapshot_composite, + oav_snapshot_params, + OAVParameters(oav_config_json="tests/test_data/test_OAVCentring.json"), + ) + ) + + msgs = sim_run_engine.assert_message_and_return_remaining( + msgs, + lambda msg: msg.command == "set" + and msg.obj.name == "oav_cam_color_mode" + and msg.args[0] == ColorMode.RGB1, + ) + msgs = sim_run_engine.assert_message_and_return_remaining( + msgs, + lambda msg: msg.command == "set" + and msg.obj.name == "oav_cam_acquire_period" + and msg.args[0] == 0.05, + ) + msgs = sim_run_engine.assert_message_and_return_remaining( + msgs, + lambda msg: msg.command == "set" + and msg.obj.name == "oav_cam_acquire_time" + and msg.args[0] == 0.075, + ) + msgs = sim_run_engine.assert_message_and_return_remaining( + msgs, + lambda msg: msg.command == "set" + and msg.obj.name == "oav_cam_gain" + and msg.args[0] == 1, + ) + msgs = sim_run_engine.assert_message_and_return_remaining( + msgs, + lambda msg: msg.command == "set" + and msg.obj.name == "oav_zoom_controller" + and msg.args[0] == "5.0x", + ) + msgs = sim_run_engine.assert_message_and_return_remaining( + msgs, + lambda msg: msg.command == "set" + and msg.obj.name == "oav_snapshot_directory" + and msg.args[0] == "/tmp/my_snapshots", + ) + for expected in [ + {"omega": 0, "filename": "100623_oav_snapshot_0"}, + {"omega": 90, "filename": "100623_oav_snapshot_90"}, + {"omega": 180, "filename": "100623_oav_snapshot_180"}, + {"omega": 270, "filename": "100623_oav_snapshot_270"}, + ]: + msgs = sim_run_engine.assert_message_and_return_remaining( + msgs, + lambda msg: msg.command == "set" + and msg.obj.name == "smargon_omega" + and msg.args[0] == expected["omega"] + and msg.kwargs["group"] == OAV_SNAPSHOT_GROUP, + ) + msgs = sim_run_engine.assert_message_and_return_remaining( + msgs, + lambda msg: msg.command == "set" + and msg.obj.name == "oav_snapshot_filename" + and msg.args[0] == expected["filename"], + ) + msgs = sim_run_engine.assert_message_and_return_remaining( + msgs, + lambda msg: msg.command == "trigger" + and msg.obj.name == "oav_snapshot" + and msg.kwargs["group"] == OAV_SNAPSHOT_GROUP, + ) + msgs = sim_run_engine.assert_message_and_return_remaining( + msgs, + lambda msg: msg.command == "wait" + and msg.kwargs["group"] == OAV_SNAPSHOT_GROUP, + ) + msgs = sim_run_engine.assert_message_and_return_remaining( + msgs, + lambda msg: msg.command == "create" + and msg.kwargs["name"] + == DocDescriptorNames.OAV_ROTATION_SNAPSHOT_TRIGGERED, + ) + msgs = sim_run_engine.assert_message_and_return_remaining( + msgs, lambda msg: msg.command == "read" and msg.obj.name == "oav_snapshot" + ) + msgs = sim_run_engine.assert_message_and_return_remaining( + msgs, lambda msg: msg.command == "save" + ) diff --git a/tests/unit_tests/external_interaction/callbacks/conftest.py b/tests/unit_tests/external_interaction/callbacks/conftest.py index 4a648044a..c797f67bd 100644 --- a/tests/unit_tests/external_interaction/callbacks/conftest.py +++ b/tests/unit_tests/external_interaction/callbacks/conftest.py @@ -126,7 +126,12 @@ class TestData: test_descriptor_document_oav_snapshot: EventDescriptor = { "uid": "b5ba4aec-de49-4970-81a4-b4a847391d34", "run_start": "d8bee3ee-f614-4e7a-a516-25d6b9e87ef3", - "name": CONST.DESCRIPTORS.OAV_SNAPSHOT_TRIGGERED, + "name": CONST.DESCRIPTORS.OAV_GRID_SNAPSHOT_TRIGGERED, + } # type: ignore + test_descriptor_document_oav_rotation_snapshot: EventDescriptor = { + "uid": "c7d698ce-6d49-4c56-967e-7d081f964573", + "run_start": "d8bee3ee-f614-4e7a-a516-25d6b9e87ef3", + "name": CONST.DESCRIPTORS.OAV_ROTATION_SNAPSHOT_TRIGGERED, } # type: ignore test_descriptor_document_pre_data_collection: EventDescriptor = { "uid": "bd45c2e5-2b85-4280-95d7-a9a15800a78b", @@ -148,6 +153,14 @@ class TestData: "run_start": "d8bee3ee-f614-4e7a-a516-25d6b9e87ef3", "name": CONST.DESCRIPTORS.NEXUS_READ, } # type: ignore + test_event_document_oav_rotation_snapshot: Event = { + "descriptor": "c7d698ce-6d49-4c56-967e-7d081f964573", + "time": 1666604299.828203, + "timestamps": {}, + "seq_num": 1, + "uid": "32d7c25c-c310-4292-ac78-36ce6509be3d", + "data": {"oav_snapshot_last_saved_path": "snapshot_0"}, + } test_event_document_oav_snapshot_xy: Event = { "descriptor": "b5ba4aec-de49-4970-81a4-b4a847391d34", "time": 1666604299.828203, 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 cf26cc9c8..9a47f2c3c 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 @@ -1,5 +1,7 @@ from unittest.mock import MagicMock, patch +import pytest + from hyperion.external_interaction.callbacks.rotation.ispyb_callback import ( RotationISPyBCallback, ) @@ -37,9 +39,6 @@ "start_image_number": 1, "xbeam": 150.0, "ybeam": 160.0, - "xtal_snapshot1": "test_1_y", - "xtal_snapshot2": "test_2_y", - "xtal_snapshot3": "test_3_y", "synchrotron_mode": None, "starttime": EXPECTED_START_TIME, "filetemplate": "file_name_1_master.h5", @@ -48,14 +47,27 @@ } +@pytest.fixture +def rotation_start_outer_doc_without_snapshots( + test_rotation_start_outer_document, dummy_rotation_params +): + dummy_rotation_params.ispyb_extras.xtal_snapshots_omega_start = None + test_rotation_start_outer_document["hyperion_parameters"] = ( + dummy_rotation_params.json() + ) + return test_rotation_start_outer_document + + @patch( "hyperion.external_interaction.callbacks.common.ispyb_mapping.get_current_time_string", new=MagicMock(return_value=EXPECTED_START_TIME), ) -def test_activity_gated_start(mock_ispyb_conn, test_rotation_start_outer_document): +def test_activity_gated_start( + mock_ispyb_conn, rotation_start_outer_doc_without_snapshots +): callback = RotationISPyBCallback() - callback.activity_gated_start(test_rotation_start_outer_document) + callback.activity_gated_start(rotation_start_outer_doc_without_snapshots) mx = mx_acquisition_from_conn(mock_ispyb_conn) assert_upsert_call_with( mx.upsert_data_collection_group.mock_calls[0], @@ -73,6 +85,38 @@ def test_activity_gated_start(mock_ispyb_conn, test_rotation_start_outer_documen ) +@patch( + "hyperion.external_interaction.callbacks.common.ispyb_mapping.get_current_time_string", + new=MagicMock(return_value=EXPECTED_START_TIME), +) +def test_activity_gated_start_with_snapshot_parameters( + mock_ispyb_conn, test_rotation_start_outer_document +): + callback = RotationISPyBCallback() + + callback.activity_gated_start(test_rotation_start_outer_document) + mx = mx_acquisition_from_conn(mock_ispyb_conn) + assert_upsert_call_with( + mx.upsert_data_collection_group.mock_calls[0], + mx.get_data_collection_group_params(), + { + "parentid": TEST_SESSION_ID, + "experimenttype": "SAD", + "sampleid": TEST_SAMPLE_ID, + }, + ) + assert_upsert_call_with( + mx.upsert_data_collection.mock_calls[0], + mx.get_data_collection_params(), + EXPECTED_DATA_COLLECTION + | { + "xtal_snapshot1": "test_1_y", + "xtal_snapshot2": "test_2_y", + "xtal_snapshot3": "test_3_y", + }, + ) + + @patch( "hyperion.external_interaction.callbacks.common.ispyb_mapping.get_current_time_string", new=MagicMock(return_value=EXPECTED_START_TIME), @@ -165,6 +209,51 @@ def test_flux_read_events( ) +@patch( + "hyperion.external_interaction.callbacks.common.ispyb_mapping.get_current_time_string", + new=MagicMock(return_value=EXPECTED_START_TIME), +) +def test_oav_rotation_snapshot_triggered_event( + mock_ispyb_conn, dummy_rotation_params, rotation_start_outer_doc_without_snapshots +): + callback = RotationISPyBCallback() + callback.activity_gated_start( + rotation_start_outer_doc_without_snapshots + ) # pyright: ignore + callback.activity_gated_start( + TestData.test_rotation_start_main_document # pyright: ignore + ) + mx = mx_acquisition_from_conn(mock_ispyb_conn) + callback.activity_gated_descriptor( + TestData.test_descriptor_document_oav_rotation_snapshot + ) + + for snapshot in [ + {"filename": "snapshot_0", "colname": "xtal_snapshot1"}, + {"filename": "snapshot_90", "colname": "xtal_snapshot2"}, + {"filename": "snapshot_180", "colname": "xtal_snapshot3"}, + {"filename": "snapshot_270", "colname": "xtal_snapshot4"}, + ]: + mx.upsert_data_collection.reset_mock() + event_doc = dict(TestData.test_event_document_oav_rotation_snapshot) + event_doc["data"]["oav_snapshot_last_saved_path"] = snapshot["filename"] # type: ignore + callback.activity_gated_event( + TestData.test_event_document_oav_rotation_snapshot + ) + mx.upsert_data_collection_group.reset_mock() + assert_upsert_call_with( + mx.upsert_data_collection.mock_calls[0], + mx.get_data_collection_params(), + { + "parentid": TEST_DATA_COLLECTION_GROUP_ID, + "id": TEST_DATA_COLLECTION_IDS[0], + snapshot["colname"]: snapshot["filename"], + }, + ) + + mx.upsert_data_collection_group.assert_not_called() + + @patch( "hyperion.external_interaction.callbacks.common.ispyb_mapping.get_current_time_string", new=MagicMock(return_value=EXPECTED_START_TIME), diff --git a/tests/unit_tests/parameters/test_parameter_model.py b/tests/unit_tests/parameters/test_parameter_model.py index 79d6a9a82..488aff21b 100644 --- a/tests/unit_tests/parameters/test_parameter_model.py +++ b/tests/unit_tests/parameters/test_parameter_model.py @@ -27,7 +27,6 @@ def minimal_3d_gridscan_params(): "y_steps": 7, "z_steps": 9, "storage_directory": "/tmp/dls/i03/data/2024/cm31105-4/xraycentring/123456/", - "ispyb_extras": {}, } @@ -77,7 +76,6 @@ def test_robot_load_then_centre_params(): "visit": "cm12345", "file_name": "file_name", "storage_directory": "/tmp/dls/i03/data/2024/cm31105-4/xraycentring/123456/", - "ispyb_extras": {}, } params["detector_distance_mm"] = 200 test_params = RobotLoadThenCentre(**params)