From a17a6921c6f1c7a67ba708974e847bf4d70bd5eb Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Mon, 31 Jul 2023 20:23:03 +0100 Subject: [PATCH 1/2] (#835) Fix transmission units * Rename to be more explicit what units are expected externally * Convert to percentage before sending to ispyb --- src/artemis/experiment_plans/full_grid_scan.py | 2 +- .../experiment_plans/tests/test_fast_grid_scan_plan.py | 5 ++++- .../experiment_plans/tests/test_full_grid_scan_plan.py | 2 +- .../external_interaction/callbacks/ispyb_callback_base.py | 2 +- src/artemis/external_interaction/ispyb/ispyb_dataclass.py | 4 ++-- src/artemis/external_interaction/ispyb/store_in_ispyb.py | 3 ++- src/artemis/external_interaction/nexus/nexus_utils.py | 2 +- .../system_tests/test_write_rotation_nexus.py | 2 +- src/artemis/external_interaction/unit_tests/conftest.py | 4 ++-- .../external_interaction/unit_tests/test_store_in_ispyb.py | 2 +- .../parameters/schemas/full_external_parameters_schema.json | 2 +- src/artemis/parameters/schemas/ispyb_parameters_schema.json | 4 ++-- .../parameters/tests/test_data/artemis_parameters.json | 2 +- .../tests/test_data/bad_test_parameters_wrong_version.json | 2 +- .../good_test_grid_with_edge_detect_parameters.json | 4 ++-- .../parameters/tests/test_data/good_test_parameters.json | 4 ++-- .../tests/test_data/good_test_rotation_scan_parameters.json | 4 ++-- test_parameter_defaults.json | 4 ++-- test_parameters.json | 4 ++-- 19 files changed, 31 insertions(+), 27 deletions(-) diff --git a/src/artemis/experiment_plans/full_grid_scan.py b/src/artemis/experiment_plans/full_grid_scan.py index 86436e1c6..426b502ac 100644 --- a/src/artemis/experiment_plans/full_grid_scan.py +++ b/src/artemis/experiment_plans/full_grid_scan.py @@ -92,7 +92,7 @@ def start_arming_then_do_grid( yield from bps.abs_set(eiger.do_arm, 1, group="ready_for_data_collection") yield from bps.abs_set( attenuator, - parameters.artemis_params.ispyb_params.transmission, + parameters.artemis_params.ispyb_params.transmission_fraction, group="ready_for_data_collection", ) diff --git a/src/artemis/experiment_plans/tests/test_fast_grid_scan_plan.py b/src/artemis/experiment_plans/tests/test_fast_grid_scan_plan.py index 278317f0e..c4cc80ebf 100644 --- a/src/artemis/experiment_plans/tests/test_fast_grid_scan_plan.py +++ b/src/artemis/experiment_plans/tests/test_fast_grid_scan_plan.py @@ -116,7 +116,10 @@ def standalone_read_hardware_for_ispyb(und, syn, slits, attn, fl): assert params.artemis_params.ispyb_params.synchrotron_mode == synchrotron_test_value assert params.artemis_params.ispyb_params.slit_gap_size_x == xgap_test_value assert params.artemis_params.ispyb_params.slit_gap_size_y == ygap_test_value - assert params.artemis_params.ispyb_params.transmission == transmission_test_value + assert ( + params.artemis_params.ispyb_params.transmission_fraction + == transmission_test_value + ) assert params.artemis_params.ispyb_params.flux == flux_test_value diff --git a/src/artemis/experiment_plans/tests/test_full_grid_scan_plan.py b/src/artemis/experiment_plans/tests/test_full_grid_scan_plan.py index c53604677..46cbfc684 100644 --- a/src/artemis/experiment_plans/tests/test_full_grid_scan_plan.py +++ b/src/artemis/experiment_plans/tests/test_full_grid_scan_plan.py @@ -284,5 +284,5 @@ def test_when_start_arming_then_transmission_set( # Check transmission set attenuator.set.assert_called_once_with( - test_full_grid_scan_params.artemis_params.ispyb_params.transmission + test_full_grid_scan_params.artemis_params.ispyb_params.transmission_fraction ) diff --git a/src/artemis/external_interaction/callbacks/ispyb_callback_base.py b/src/artemis/external_interaction/callbacks/ispyb_callback_base.py index 834b7a053..b4c1740ff 100644 --- a/src/artemis/external_interaction/callbacks/ispyb_callback_base.py +++ b/src/artemis/external_interaction/callbacks/ispyb_callback_base.py @@ -63,7 +63,7 @@ def event(self, doc: dict): self.params.artemis_params.ispyb_params.slit_gap_size_y = doc["data"][ "s4_slit_gaps_ygap" ] - self.params.artemis_params.ispyb_params.transmission = doc["data"][ + self.params.artemis_params.ispyb_params.transmission_fraction = doc["data"][ "attenuator_actual_transmission" ] diff --git a/src/artemis/external_interaction/ispyb/ispyb_dataclass.py b/src/artemis/external_interaction/ispyb/ispyb_dataclass.py index 2fb8faded..e408c8aee 100644 --- a/src/artemis/external_interaction/ispyb/ispyb_dataclass.py +++ b/src/artemis/external_interaction/ispyb/ispyb_dataclass.py @@ -15,7 +15,7 @@ "position": [0, 0, 0], "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"], - "transmission": 1.0, + "transmission_fraction": 1.0, "flux": 10.0, "wavelength": 0.01, "beam_size_x": 0.1, @@ -55,7 +55,7 @@ def _parse_position( return position return np.array(position) - transmission: float + transmission_fraction: float wavelength: float beam_size_x: float beam_size_y: float diff --git a/src/artemis/external_interaction/ispyb/store_in_ispyb.py b/src/artemis/external_interaction/ispyb/store_in_ispyb.py index c6bf11dfe..37daa0cb5 100755 --- a/src/artemis/external_interaction/ispyb/store_in_ispyb.py +++ b/src/artemis/external_interaction/ispyb/store_in_ispyb.py @@ -204,7 +204,8 @@ def _store_data_collection_table( params["slitgap_horizontal"] = self.ispyb_params.slit_gap_size_x params["beamsize_at_samplex"] = self.ispyb_params.beam_size_x params["beamsize_at_sampley"] = self.ispyb_params.beam_size_y - params["transmission"] = self.ispyb_params.transmission + # Ispyb wants the transmission in a percentage, we use fractions + params["transmission"] = self.ispyb_params.transmission_fraction * 100 params["comments"] = self._construct_comment() params["data_collection_number"] = self.run_number params["detector_distance"] = self.detector_params.detector_distance diff --git a/src/artemis/external_interaction/nexus/nexus_utils.py b/src/artemis/external_interaction/nexus/nexus_utils.py index 77c3f9679..5b19e631e 100644 --- a/src/artemis/external_interaction/nexus/nexus_utils.py +++ b/src/artemis/external_interaction/nexus/nexus_utils.py @@ -111,5 +111,5 @@ def create_beam_and_attenuator_parameters( """ return ( Beam(ispyb_params.wavelength, ispyb_params.flux), - Attenuator(ispyb_params.transmission), + Attenuator(ispyb_params.transmission_fraction), ) diff --git a/src/artemis/external_interaction/system_tests/test_write_rotation_nexus.py b/src/artemis/external_interaction/system_tests/test_write_rotation_nexus.py index 27dc2742a..48e7707c4 100644 --- a/src/artemis/external_interaction/system_tests/test_write_rotation_nexus.py +++ b/src/artemis/external_interaction/system_tests/test_write_rotation_nexus.py @@ -38,7 +38,7 @@ def test_params(): params.experiment_params.z = 0 params.artemis_params.detector_params.exposure_time = 0.004 params.artemis_params.detector_params.current_energy_ev = 12700 - params.artemis_params.ispyb_params.transmission = 0.49118047952 + params.artemis_params.ispyb_params.transmission_fraction = 0.49118047952 params.artemis_params.ispyb_params.wavelength = 0.9762535433 return params diff --git a/src/artemis/external_interaction/unit_tests/conftest.py b/src/artemis/external_interaction/unit_tests/conftest.py index 49b663674..8220ee8f2 100644 --- a/src/artemis/external_interaction/unit_tests/conftest.py +++ b/src/artemis/external_interaction/unit_tests/conftest.py @@ -26,7 +26,7 @@ def test_rotation_params(): params.experiment_params.z = 0 params.artemis_params.detector_params.exposure_time = 0.004 params.artemis_params.detector_params.current_energy_ev = 12700 - params.artemis_params.ispyb_params.transmission = 0.49118047952 + params.artemis_params.ispyb_params.transmission_fraction = 0.49118047952 params.artemis_params.ispyb_params.wavelength = 0.9762535433 return params @@ -36,7 +36,7 @@ def test_fgs_params(request): params = FGSInternalParameters(**default_raw_params()) params.artemis_params.ispyb_params.wavelength = 1.0 params.artemis_params.ispyb_params.flux = 9.0 - params.artemis_params.ispyb_params.transmission = 0.5 + params.artemis_params.ispyb_params.transmission_fraction = 0.5 params.artemis_params.detector_params.use_roi_mode = True params.artemis_params.detector_params.num_triggers = request.param params.artemis_params.detector_params.directory = ( diff --git a/src/artemis/external_interaction/unit_tests/test_store_in_ispyb.py b/src/artemis/external_interaction/unit_tests/test_store_in_ispyb.py index 0f194b597..dbb9bdff9 100644 --- a/src/artemis/external_interaction/unit_tests/test_store_in_ispyb.py +++ b/src/artemis/external_interaction/unit_tests/test_store_in_ispyb.py @@ -60,7 +60,7 @@ "comments": None, "slitgapvertical": None, "slitgaphorizontal": None, - "transmission": None, + "transmission_fraction": None, "synchrotronmode": None, "xtalsnapshot1": None, "xtalsnapshot2": None, diff --git a/src/artemis/parameters/schemas/full_external_parameters_schema.json b/src/artemis/parameters/schemas/full_external_parameters_schema.json index affbce4e5..3ac0e7f4a 100644 --- a/src/artemis/parameters/schemas/full_external_parameters_schema.json +++ b/src/artemis/parameters/schemas/full_external_parameters_schema.json @@ -3,7 +3,7 @@ "type": "object", "properties": { "params_version": { - "const": "2.0.0" + "const": "2.1.0" }, "artemis_params": { "type": "object", diff --git a/src/artemis/parameters/schemas/ispyb_parameters_schema.json b/src/artemis/parameters/schemas/ispyb_parameters_schema.json index f5b2a265d..3a934cbcf 100644 --- a/src/artemis/parameters/schemas/ispyb_parameters_schema.json +++ b/src/artemis/parameters/schemas/ispyb_parameters_schema.json @@ -45,7 +45,7 @@ "type": "string" } }, - "transmission": { + "transmission_fraction": { "type": "number" }, "flux": { @@ -84,7 +84,7 @@ "microns_per_pixel_x", "microns_per_pixel_y", "position", - "transmission", + "transmission_fraction", "flux", "wavelength", "beam_size_x", diff --git a/src/artemis/parameters/tests/test_data/artemis_parameters.json b/src/artemis/parameters/tests/test_data/artemis_parameters.json index 4ba84ae17..f8cf61d89 100644 --- a/src/artemis/parameters/tests/test_data/artemis_parameters.json +++ b/src/artemis/parameters/tests/test_data/artemis_parameters.json @@ -36,7 +36,7 @@ 20.0, 30.0 ], - "transmission": 1.0, + "transmission_fraction": 1.0, "flux": 10.0, "wavelength": 0.01, "beam_size_x": 1.0, diff --git a/src/artemis/parameters/tests/test_data/bad_test_parameters_wrong_version.json b/src/artemis/parameters/tests/test_data/bad_test_parameters_wrong_version.json index ca0f68608..7bb81cc37 100644 --- a/src/artemis/parameters/tests/test_data/bad_test_parameters_wrong_version.json +++ b/src/artemis/parameters/tests/test_data/bad_test_parameters_wrong_version.json @@ -47,7 +47,7 @@ "test_2", "test_3" ], - "transmission": 1.0, + "transmission_fraction": 1.0, "flux": 10.0, "wavelength": 0.01, "beam_size_x": 1.0, diff --git a/src/artemis/parameters/tests/test_data/good_test_grid_with_edge_detect_parameters.json b/src/artemis/parameters/tests/test_data/good_test_grid_with_edge_detect_parameters.json index 649228c19..aa6ad529b 100644 --- a/src/artemis/parameters/tests/test_data/good_test_grid_with_edge_detect_parameters.json +++ b/src/artemis/parameters/tests/test_data/good_test_grid_with_edge_detect_parameters.json @@ -1,5 +1,5 @@ { - "params_version": "2.0.0", + "params_version": "2.1.0", "artemis_params": { "beamline": "BL03S", "insertion_prefix": "SR03S", @@ -23,7 +23,7 @@ 20.0, 30.0 ], - "transmission": 1.0, + "transmission_fraction": 1.0, "flux": 10.0, "wavelength": 0.01, "beam_size_x": 1.0, diff --git a/src/artemis/parameters/tests/test_data/good_test_parameters.json b/src/artemis/parameters/tests/test_data/good_test_parameters.json index 9bf1803c0..d9034ddab 100644 --- a/src/artemis/parameters/tests/test_data/good_test_parameters.json +++ b/src/artemis/parameters/tests/test_data/good_test_parameters.json @@ -1,5 +1,5 @@ { - "params_version": "2.0.0", + "params_version": "2.1.0", "artemis_params": { "beamline": "BL03S", "insertion_prefix": "SR03S", @@ -43,7 +43,7 @@ "test_2", "test_3" ], - "transmission": 1.0, + "transmission_fraction": 1.0, "flux": 10.0, "wavelength": 0.01, "beam_size_x": 1.0, diff --git a/src/artemis/parameters/tests/test_data/good_test_rotation_scan_parameters.json b/src/artemis/parameters/tests/test_data/good_test_rotation_scan_parameters.json index 51323aa04..6a707a416 100644 --- a/src/artemis/parameters/tests/test_data/good_test_rotation_scan_parameters.json +++ b/src/artemis/parameters/tests/test_data/good_test_rotation_scan_parameters.json @@ -1,5 +1,5 @@ { - "params_version": "2.0.0", + "params_version": "2.1.0", "artemis_params": { "beamline": "BL03S", "insertion_prefix": "SR03S", @@ -43,7 +43,7 @@ "test_2", "test_3" ], - "transmission": 1.0, + "transmission_fraction": 1.0, "flux": 10.0, "wavelength": 0.01, "beam_size_x": 1.0, diff --git a/test_parameter_defaults.json b/test_parameter_defaults.json index 479ed7b52..cbde8d004 100644 --- a/test_parameter_defaults.json +++ b/test_parameter_defaults.json @@ -1,5 +1,5 @@ { - "params_version": "2.0.0", + "params_version": "2.1.0", "artemis_params": { "zocalo_environment": "dev_artemis", "beamline": "BL03S", @@ -37,7 +37,7 @@ "test_2_z", "test_3_z" ], - "transmission": 1.0, + "transmission_fraction": 1.0, "flux": 10.0, "wavelength": 0.01, "beam_size_x": 0.1, diff --git a/test_parameters.json b/test_parameters.json index 9bf1803c0..d9034ddab 100644 --- a/test_parameters.json +++ b/test_parameters.json @@ -1,5 +1,5 @@ { - "params_version": "2.0.0", + "params_version": "2.1.0", "artemis_params": { "beamline": "BL03S", "insertion_prefix": "SR03S", @@ -43,7 +43,7 @@ "test_2", "test_3" ], - "transmission": 1.0, + "transmission_fraction": 1.0, "flux": 10.0, "wavelength": 0.01, "beam_size_x": 1.0, From 3c8c602427fa28df9c4c81326e89bf5d67165f54 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Mon, 31 Jul 2023 20:43:33 +0100 Subject: [PATCH 2/2] (#835) Raise exception if too large a transmission given --- .../ispyb/ispyb_dataclass.py | 8 ++++ .../unit_tests/test_ispyb_dataclass.py | 37 +++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 src/artemis/external_interaction/unit_tests/test_ispyb_dataclass.py diff --git a/src/artemis/external_interaction/ispyb/ispyb_dataclass.py b/src/artemis/external_interaction/ispyb/ispyb_dataclass.py index e408c8aee..d6764264c 100644 --- a/src/artemis/external_interaction/ispyb/ispyb_dataclass.py +++ b/src/artemis/external_interaction/ispyb/ispyb_dataclass.py @@ -76,6 +76,14 @@ def _parse_position( xtal_snapshots_omega_start: Optional[List[str]] = None xtal_snapshots_omega_end: Optional[List[str]] = None + @validator("transmission_fraction") + def transmission_not_percentage(cls, transmission_fraction: float): + if transmission_fraction > 1: + raise ValueError( + "Transmission_fraction of >1 given. Did you give a percentage instead of a fraction?" + ) + return transmission_fraction + class RotationIspybParams(IspybParams): ... diff --git a/src/artemis/external_interaction/unit_tests/test_ispyb_dataclass.py b/src/artemis/external_interaction/unit_tests/test_ispyb_dataclass.py new file mode 100644 index 000000000..fb322fb55 --- /dev/null +++ b/src/artemis/external_interaction/unit_tests/test_ispyb_dataclass.py @@ -0,0 +1,37 @@ +from copy import deepcopy + +import numpy as np +import pytest + +from artemis.external_interaction.ispyb.ispyb_dataclass import ( + GRIDSCAN_ISPYB_PARAM_DEFAULTS, + IspybParams, +) + + +def test_given_position_as_list_when_ispyb_params_created_then_converted_to_numpy_array(): + params = deepcopy(GRIDSCAN_ISPYB_PARAM_DEFAULTS) + params["position"] = [1, 2, 3] + + ispyb_params = IspybParams(**params) + + assert isinstance(ispyb_params.position, np.ndarray) + assert np.array_equal(ispyb_params.position, [1, 2, 3]) + + +def test_given_ispyb_params_when_converted_to_dict_then_position_is_a_list(): + params = deepcopy(GRIDSCAN_ISPYB_PARAM_DEFAULTS) + params["position"] = [1, 2, 3] + + ispyb_params_dict = IspybParams(**params).dict() + + assert isinstance(ispyb_params_dict["position"], list) + assert ispyb_params_dict["position"] == [1, 2, 3] + + +def test_given_transmission_greater_than_1_when_ispyb_params_created_then_throws_exception(): + params = deepcopy(GRIDSCAN_ISPYB_PARAM_DEFAULTS) + params["transmission_fraction"] = 20.5 + + with pytest.raises(ValueError): + IspybParams(**params)