Merge pull request #1183 from DiamondLightSource/1182_rotation_scans_…

…should_put_correct_experiment_kind_into_ispyb (#1182) Allow the ispyb experiment_type field to be populated by GDA
DiamondLightSource · Feb 27, 2024 · abb5814 · abb5814
2 parents d4c3d19 + 0da7dba
commit abb5814
Show file tree

Hide file tree

Showing 22 changed files with 96 additions and 18 deletions.
diff --git a/setup.cfg b/setup.cfg
@@ -35,7 +35,7 @@ install_requires =
     xarray
     doct
     databroker
-    dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@8d25746e3407c8331357e8ce159e89c5d3bfee92
+    dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@00ef53ddf55c53fd000990c25ad6093e88f39fe4
     pydantic<2.0 # See https://github.com/DiamondLightSource/hyperion/issues/774
     scipy
     pyzmq<25 # See https://github.com/DiamondLightSource/hyperion/issues/1103

diff --git a/src/hyperion/external_interaction/callbacks/rotation/ispyb_callback.py b/src/hyperion/external_interaction/callbacks/rotation/ispyb_callback.py
@@ -74,7 +74,18 @@ def activity_gated_start(self, doc: RunStart):
                     f"Collection has {n_images} images - treating as a genuine dataset - storing sampleID to bundle images"
                 )
                 self.last_sample_id = self.params.hyperion_params.ispyb_params.sample_id
-            self.ispyb = StoreRotationInIspyb(self.ispyb_config, self.params, dcgid)
+            experiment_type = (
+                self.params.hyperion_params.ispyb_params.ispyb_experiment_type
+            )
+            if experiment_type:
+                self.ispyb = StoreRotationInIspyb(
+                    self.ispyb_config,
+                    self.params,
+                    dcgid,
+                    experiment_type,
+                )
+            else:
+                self.ispyb = StoreRotationInIspyb(self.ispyb_config, self.params, dcgid)
             ISPYB_LOGGER.info("Beginning ispyb deposition")
             self.ispyb_ids = self.ispyb.begin_deposition()
         ISPYB_LOGGER.info("ISPYB handler received start document.")

diff --git a/src/hyperion/external_interaction/ispyb/ispyb_dataclass.py b/src/hyperion/external_interaction/ispyb/ispyb_dataclass.py
@@ -63,6 +63,8 @@ class IspybParams(BaseModel):
     xtal_snapshots_omega_start: Optional[list[str]] = None
     xtal_snapshots_omega_end: Optional[list[str]] = None
 
+    ispyb_experiment_type: Optional[str] = None
+
     class Config:
         arbitrary_types_allowed = True
         json_encoders = {np.ndarray: lambda a: a.tolist()}

diff --git a/src/hyperion/external_interaction/ispyb/rotation_ispyb_store.py b/src/hyperion/external_interaction/ispyb/rotation_ispyb_store.py
@@ -22,8 +22,9 @@ def __init__(
         ispyb_config,
         parameters: RotationInternalParameters,
         datacollection_group_id: int | None = None,
+        experiment_type: str = "SAD",
     ) -> None:
-        super().__init__(ispyb_config, "SAD")
+        super().__init__(ispyb_config, experiment_type)
         self.full_params: RotationInternalParameters = parameters
         self._ispyb_params: RotationIspybParams = (  # pyright: ignore
             parameters.hyperion_params.ispyb_params

diff --git a/src/hyperion/parameters/schemas/full_external_parameters_schema.json b/src/hyperion/parameters/schemas/full_external_parameters_schema.json
@@ -3,7 +3,7 @@
     "type": "object",
     "properties": {
         "params_version": {
-            "const": "4.0.3"
+            "const": "4.0.4"
         },
         "hyperion_params": {
             "type": "object",

diff --git a/src/hyperion/parameters/schemas/ispyb_parameters_schema.json b/src/hyperion/parameters/schemas/ispyb_parameters_schema.json
@@ -74,6 +74,9 @@
         },
         "resolution": {
             "type": "number"
+        },
+        "ispyb_experiment_type": {
+            "type": ["string", "null"]
         }
     },
     "required": [

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
@@ -1,5 +1,5 @@
 {
-    "params_version": "4.0.3",
+    "params_version": "4.0.4",
     "hyperion_params": {
         "beamline": "BL03S",
         "insertion_prefix": "SR03S",

diff --git a/tests/test_data/parameter_json_files/good_test_parameters.json b/tests/test_data/parameter_json_files/good_test_parameters.json
@@ -1,5 +1,5 @@
 {
-    "params_version": "4.0.3",
+    "params_version": "4.0.4",
     "hyperion_params": {
         "beamline": "BL03S",
         "insertion_prefix": "SR03S",

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
@@ -1,5 +1,5 @@
 {
-    "params_version": "4.0.3",
+    "params_version": "4.0.4",
     "hyperion_params": {
         "beamline": "BL03S",
         "insertion_prefix": "SR03S",

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
@@ -1,5 +1,5 @@
 {
-    "params_version": "4.0.3",
+    "params_version": "4.0.4",
     "hyperion_params": {
         "beamline": "BL03S",
         "insertion_prefix": "SR03S",

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
@@ -1,5 +1,5 @@
 {
-    "params_version": "4.0.3",
+    "params_version": "4.0.4",
     "hyperion_params": {
         "beamline": "BL03S",
         "insertion_prefix": "SR03S",

diff --git a/tests/test_data/parameter_json_files/good_test_stepped_grid_scan_parameters.json b/tests/test_data/parameter_json_files/good_test_stepped_grid_scan_parameters.json
@@ -1,5 +1,5 @@
 {
-    "params_version": "4.0.3",
+    "params_version": "4.0.4",
     "hyperion_params": {
         "beamline": "BL03S",
         "insertion_prefix": "SR03S",

diff --git a/tests/test_data/parameter_json_files/good_test_wait_for_robot_load_params.json b/tests/test_data/parameter_json_files/good_test_wait_for_robot_load_params.json
@@ -1,5 +1,5 @@
 {
-    "params_version": "4.0.3",
+    "params_version": "4.0.4",
     "hyperion_params": {
         "zocalo_environment": "artemis",
         "beamline": "BL03I",

diff --git a/tests/test_data/parameter_json_files/good_test_wait_for_robot_load_params_no_energy.json b/tests/test_data/parameter_json_files/good_test_wait_for_robot_load_params_no_energy.json
@@ -1,5 +1,5 @@
 {
-    "params_version": "4.0.3",
+    "params_version": "4.0.4",
     "hyperion_params": {
         "zocalo_environment": "artemis",
         "beamline": "BL03I",

diff --git a/tests/test_data/parameter_json_files/live_test_rotation_params.json b/tests/test_data/parameter_json_files/live_test_rotation_params.json
@@ -1,5 +1,5 @@
 {
-    "params_version": "4.0.3",
+    "params_version": "4.0.4",
     "hyperion_params": {
         "beamline": "BL03I",
         "insertion_prefix": "SR03I",

diff --git a/tests/test_data/parameter_json_files/live_test_rotation_params_move_xyz.json b/tests/test_data/parameter_json_files/live_test_rotation_params_move_xyz.json
@@ -1,5 +1,5 @@
 {
-    "params_version": "4.0.3",
+    "params_version": "4.0.4",
     "hyperion_params": {
         "beamline": "BL03I",
         "insertion_prefix": "SR03I",

diff --git a/tests/test_data/parameter_json_files/panda_test_parameters.json b/tests/test_data/parameter_json_files/panda_test_parameters.json
@@ -1,5 +1,5 @@
 {
-    "params_version": "4.0.3",
+    "params_version": "4.0.4",
     "hyperion_params": {
         "beamline": "BL03I",
         "insertion_prefix": "SR03I",

diff --git a/tests/test_data/parameter_json_files/test_internal_parameter_defaults.json b/tests/test_data/parameter_json_files/test_internal_parameter_defaults.json
@@ -1,5 +1,5 @@
 {
-    "params_version": "4.0.3",
+    "params_version": "4.0.4",
     "hyperion_params": {
         "zocalo_environment": "dev_artemis",
         "beamline": "BL03S",

diff --git a/tests/test_data/parameter_json_files/test_parameter_defaults.json b/tests/test_data/parameter_json_files/test_parameter_defaults.json
@@ -1,5 +1,5 @@
 {
-    "params_version": "4.0.3",
+    "params_version": "4.0.4",
     "hyperion_params": {
         "zocalo_environment": "dev_artemis",
         "beamline": "BL03S",

diff --git a/tests/test_data/parameter_json_files/test_parameters.json b/tests/test_data/parameter_json_files/test_parameters.json
@@ -1,5 +1,5 @@
 {
-    "params_version": "4.0.3",
+    "params_version": "4.0.4",
     "hyperion_params": {
         "beamline": "BL03S",
         "insertion_prefix": "SR03S",

diff --git a/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py b/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py
@@ -370,6 +370,9 @@ def test_ispyb_reuses_dcgid_on_same_sampleID(
 ):
     cb = [RotationISPyBCallback()]
     cb[0].active = True
+    ispyb_ids = IspybIds(
+        data_collection_group_id=23, data_collection_ids=45, grid_ids=None
+    )
     rotation_ispyb.return_value.begin_deposition.return_value = ispyb_ids
 
     test_cases = zip(
@@ -399,6 +402,30 @@ def after_main_do(callbacks: list[RotationISPyBCallback]):
         last_dcgid = cb[0].ispyb_ids.data_collection_group_id
 
 
+@patch(
+    "hyperion.external_interaction.callbacks.rotation.ispyb_callback.StoreRotationInIspyb",
+    autospec=True,
+)
+def test_ispyb_specifies_experiment_type_if_supplied(
+    rotation_ispyb: MagicMock,
+    RE: RunEngine,
+    params: RotationInternalParameters,
+):
+    cb = [RotationISPyBCallback()]
+    cb[0].active = True
+    params.hyperion_params.ispyb_params.ispyb_experiment_type = "Characterization"
+    rotation_ispyb.return_value.begin_deposition.return_value = IspybIds(
+        data_collection_group_id=23, data_collection_ids=45, grid_ids=None
+    )
+
+    params.hyperion_params.ispyb_params.sample_id = "abc"
+
+    RE(fake_rotation_scan(params, cb))
+
+    assert rotation_ispyb.call_args.args[3] == "Characterization"
+    assert rotation_ispyb.call_args.args[2] is None
+
+
 n_images_store_id = [
     (123, False),
     (3600, True),

diff --git a/tests/unit_tests/external_interaction/ispyb/test_rotation_ispyb_store.py b/tests/unit_tests/external_interaction/ispyb/test_rotation_ispyb_store.py
@@ -30,6 +30,14 @@
 EXPECTED_END_TIME = "2024-02-08 14:04:01"
 
 
+@pytest.fixture
+def dummy_rotation_ispyb_with_experiment_type(dummy_rotation_params):
+    store_in_ispyb = StoreRotationInIspyb(
+        SIM_ISPYB_CONFIG, dummy_rotation_params, None, "Characterization"
+    )
+    return store_in_ispyb
+
+
 @patch(
     "hyperion.external_interaction.ispyb.ispyb_store.get_current_time_string",
     new=MagicMock(return_value=EXPECTED_START_TIME),
@@ -100,6 +108,32 @@ def test_begin_deposition(
     )
 
 
+@patch(
+    "hyperion.external_interaction.ispyb.ispyb_store.get_current_time_string",
+    new=MagicMock(return_value=EXPECTED_START_TIME),
+)
+def test_begin_deposition_with_alternate_experiment_type(
+    ispyb_conn_with_2x2_collections_and_grid_info,
+    dummy_rotation_ispyb_with_experiment_type,
+    dummy_rotation_params,
+):
+    assert dummy_rotation_ispyb_with_experiment_type.begin_deposition() == IspybIds(
+        data_collection_ids=TEST_DATA_COLLECTION_IDS[0],
+        data_collection_group_id=TEST_DATA_COLLECTION_GROUP_ID,
+    )
+    mx_acq = mx_acquisition_from_conn(ispyb_conn_with_2x2_collections_and_grid_info)
+    assert_upsert_call_with(
+        mx_acq.upsert_data_collection_group.mock_calls[0],
+        mx_acq.get_data_collection_group_params(),
+        {
+            "parentid": TEST_SESSION_ID,
+            "experimenttype": "Characterization",
+            "sampleid": TEST_SAMPLE_ID,
+            "sample_barcode": TEST_BARCODE,  # deferred
+        },
+    )
+
+
 @patch(
     "hyperion.external_interaction.ispyb.ispyb_store.get_current_time_string",
     new=MagicMock(return_value=EXPECTED_START_TIME),