Merge pull request #1297 from DiamondLightSource/1292_remove_redundan…

…t_params

Remove redundant ispyb params param

src/hyperion/experiment_plans/robot_load_then_centre_plan.py

@@ -200,8 +200,6 @@ def robot_load_then_centre(
         yield from read_energy(cast(SetEnergyComposite, composite))
     )
 
-    # XXX TODO 1173 remove this - left in to avoid breaking nexus files?
-    parameters.hyperion_params.ispyb_params.current_energy_ev = actual_energy_ev
     if not parameters.experiment_params.requested_energy_kev:
         parameters.hyperion_params.detector_params.expected_energy_ev = actual_energy_ev
     else:

src/hyperion/external_interaction/callbacks/ispyb_callback_base.py

@@ -109,29 +109,19 @@ def activity_gated_event(self, doc: Event) -> Event:
 
         if event_descriptor.get("name") == CONST.PLAN.ISPYB_TRANSMISSION_FLUX_READ:
             assert self._event_driven_data_collection_info
-            if doc["data"]["attenuator_actual_transmission"]:
+            if transmission := doc["data"]["attenuator_actual_transmission"]:
                 # Ispyb wants the transmission in a percentage, we use fractions
                 self._event_driven_data_collection_info.transmission = (
-                    doc["data"]["attenuator_actual_transmission"] * 100
+                    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"
             ]
-            # 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
-            )
             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(energy_ev)
                 )
-                # TODO 1173 Remove this once nexus_utils no longer needs wavelength_angstroms
-                self.params.hyperion_params.ispyb_params.current_energy_ev = energy_ev
 
             scan_data_infos = self.populate_info_for_update(
                 self._event_driven_data_collection_info, self.params

src/hyperion/external_interaction/ispyb/ispyb_dataclass.py

@@ -4,21 +4,16 @@
 import numpy as np
 from pydantic import BaseModel, validator
 
-from hyperion.utils.utils import convert_eV_to_angstrom
-
 GRIDSCAN_ISPYB_PARAM_DEFAULTS = {
     "sample_id": None,
     "visit_path": "",
     "microns_per_pixel_x": 0.0,
     "microns_per_pixel_y": 0.0,
-    "current_energy_ev": None,
     # gets stored as 2x2D coords - (x, y) and (x, z). Values in pixels
     "upper_left": [0, 0, 0],
     "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_fraction": None,
-    "flux": None,
     "beam_size_x": 0.1,
     "beam_size_y": 0.1,
     "focal_spot_size_x": 0.0,
@@ -35,9 +30,6 @@ class IspybParams(BaseModel):
     microns_per_pixel_y: float
     position: np.ndarray
 
-    transmission_fraction: Optional[float]  # TODO 1033 this is now deprecated
-    # populated by robot_load_then_centre
-    current_energy_ev: Optional[float]
     beam_size_x: float
     beam_size_y: float
     focal_spot_size_x: float
@@ -49,7 +41,6 @@ class IspybParams(BaseModel):
     sample_id: Optional[str] = None
 
     # Optional from GDA as populated by Ophyd
-    flux: Optional[float] = None
     undulator_gap: Optional[float] = None
     xtal_snapshots_omega_start: Optional[list[str]] = None
     xtal_snapshots_omega_end: Optional[list[str]] = None
@@ -74,20 +65,6 @@ def _parse_position(
             return position
         return np.array(position)
 
-    @validator("transmission_fraction")
-    def _transmission_not_percentage(cls, transmission_fraction: Optional[float]):
-        if transmission_fraction and transmission_fraction > 1:
-            raise ValueError(
-                "Transmission_fraction of >1 given. Did you give a percentage instead of a fraction?"
-            )
-        return transmission_fraction
-
-    @property
-    def wavelength_angstroms(self) -> Optional[float]:
-        if self.current_energy_ev:
-            return convert_eV_to_angstrom(self.current_energy_ev)
-        return None  # Return None instead of 0 in order to avoid overwriting previously written values
-
 
 class RobotLoadIspybParams(IspybParams): ...
 

src/hyperion/parameters/plan_specific/rotation_scan_internal_params.py

@@ -127,7 +127,6 @@ def _preprocess_hyperion_params(
             all_params["omega_increment"] = 0
         all_params["num_triggers"] = 1
         all_params["num_images_per_trigger"] = all_params["num_images"]
-        all_params["current_energy_ev"] = all_params["expected_energy_ev"]
         return RotationHyperionParameters(
             **extract_hyperion_params_from_flat_dict(
                 all_params, cls._hyperion_param_key_definitions()

src/hyperion/parameters/schemas/ispyb_parameters_schema.json

@@ -45,9 +45,6 @@
                 "type": "string"
             }
         },
-        "flux": {
-            "type": ["number", "null"]
-        },
         "beam_size_x": {
             "type": "number"
         },

tests/system_tests/experiment_plans/test_fgs_plan.py

@@ -54,7 +54,6 @@
 def params():
     params = GridscanInternalParameters(**default_raw_params())
     params.hyperion_params.beamline = CONST.SIM.BEAMLINE
-    params.hyperion_params.ispyb_params.current_energy_ev = 10000
     params.hyperion_params.zocalo_environment = "dev_artemis"
     params.hyperion_params.ispyb_params.microns_per_pixel_x = 10
     params.hyperion_params.ispyb_params.microns_per_pixel_y = 10

tests/system_tests/external_interaction/callbacks/test_external_callbacks.py

@@ -220,10 +220,6 @@ def test_remote_callbacks_write_to_dev_ispyb_for_rotation(
     test_rotation_params.hyperion_params.ispyb_params.beam_size_x = test_bs_x
     test_rotation_params.hyperion_params.ispyb_params.beam_size_y = test_bs_y
     test_rotation_params.hyperion_params.detector_params.exposure_time = test_exp_time
-    # TODO 1173 this should not be set here
-    test_rotation_params.hyperion_params.ispyb_params.current_energy_ev = (
-        convert_angstrom_to_eV(test_wl)
-    )
     test_rotation_params.hyperion_params.detector_params.expected_energy_ev = (
         convert_angstrom_to_eV(test_wl)
     )

tests/test_data/hyperion_parameters.json

@@ -4,7 +4,7 @@
     "insertion_prefix": "SR03S",
     "experiment_type": "flyscan_xray_centre",
     "detector_params": {
-        "current_energy_ev": 100.0,
+        "expected_energy_ev": 100.0,
         "exposure_time": 0.1,
         "directory": "/tmp/",
         "prefix": "file_name",
@@ -34,8 +34,6 @@
             20.0,
             30.0
         ],
-        "current_energy_ev": 100.0,
-        "transmission_fraction": 1.0,
         "beam_size_x": 1.0,
         "beam_size_y": 1.0,
         "focal_spot_size_x": 1.0,

tests/test_data/parameter_json_files/bad_test_parameters_wrong_version.json

@@ -47,7 +47,6 @@
                 "test_2",
                 "test_3"
             ],
-            "transmission_fraction": 1.0,
             "beam_size_x": 1.0,
             "beam_size_y": 1.0,
             "focal_spot_size_x": 1.0,

tests/unit_tests/experiment_plans/test_rotation_scan_plan.py

@@ -189,20 +189,6 @@ async def test_rotation_plan_zebra_settings(
     assert await zebra.pc.pulse_start.get_value() == expt_params.shutter_opening_time_s
 
 
-def test_rotation_plan_energy_settings(setup_and_run_rotation_plan_for_tests_standard):
-    params: RotationInternalParameters = setup_and_run_rotation_plan_for_tests_standard[
-        "test_rotation_params"
-    ]
-
-    assert (
-        params.hyperion_params.detector_params.expected_energy_ev == 100
-    )  # from good_test_rotation_scan_parameters.json
-    assert (
-        params.hyperion_params.detector_params.expected_energy_ev
-        == params.hyperion_params.ispyb_params.current_energy_ev
-    )
-
-
 def test_full_rotation_plan_smargon_settings(
     run_full_rotation_plan,
     test_rotation_params,

tests/unit_tests/experiment_plans/test_wait_for_robot_load_then_centre.py

@@ -86,7 +86,6 @@ def test_when_plan_run_then_centring_plan_run_with_expected_parameters(
 
     assert isinstance(params_passed, PinCentreThenXrayCentreInternalParameters)
     assert params_passed.hyperion_params.detector_params.expected_energy_ev == 11100
-    assert params_passed.hyperion_params.ispyb_params.current_energy_ev == 11105
     assert isinstance(
         resolution := params_passed.hyperion_params.ispyb_params.resolution, float
     )
@@ -121,7 +120,6 @@ def test_when_plan_run_with_requested_energy_specified_energy_change_executes(
         mock_centring_plan.call_args[0][1]
     )
     assert params_passed.hyperion_params.detector_params.expected_energy_ev == 11100
-    assert params_passed.hyperion_params.ispyb_params.current_energy_ev == 11105
 
 
 @patch(
@@ -153,7 +151,6 @@ def test_robot_load_then_centre_doesnt_set_energy_if_not_specified(
         mock_centring_plan.call_args[0][1]
     )
     assert params_passed.hyperion_params.detector_params.expected_energy_ev == 11105
-    assert params_passed.hyperion_params.ispyb_params.current_energy_ev == 11105
 
 
 def run_simulating_smargon_wait(

tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py

@@ -139,9 +139,6 @@ def activated_mocked_cbs():
     "hyperion.external_interaction.callbacks.rotation.ispyb_callback.StoreInIspyb",
     autospec=True,
 )
-@pytest.mark.skip(
-    reason="TODO 1173 fix this test and nexus_callback implementation so that it doesn't assume that hw info is available at activity_gated_start()"
-)
 def test_nexus_handler_gets_documents_in_mock_plan(
     ispyb,
     RE: RunEngine,
@@ -151,9 +148,6 @@ def test_nexus_handler_gets_documents_in_mock_plan(
     nexus_handler, _ = activated_mocked_cbs
     RE(fake_rotation_scan(params, [nexus_handler]))
 
-    params.hyperion_params.ispyb_params.transmission_fraction = 1.0
-    params.hyperion_params.ispyb_params.flux = 10.0
-
     assert nexus_handler.activity_gated_start.call_count == 2  # type: ignore
     call_content_outer = nexus_handler.activity_gated_start.call_args_list[0].args[0]  # type: ignore
     assert call_content_outer["hyperion_internal_parameters"] == params.json()

tests/unit_tests/external_interaction/conftest.py

@@ -70,23 +70,18 @@ def test_rotation_params():
     param_dict["hyperion_params"]["detector_params"]["directory"] = "tests/test_data"
     param_dict["hyperion_params"]["detector_params"]["prefix"] = "TEST_FILENAME"
     param_dict["hyperion_params"]["detector_params"]["expected_energy_ev"] = 12700
-    param_dict["hyperion_params"]["ispyb_params"]["current_energy_ev"] = 12700
     param_dict["experiment_params"]["rotation_angle"] = 360.0
     params = RotationInternalParameters(**param_dict)
     params.experiment_params.x = 0
     params.experiment_params.y = 0
     params.experiment_params.z = 0
     params.hyperion_params.detector_params.exposure_time = 0.004
-    params.hyperion_params.ispyb_params.transmission_fraction = 0.49118047952
     return params
 
 
 @pytest.fixture(params=[1044])
 def test_fgs_params(request):
     params = GridscanInternalParameters(**default_raw_params())
-    params.hyperion_params.ispyb_params.current_energy_ev = convert_angstrom_to_eV(1.0)
-    params.hyperion_params.ispyb_params.flux = 9.0
-    params.hyperion_params.ispyb_params.transmission_fraction = 0.5
     params.hyperion_params.detector_params.expected_energy_ev = convert_angstrom_to_eV(
         1.0
     )

tests/unit_tests/external_interaction/test_ispyb_dataclass.py

@@ -1,7 +1,6 @@
 from copy import deepcopy
 
 import numpy as np
-import pytest
 
 from hyperion.external_interaction.ispyb.ispyb_dataclass import (
     GRIDSCAN_ISPYB_PARAM_DEFAULTS,
@@ -27,11 +26,3 @@ def test_given_ispyb_params_when_converted_to_dict_then_position_is_a_list():
 
     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)

tests/unit_tests/external_interaction/test_write_rotation_nexus.py

@@ -38,7 +38,6 @@ def test_params(tmpdir):
     ] = f"{tmpdir}/{TEST_FILENAME}"
     param_dict["experiment_params"]["rotation_angle"] = 360.0
     param_dict["hyperion_params"]["detector_params"]["expected_energy_ev"] = 12700
-    param_dict["hyperion_params"]["ispyb_params"]["current_energy_ev"] = 12700
     param_dict["experiment_params"]["rotation_angle"] = 360.0
     params = RotationInternalParameters(**param_dict)
     params.experiment_params.x = 0

tests/unit_tests/parameters/test_internal_parameters.py

@@ -67,19 +67,7 @@ def rotation_params(rotation_raw_params):
 
 def test_cant_initialise_abstract_internalparams():
     with pytest.raises(TypeError):
-        internal_parameters = InternalParameters(  # noqa
-            **hyperion.parameters.external_parameters.from_file()
-        )
-
-
-def test_ispyb_param_wavelength():
-    from hyperion.utils.utils import convert_eV_to_angstrom
-
-    ispyb_params = GridscanIspybParams(**GRIDSCAN_ISPYB_PARAM_DEFAULTS)
-    ispyb_params.current_energy_ev = 12700
-    assert ispyb_params.wavelength_angstroms == pytest.approx(
-        convert_eV_to_angstrom(12700)
-    )
+        InternalParameters(**hyperion.parameters.external_parameters.from_file())
 
 
 def test_ispyb_param_dict():