From a0affdc748b55d51abf5a71af46504ef34553b33 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Fri, 9 Jun 2023 12:17:31 +0100 Subject: [PATCH 01/36] (#679) Use one return status for all exceptions --- src/artemis/__main__.py | 31 +++++++++----------- src/artemis/system_tests/test_main_system.py | 31 ++++++++++++++------ 2 files changed, 36 insertions(+), 26 deletions(-) diff --git a/src/artemis/__main__.py b/src/artemis/__main__.py index 8390c45a3..12d6432c9 100755 --- a/src/artemis/__main__.py +++ b/src/artemis/__main__.py @@ -2,7 +2,6 @@ import atexit import threading from dataclasses import dataclass -from json import JSONDecodeError from queue import Queue from traceback import format_exception from typing import Callable, Optional, Tuple @@ -11,7 +10,6 @@ from dataclasses_json import dataclass_json from flask import Flask, request from flask_restful import Api, Resource -from jsonschema.exceptions import ValidationError import artemis.log from artemis.exceptions import WarningException @@ -50,6 +48,16 @@ def __init__(self, status: Status, message: str = "") -> None: self.message = message +@dataclass_json +@dataclass +class ErrorStatusAndMessage(StatusAndMessage): + exception_type: str = "" + + def __init__(self, exception: Exception) -> None: + super().__init__(Status.FAILED, repr(exception)) + self.exception_type = type(exception).__name__ + + class BlueskyRunner: callbacks: AbstractPlanCallbackCollection command_queue: "Queue[Command]" = Queue() @@ -94,7 +102,7 @@ def stopping_thread(self): self.RE.abort() self.current_status = StatusAndMessage(Status.IDLE) except Exception as e: - self.current_status = StatusAndMessage(Status.FAILED, repr(e)) + self.current_status = ErrorStatusAndMessage(e) def stop(self) -> StatusAndMessage: if self.current_status.status == Status.IDLE.value: @@ -131,16 +139,14 @@ def wait_on_queue(self): self.last_run_aborted = False except WarningException as exception: artemis.log.LOGGER.warning("Warning Exception", exc_info=True) - self.current_status = StatusAndMessage(Status.WARN, repr(exception)) + self.current_status = ErrorStatusAndMessage(exception) except Exception as exception: artemis.log.LOGGER.error("Exception on running plan", exc_info=True) if self.last_run_aborted: # Aborting will cause an exception here that we want to swallow self.last_run_aborted = False else: - self.current_status = StatusAndMessage( - Status.FAILED, repr(exception) - ) + self.current_status = ErrorStatusAndMessage(exception) class RunExperiment(Resource): @@ -180,17 +186,8 @@ def put(self, plan_name: str, action: Actions): status_and_message = self.runner.start( experiment, parameters, plan_name ) - except JSONDecodeError as e: - status_and_message = StatusAndMessage(Status.FAILED, repr(e)) - except PlanNotFound as e: - status_and_message = StatusAndMessage(Status.FAILED, repr(e)) - except ValidationError as e: - status_and_message = StatusAndMessage(Status.FAILED, repr(e)) - artemis.log.LOGGER.error( - f" {format_exception(e)}: Invalid json parameters" - ) except Exception as e: - status_and_message = StatusAndMessage(Status.FAILED, repr(e)) + status_and_message = ErrorStatusAndMessage(e) artemis.log.LOGGER.error(format_exception(e)) elif action == Actions.STOP.value: diff --git a/src/artemis/system_tests/test_main_system.py b/src/artemis/system_tests/test_main_system.py index 6787add05..5b7eda497 100644 --- a/src/artemis/system_tests/test_main_system.py +++ b/src/artemis/system_tests/test_main_system.py @@ -12,6 +12,7 @@ from flask.testing import FlaskClient from artemis.__main__ import Actions, BlueskyRunner, Status, cli_arg_parse, create_app +from artemis.exceptions import WarningException from artemis.experiment_plans.experiment_registry import PLAN_REGISTRY from artemis.external_interaction.callbacks.abstract_plan_callback_collection import ( AbstractPlanCallbackCollection, @@ -31,19 +32,19 @@ class MockRunEngine: RE_takes_time = True aborting_takes_time = False - error: Optional[str] = None + error: Optional[Exception] = None def __call__(self, *args: Any, **kwds: Any) -> Any: while self.RE_takes_time: sleep(0.1) if self.error: - raise Exception(self.error) + raise self.error def abort(self): while self.aborting_takes_time: sleep(0.1) if self.error: - raise Exception(self.error) + raise self.error self.RE_takes_time = False def subscribe(self, *args): @@ -218,14 +219,16 @@ def test_given_started_when_stopped_and_started_again_then_runs( def test_given_started_when_RE_stops_on_its_own_with_error_then_error_reported( + caplog, test_env: ClientAndRunEngine, ): test_env.client.put(START_ENDPOINT, data=TEST_PARAMS) - error_message = "D'Oh" - test_env.mock_run_engine.error = error_message + test_env.mock_run_engine.error = Exception("D'Oh") response_json = wait_for_run_engine_status(test_env.client) assert response_json["status"] == Status.FAILED.value assert response_json["message"] == 'Exception("D\'Oh")' + assert response_json["exception_type"] == "Exception" + assert caplog.records[-1].levelname == "ERROR" def test_when_started_n_returnstatus_interrupted_bc_RE_aborted_thn_error_reptd( @@ -233,14 +236,14 @@ def test_when_started_n_returnstatus_interrupted_bc_RE_aborted_thn_error_reptd( ): test_env.mock_run_engine.aborting_takes_time = True test_env.client.put(START_ENDPOINT, data=TEST_PARAMS) - error_message = "D'Oh" test_env.client.put(STOP_ENDPOINT) - test_env.mock_run_engine.error = error_message + test_env.mock_run_engine.error = Exception("D'Oh") response_json = wait_for_run_engine_status( test_env.client, lambda status: status != Status.ABORTING.value ) assert response_json["status"] == Status.FAILED.value assert response_json["message"] == 'Exception("D\'Oh")' + assert response_json["exception_type"] == "Exception" def test_given_started_when_RE_stops_on_its_own_happily_then_no_error_reported( @@ -422,7 +425,7 @@ def test_when_blueskyrunner_initiated_and_skip_flag_is_not_set_then_all_plans_se assert mock_setup.call_count == 4 -def test_log_on_invalid_json_params(caplog, test_env: ClientAndRunEngine): +def test_log_on_invalid_json_params(test_env: ClientAndRunEngine): response = test_env.client.put(TEST_BAD_PARAM_ENDPOINT, data='{"bad":1}').json assert isinstance(response, dict) assert response.get("status") == Status.FAILED.value @@ -430,4 +433,14 @@ def test_log_on_invalid_json_params(caplog, test_env: ClientAndRunEngine): response.get("message") == "" ) - assert "Invalid json parameters" in caplog.text + assert response.get("exception_type") == "ValidationError" + + +def test_warn_exception_during_plan_causes_warning_in_log(caplog, test_env): + test_env.client.put(START_ENDPOINT, data=TEST_PARAMS) + test_env.mock_run_engine.error = WarningException("D'Oh") + response_json = wait_for_run_engine_status(test_env.client) + assert response_json["status"] == Status.FAILED.value + assert response_json["message"] == 'WarningException("D\'Oh")' + assert response_json["exception_type"] == "WarningException" + assert caplog.records[-1].levelname == "WARNING" From 12af18c9ccc30c0070b80c9cb42ab203034db82f Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Mon, 26 Jun 2023 17:08:50 +0100 Subject: [PATCH 02/36] Remove redundant setting the snapshot plugin --- setup.cfg | 2 +- src/artemis/device_setup_plans/setup_oav.py | 2 -- src/artemis/experiment_plans/oav_grid_detection_plan.py | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/setup.cfg b/setup.cfg index 26e0c0bd4..9bae1aa60 100644 --- a/setup.cfg +++ b/setup.cfg @@ -36,7 +36,7 @@ install_requires = xarray doct databroker - dodal @ git+https://github.com/DiamondLightSource/python-dodal.git@5cb5579b3eb29bdba8d5bf966ec965ed3aeba873 + dodal @ git+https://github.com/DiamondLightSource/python-dodal.git@64f4a6cf3c2ed59dcd03fdc9ded1fc8d02f5cde7 [options.extras_require] dev = diff --git a/src/artemis/device_setup_plans/setup_oav.py b/src/artemis/device_setup_plans/setup_oav.py index 5b76b5295..56545641b 100644 --- a/src/artemis/device_setup_plans/setup_oav.py +++ b/src/artemis/device_setup_plans/setup_oav.py @@ -87,8 +87,6 @@ def pre_centring_setup_oav(oav: OAV, parameters: OAVParameters): parameters.detection_script_filename, ) - yield from bps.abs_set(oav.snapshot.input_plugin, parameters.input_plugin + ".CAM") - zoom_level_str = f"{float(parameters.zoom)}x" if zoom_level_str not in oav.zoom_controller.allowed_zoom_levels: raise OAVError_ZoomLevelNotFound( diff --git a/src/artemis/experiment_plans/oav_grid_detection_plan.py b/src/artemis/experiment_plans/oav_grid_detection_plan.py index 066d3b738..9980f1e89 100644 --- a/src/artemis/experiment_plans/oav_grid_detection_plan.py +++ b/src/artemis/experiment_plans/oav_grid_detection_plan.py @@ -209,5 +209,4 @@ def grid_detection_main_plan( def reset_oav(parameters: OAVParameters): oav = i03.oav() - yield from bps.abs_set(oav.snapshot.input_plugin, parameters.input_plugin + ".CAM") yield from bps.abs_set(oav.mxsc.enable_callbacks, 0) From df059cbf49a2059387a851dbe0559b9c8762e1e9 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Mon, 26 Jun 2023 17:13:20 +0100 Subject: [PATCH 03/36] Updated to use renamed zoom device --- src/artemis/device_setup_plans/setup_oav.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/artemis/device_setup_plans/setup_oav.py b/src/artemis/device_setup_plans/setup_oav.py index 56545641b..0a489e752 100644 --- a/src/artemis/device_setup_plans/setup_oav.py +++ b/src/artemis/device_setup_plans/setup_oav.py @@ -88,13 +88,13 @@ def pre_centring_setup_oav(oav: OAV, parameters: OAVParameters): ) zoom_level_str = f"{float(parameters.zoom)}x" - if zoom_level_str not in oav.zoom_controller.allowed_zoom_levels: + if zoom_level_str not in oav.zoom.allowed_zoom_levels: raise OAVError_ZoomLevelNotFound( - f"Found {zoom_level_str} as a zoom level but expected one of {oav.zoom_controller.allowed_zoom_levels}" + f"Found {zoom_level_str} as a zoom level but expected one of {oav.zoom.allowed_zoom_levels}" ) yield from bps.abs_set( - oav.zoom_controller.level, + oav.zoom.level, zoom_level_str, wait=True, ) From 017e17f80d934605c8868df5c89563ea3822ecb9 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Mon, 26 Jun 2023 17:17:15 +0100 Subject: [PATCH 04/36] Remove setting the mxsc input as this is handled by zoom device --- src/artemis/device_setup_plans/setup_oav.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/artemis/device_setup_plans/setup_oav.py b/src/artemis/device_setup_plans/setup_oav.py index 0a489e752..af6da359f 100644 --- a/src/artemis/device_setup_plans/setup_oav.py +++ b/src/artemis/device_setup_plans/setup_oav.py @@ -7,18 +7,15 @@ from artemis.log import LOGGER -def start_mxsc(oav: OAV, input_plugin, min_callback_time, filename): +def start_mxsc(oav: OAV, min_callback_time, filename): """ Sets PVs relevant to edge detection plugin. Args: - input_plugin: link to the camera stream min_callback_time: the value to set the minimum callback time to filename: filename of the python script to detect edge waveforms from camera stream. Returns: None """ - yield from bps.abs_set(oav.mxsc.input_plugin, input_plugin) - # Turns the area detector plugin on yield from bps.abs_set(oav.mxsc.enable_callbacks, 1) @@ -82,7 +79,6 @@ def pre_centring_setup_oav(oav: OAV, parameters: OAVParameters): # Connect MXSC output to MJPG input yield from start_mxsc( oav, - parameters.input_plugin + "." + parameters.mxsc_input, parameters.min_callback_time, parameters.detection_script_filename, ) From 202c91a60edfb796460f372f1bb7a9000b2fb50b Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Mon, 26 Jun 2023 17:22:35 +0100 Subject: [PATCH 05/36] Fix tests to use properly named zoom --- .../tests/test_grid_detection_plan.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/artemis/experiment_plans/tests/test_grid_detection_plan.py b/src/artemis/experiment_plans/tests/test_grid_detection_plan.py index 3799a39d5..7ab31a41c 100644 --- a/src/artemis/experiment_plans/tests/test_grid_detection_plan.py +++ b/src/artemis/experiment_plans/tests/test_grid_detection_plan.py @@ -27,12 +27,12 @@ def fake_create_devices(): bl = i03.backlight(fake_with_ophyd_sim=True) bl.wait_for_connection() - oav.zoom_controller.zrst.set("1.0x") - oav.zoom_controller.onst.set("2.0x") - oav.zoom_controller.twst.set("3.0x") - oav.zoom_controller.thst.set("5.0x") - oav.zoom_controller.frst.set("7.0x") - oav.zoom_controller.fvst.set("9.0x") + oav.zoom.zrst.set("1.0x") + oav.zoom.onst.set("2.0x") + oav.zoom.twst.set("3.0x") + oav.zoom.thst.set("5.0x") + oav.zoom.frst.set("7.0x") + oav.zoom.fvst.set("9.0x") # fmt: off oav.mxsc.bottom.set([0,0,0,0,0,0,0,0,1,1,1,1,1,2,2,2,2,3,3,3,3,33,3,4,4,4]) # noqa: E231 From 2ebdfbe35b0020420f1536a7955bd91093e34bc7 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Mon, 26 Jun 2023 17:29:08 +0100 Subject: [PATCH 06/36] Minor tidy up --- src/artemis/experiment_plans/oav_grid_detection_plan.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/artemis/experiment_plans/oav_grid_detection_plan.py b/src/artemis/experiment_plans/oav_grid_detection_plan.py index 9980f1e89..9ecb131e3 100644 --- a/src/artemis/experiment_plans/oav_grid_detection_plan.py +++ b/src/artemis/experiment_plans/oav_grid_detection_plan.py @@ -45,7 +45,7 @@ def grid_detection_plan( width, box_size_microns, ), - reset_oav(parameters), + reset_oav(), ) @@ -207,6 +207,6 @@ def grid_detection_main_plan( out_parameters.z_step_size = box_size_um / 1000 -def reset_oav(parameters: OAVParameters): +def reset_oav(): oav = i03.oav() yield from bps.abs_set(oav.mxsc.enable_callbacks, 0) From 14e9cca732d1ebf60ed71e87faaccf5f7611b624 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Fri, 30 Jun 2023 13:02:29 +0100 Subject: [PATCH 07/36] (DiamondLightSource/dodal#103) Fix merge mistake --- src/artemis/device_setup_plans/setup_oav.py | 6 +++--- .../tests/test_grid_detection_plan.py | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/artemis/device_setup_plans/setup_oav.py b/src/artemis/device_setup_plans/setup_oav.py index af6da359f..e0ace9640 100644 --- a/src/artemis/device_setup_plans/setup_oav.py +++ b/src/artemis/device_setup_plans/setup_oav.py @@ -84,13 +84,13 @@ def pre_centring_setup_oav(oav: OAV, parameters: OAVParameters): ) zoom_level_str = f"{float(parameters.zoom)}x" - if zoom_level_str not in oav.zoom.allowed_zoom_levels: + if zoom_level_str not in oav.zoom_controller.allowed_zoom_levels: raise OAVError_ZoomLevelNotFound( - f"Found {zoom_level_str} as a zoom level but expected one of {oav.zoom.allowed_zoom_levels}" + f"Found {zoom_level_str} as a zoom level but expected one of {oav.zoom_controller.allowed_zoom_levels}" ) yield from bps.abs_set( - oav.zoom.level, + oav.zoom_controller.level, zoom_level_str, wait=True, ) diff --git a/src/artemis/experiment_plans/tests/test_grid_detection_plan.py b/src/artemis/experiment_plans/tests/test_grid_detection_plan.py index c8703dffa..b14463f0b 100644 --- a/src/artemis/experiment_plans/tests/test_grid_detection_plan.py +++ b/src/artemis/experiment_plans/tests/test_grid_detection_plan.py @@ -27,12 +27,12 @@ def fake_create_devices(): bl = i03.backlight(fake_with_ophyd_sim=True) bl.wait_for_connection() - oav.zoom.zrst.set("1.0x") - oav.zoom.onst.set("2.0x") - oav.zoom.twst.set("3.0x") - oav.zoom.thst.set("5.0x") - oav.zoom.frst.set("7.0x") - oav.zoom.fvst.set("9.0x") + oav.zoom_controller.zrst.set("1.0x") + oav.zoom_controller.onst.set("2.0x") + oav.zoom_controller.twst.set("3.0x") + oav.zoom_controller.thst.set("5.0x") + oav.zoom_controller.frst.set("7.0x") + oav.zoom_controller.fvst.set("9.0x") # fmt: off oav.mxsc.bottom.set([0,0,0,0,0,0,0,0,1,1,1,1,1,2,2,2,2,3,3,3,3,33,3,4,4,4]) # noqa: E231 From c11d0ad886fb52c301e4a1509ddaf0ca180ed92e Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Fri, 30 Jun 2023 15:34:34 +0100 Subject: [PATCH 08/36] (#767) Fix tests for the centre being returned from zocalo --- .../experiment_plans/tests/test_fast_grid_scan_plan.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 30e72451e..8202dc957 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 @@ -219,7 +219,7 @@ def test_individual_plans_triggered_once_and_only_once_in_composite_run( run_gridscan.assert_called_once_with(fake_fgs_composite, test_fgs_params) array_arg = move_xyz.call_args.args[1] - np.testing.assert_allclose(array_arg, np.array([-0.05, 0.05, 0.15])) + np.testing.assert_allclose(array_arg, np.array([0.05, 0.15, 0.25])) move_xyz.assert_called_once() @@ -254,7 +254,7 @@ def test_logging_within_plan( run_gridscan.assert_called_once_with(fake_fgs_composite, test_fgs_params) array_arg = move_xyz.call_args.args[1] - np.testing.assert_array_almost_equal(array_arg, np.array([-0.05, 0.05, 0.15])) + np.testing.assert_array_almost_equal(array_arg, np.array([0.05, 0.15, 0.25])) move_xyz.assert_called_once() From 473c4c8f3ad3e80ef002e7f4991099565074e107 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Fri, 30 Jun 2023 15:36:36 +0100 Subject: [PATCH 09/36] Update dodal version --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 18ac81d25..c8aa9fdc3 100644 --- a/setup.cfg +++ b/setup.cfg @@ -36,7 +36,7 @@ install_requires = xarray doct databroker - dodal @ git+https://github.com/DiamondLightSource/python-dodal.git@b49b2dffa27e44eebf42e8c7e35a375da42654cd + dodal @ git+https://github.com/DiamondLightSource/python-dodal.git@6c9b4a29b412da6ef62553d86ae705236baac77d [options.extras_require] dev = From 08a29d5e45cb51581d1c4a92333d74fa7d06570e Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Fri, 30 Jun 2023 16:44:02 +0100 Subject: [PATCH 10/36] (#765) Unstage detector if there is an issue in grid detection --- .../experiment_plans/full_grid_scan.py | 27 ++++++++--- .../tests/test_full_grid_scan_plan.py | 45 +++++++++++++++---- 2 files changed, 59 insertions(+), 13 deletions(-) diff --git a/src/artemis/experiment_plans/full_grid_scan.py b/src/artemis/experiment_plans/full_grid_scan.py index 0f3f12138..b6202da50 100644 --- a/src/artemis/experiment_plans/full_grid_scan.py +++ b/src/artemis/experiment_plans/full_grid_scan.py @@ -65,18 +65,37 @@ def wait_for_det_to_finish_moving(detector: DetectorMotion, timeout=120): raise TimeoutError("Detector not finished moving") -def detect_grid_and_do_gridscan( +def start_arming_then_do_grid( parameters: GridScanWithEdgeDetectInternalParameters, backlight: Backlight, eiger: EigerDetector, aperture_scatterguard: ApertureScatterguard, detector_motion: DetectorMotion, oav_params: OAVParameters, - experiment_params: GridScanWithEdgeDetectParams, ): # Start stage with asynchronous arming here yield from bps.abs_set(eiger.do_arm, 1, group="arming") + yield from bpp.finalize_wrapper( + detect_grid_and_do_gridscan( + parameters, + backlight, + aperture_scatterguard, + detector_motion, + oav_params, + ), + bps.unstage(eiger), + ) + + +def detect_grid_and_do_gridscan( + parameters: GridScanWithEdgeDetectInternalParameters, + backlight: Backlight, + aperture_scatterguard: ApertureScatterguard, + detector_motion: DetectorMotion, + oav_params: OAVParameters, +): + experiment_params: GridScanWithEdgeDetectParams = parameters.experiment_params fgs_params = GridScanParams(dwell_time=experiment_params.exposure_time * 1000) detector_params = parameters.artemis_params.detector_params @@ -156,14 +175,12 @@ def get_plan( eiger.set_detector_parameters(parameters.artemis_params.detector_params) oav_params = OAVParameters("xrayCentring", **oav_param_files) - experiment_params: GridScanWithEdgeDetectParams = parameters.experiment_params - return detect_grid_and_do_gridscan( + return start_arming_then_do_grid( parameters, backlight, eiger, aperture_scatterguard, detector_motion, oav_params, - experiment_params, ) 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 6a0c410a8..fbf4c20a9 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 @@ -14,6 +14,7 @@ create_devices, detect_grid_and_do_gridscan, get_plan, + start_arming_then_do_grid, wait_for_det_to_finish_moving, ) from artemis.parameters.plan_specific.grid_scan_with_edge_detect_params import ( @@ -93,7 +94,6 @@ def test_detect_grid_and_do_gridscan( mock_fast_grid_scan_plan: MagicMock, mock_grid_detection_plan: MagicMock, mock_wait_for_detector: MagicMock, - eiger: EigerDetector, backlight: Backlight, detector_motion: DetectorMotion, aperture_scatterguard: ApertureScatterguard, @@ -106,7 +106,7 @@ def test_detect_grid_and_do_gridscan( mock_oav_callback.out_upper_left = [[1, 1], [1, 1]] mock_grid_detection_plan.side_effect = _fake_grid_detection - with patch.object(eiger.do_arm, "set", MagicMock()) as mock_eiger_set, patch.object( + with patch.object( aperture_scatterguard, "set", MagicMock() ) as mock_aperture_scatterguard, patch( "artemis.external_interaction.callbacks.fgs.fgs_callback_collection.FGSCallbackCollection.from_params", @@ -116,17 +116,11 @@ def test_detect_grid_and_do_gridscan( detect_grid_and_do_gridscan( parameters=test_full_grid_scan_params, backlight=backlight, - eiger=eiger, aperture_scatterguard=aperture_scatterguard, detector_motion=detector_motion, oav_params=OAVParameters("xrayCentring", **test_config_files), - experiment_params=test_full_grid_scan_params.experiment_params, ) ) - - # Check detector was armed - mock_eiger_set.assert_called_once_with(1) - # Verify we called the grid detection plan mock_grid_detection_plan.assert_called_once() @@ -146,3 +140,38 @@ def test_detect_grid_and_do_gridscan( # Check we called out to underlying fast grid scan plan mock_fast_grid_scan_plan.assert_called_once_with(ANY, mock_subscriptions) + + +@patch("artemis.experiment_plans.full_grid_scan.grid_detection_plan") +@patch("artemis.experiment_plans.full_grid_scan.OavSnapshotCallback") +def test_grid_detection_running_when_exception_raised_then_eiger_unstaged( + mock_oav_callback: MagicMock, + mock_grid_detection_plan: MagicMock, + RE: RunEngine, + test_full_grid_scan_params: GridScanWithEdgeDetectInternalParameters, + mock_subscriptions: MagicMock, + test_config_files: Dict, +): + mock_grid_detection_plan.side_effect = Exception() + eiger: EigerDetector = MagicMock(spec=EigerDetector) + + with patch( + "artemis.external_interaction.callbacks.fgs.fgs_callback_collection.FGSCallbackCollection.from_params", + return_value=mock_subscriptions, + ): + with pytest.raises(Exception): + RE( + start_arming_then_do_grid( + parameters=test_full_grid_scan_params, + backlight=MagicMock(), + eiger=eiger, + aperture_scatterguard=MagicMock(), + detector_motion=MagicMock(), + oav_params=OAVParameters("xrayCentring", **test_config_files), + ) + ) + + # Check detector was armed + eiger.do_arm.set.assert_called_once_with(1) + + eiger.unstage.assert_called_once() From 78bfce9b709260a9a0e3c3e4a2b3bc5e38311dc7 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Thu, 22 Jun 2023 12:08:14 +0100 Subject: [PATCH 11/36] added deadtime optimisation function --- .../optimise_attenuation_plan.py | 139 +++++++++++++++--- 1 file changed, 116 insertions(+), 23 deletions(-) diff --git a/src/artemis/experiment_plans/optimise_attenuation_plan.py b/src/artemis/experiment_plans/optimise_attenuation_plan.py index aefb3364f..dfde48fa2 100644 --- a/src/artemis/experiment_plans/optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/optimise_attenuation_plan.py @@ -18,19 +18,25 @@ class AttenuationOptimisationFailedException(Exception): class PlaceholderParams: """placeholder for the actual params needed for this function""" + # Gets parameters from GDA i03-config/scripts/beamlineParameters @classmethod def from_beamline_params(cls, params): return ( - params["attenuation_optimisation_type"], # optimisation type, - int(params["fluorescence_attenuation_low_roi"]), # low_roi, - int(params["fluorescence_attenuation_high_roi"]), # high_roi + params["attenuation_optimisation_type"], # optimisation type: deadtime + int(params["fluorescence_attenuation_low_roi"]), # low_roi: 100 + int(params["fluorescence_attenuation_high_roi"]), # high_roi: 2048 params["attenuation_optimisation_start_transmission"] - / 100, # initial transmission, /100 to get decimal from percentage - params["attenuation_optimisation_target_count"] * 10, # target - params["attenuation_optimisation_lower_limit"], # lower limit - params["attenuation_optimisation_upper_limit"], # upper limit - int(params["attenuation_optimisation_optimisation_cycles"]), # max cycles - params["attenuation_optimisation_multiplier"], # increment + / 100, # initial transmission, /100 to get decimal from percentage: 0.1 + params["attenuation_optimisation_target_count"] * 10, # target:2000 + params["attenuation_optimisation_lower_limit"], # lower limit: 20000 + params["attenuation_optimisation_upper_limit"], # upper limit: 50000 + int( + params["attenuation_optimisation_optimisation_cycles"] + ), # max cycles: 10 + params["attenuation_optimisation_multiplier"], # increment: 2 + params[ + "fluorescence_analyser_deadtimeThreshold" + ], # Threshold for edge scans: 0.002 ) @@ -58,6 +64,77 @@ def arm_devices(xspress3mini, zebra): yield from arm_zebra(zebra) +# readout scaler values +def read_scaler_values(xspress3mini: Xspress3Mini) -> dict: + # Then get timeseriescontrol + + scaler_values = {} + scaler_values["time"] = xspress3mini.channel_1.time.get() + scaler_values["reset_ticks"] = xspress3mini.channel_1.reset_ticks.get() + scaler_values["reset_count"] = xspress3mini.channel_1.reset_count.get() + scaler_values["all_event"] = xspress3mini.channel_1.all_event.get() + scaler_values["all_good"] = xspress3mini.channel_1.all_good.get() + scaler_values["pileup"] = xspress3mini.channel_1.pileup.get() + scaler_values["total_time"] = xspress3mini.channel_1.total_time.get() + + # TODO: If we only use total time and reset ticks, this function may not be needed. Check if we use the other readings at all + return scaler_values + + +def deadtime_optimisation( + attenuator, xspress3mini, zebra, transmission, increment, deadtime_threshold +): + direction = True + LOGGER.info(f"Target deadtime is {deadtime_threshold}") + + while True: + # TODO: loads of these statements (first 4 lines at least) are the same as in total counts - add to seperate function for neatness + yield from bps.abs_set(attenuator, transmission, group="set_transmission") + yield from bps.abs_set(xspress3mini.set_num_images, 1, wait=True) + arm_devices(xspress3mini, zebra) + + scaler_values = read_scaler_values(xspress3mini) + + LOGGER.info(f"Current total time = {scaler_values['total_time']}") + LOGGER.info(f"Current reset ticks = {scaler_values['reset_ticks']}") + deadtime = 0.0 + + if scaler_values["total_time"] != scaler_values["reset_ticks"]: + deadtime = 1 - abs( + float({scaler_values["total_time"]} - scaler_values["reset_ticks"]) + ) / float({scaler_values["total_time"]}) + + LOGGER.info(f"Deadtime is now at {deadtime}") + + # Check if deadtime is optmised (TODO: put in function - useful for testing) + if direction: + if deadtime >= deadtime_threshold: + direction = False + else: + # TODO: is this number the 10% cap? + if transmission >= 0.9: + optimised_tranmission = transmission + break + else: + if deadtime <= deadtime_threshold: + optimised_tranmission = transmission + break + + # Calculate new transmission (TODO: put in function) + if direction: + transmission *= increment + if transmission > 0.999: + transmission = 1 + else: + transmission /= increment + if transmission < 1.0e-6: + raise AttenuationOptimisationFailedException( + "Calculated transmission is below expected limit" + ) + + return optimised_tranmission + + def total_counts_optimisation( max_cycles, transmission, @@ -151,6 +228,7 @@ def optimise_attenuation_plan( upper_limit, max_cycles, increment, + deadtime_threshold, ) = PlaceholderParams.from_beamline_params(get_beamline_parameters()) check_parameters( @@ -178,20 +256,35 @@ def optimise_attenuation_plan( # Do the attenuation optimisation using count threshold if optimisation_type == "total_counts": LOGGER.info( - f"Starting Xspress3Mini optimisation routine \nOptimisation will be performed across ROI channels {low_roi} - {high_roi}" + f"Starting Xspress3Mini total counts optimisation routine \nOptimisation will be performed across ROI channels {low_roi} - {high_roi}" ) - return ( - yield from total_counts_optimisation( - max_cycles, - initial_transmission, - attenuator, - xspress3mini, - zebra, - low_roi, - high_roi, - lower_limit, - upper_limit, - target, - ) + optimised_transmission = yield from total_counts_optimisation( + max_cycles, + initial_transmission, + attenuator, + xspress3mini, + zebra, + low_roi, + high_roi, + lower_limit, + upper_limit, + target, + ) + + elif optimisation_type == "deadtime": + LOGGER.info( + f"Starting Xspress3Mini deadtime optimisation routine \nOptimisation will be performed across ROI channels {low_roi} - {high_roi}" + ) + optimised_transmission = yield from deadtime_optimisation( + attenuator, + xspress3mini, + zebra, + initial_transmission, + increment, + deadtime_threshold, ) + + yield from bps.abs_set(attenuator, optimised_transmission, group="set_transmission") + + return optimised_transmission From d959b6f42cc8c3c6d6369acb1ec79ce3f4b9fac1 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Thu, 22 Jun 2023 13:17:16 +0100 Subject: [PATCH 12/36] add is_deadtime_optimised, remove read_scalar values --- .../optimise_attenuation_plan.py | 70 +++++++++---------- 1 file changed, 33 insertions(+), 37 deletions(-) diff --git a/src/artemis/experiment_plans/optimise_attenuation_plan.py b/src/artemis/experiment_plans/optimise_attenuation_plan.py index dfde48fa2..ddd0d36f0 100644 --- a/src/artemis/experiment_plans/optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/optimise_attenuation_plan.py @@ -64,21 +64,22 @@ def arm_devices(xspress3mini, zebra): yield from arm_zebra(zebra) -# readout scaler values -def read_scaler_values(xspress3mini: Xspress3Mini) -> dict: - # Then get timeseriescontrol - - scaler_values = {} - scaler_values["time"] = xspress3mini.channel_1.time.get() - scaler_values["reset_ticks"] = xspress3mini.channel_1.reset_ticks.get() - scaler_values["reset_count"] = xspress3mini.channel_1.reset_count.get() - scaler_values["all_event"] = xspress3mini.channel_1.all_event.get() - scaler_values["all_good"] = xspress3mini.channel_1.all_good.get() - scaler_values["pileup"] = xspress3mini.channel_1.pileup.get() - scaler_values["total_time"] = xspress3mini.channel_1.total_time.get() - - # TODO: If we only use total time and reset ticks, this function may not be needed. Check if we use the other readings at all - return scaler_values +def is_deadtime_optimised( + direction, deadtime, deadtime_threshold, transmission +) -> tuple[bool, bool]: + flip_direction = False + if direction: + if deadtime >= deadtime_threshold: + flip_direction = True + return False, flip_direction + else: + # TODO: is this number the 10% cap? + if transmission >= 0.9: + return True, flip_direction + else: + if deadtime <= deadtime_threshold: + return True, flip_direction + return False, flip_direction def deadtime_optimisation( @@ -93,32 +94,27 @@ def deadtime_optimisation( yield from bps.abs_set(xspress3mini.set_num_images, 1, wait=True) arm_devices(xspress3mini, zebra) - scaler_values = read_scaler_values(xspress3mini) + total_time = xspress3mini.channel_1.total_time.get() + reset_ticks = xspress3mini.channel_1.reset_ticks.get() - LOGGER.info(f"Current total time = {scaler_values['total_time']}") - LOGGER.info(f"Current reset ticks = {scaler_values['reset_ticks']}") - deadtime = 0.0 + LOGGER.info(f"Current total time = {total_time}") + LOGGER.info(f"Current reset ticks = {reset_ticks}") - if scaler_values["total_time"] != scaler_values["reset_ticks"]: - deadtime = 1 - abs( - float({scaler_values["total_time"]} - scaler_values["reset_ticks"]) - ) / float({scaler_values["total_time"]}) + if total_time != reset_ticks: + deadtime = 1 - abs(float({total_time} - reset_ticks)) / float({total_time}) LOGGER.info(f"Deadtime is now at {deadtime}") - # Check if deadtime is optmised (TODO: put in function - useful for testing) - if direction: - if deadtime >= deadtime_threshold: - direction = False - else: - # TODO: is this number the 10% cap? - if transmission >= 0.9: - optimised_tranmission = transmission - break - else: - if deadtime <= deadtime_threshold: - optimised_tranmission = transmission - break + is_optimised, flip_direction = is_deadtime_optimised( + direction, deadtime, deadtime_threshold, transmission + ) + + if is_optimised: + optimised_transmission = transmission + break + + if flip_direction: + direction = not direction # Calculate new transmission (TODO: put in function) if direction: @@ -132,7 +128,7 @@ def deadtime_optimisation( "Calculated transmission is below expected limit" ) - return optimised_tranmission + return optimised_transmission def total_counts_optimisation( From 032805787b1c9184bba304af99476c5b556e0f6d Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Thu, 22 Jun 2023 13:24:00 +0100 Subject: [PATCH 13/36] add calc_new_transmission --- .../optimise_attenuation_plan.py | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/artemis/experiment_plans/optimise_attenuation_plan.py b/src/artemis/experiment_plans/optimise_attenuation_plan.py index ddd0d36f0..3875730b5 100644 --- a/src/artemis/experiment_plans/optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/optimise_attenuation_plan.py @@ -82,6 +82,20 @@ def is_deadtime_optimised( return False, flip_direction +def deadtime_calc_new_transmission(direction, transmission, increment): + if direction: + transmission *= increment + if transmission > 0.999: + transmission = 1 + else: + transmission /= increment + if transmission < 1.0e-6: + raise AttenuationOptimisationFailedException( + "Calculated transmission is below expected limit" + ) + return transmission + + def deadtime_optimisation( attenuator, xspress3mini, zebra, transmission, increment, deadtime_threshold ): @@ -116,17 +130,9 @@ def deadtime_optimisation( if flip_direction: direction = not direction - # Calculate new transmission (TODO: put in function) - if direction: - transmission *= increment - if transmission > 0.999: - transmission = 1 - else: - transmission /= increment - if transmission < 1.0e-6: - raise AttenuationOptimisationFailedException( - "Calculated transmission is below expected limit" - ) + transmission = deadtime_calc_new_transmission( + direction, transmission, increment + ) return optimised_transmission From da84647cd0ed9b9483720d06a81cd702300abd1d Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Thu, 22 Jun 2023 13:41:14 +0100 Subject: [PATCH 14/36] add do_device_optimise_iteration --- .../optimise_attenuation_plan.py | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/artemis/experiment_plans/optimise_attenuation_plan.py b/src/artemis/experiment_plans/optimise_attenuation_plan.py index 3875730b5..2ba16c015 100644 --- a/src/artemis/experiment_plans/optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/optimise_attenuation_plan.py @@ -64,7 +64,7 @@ def arm_devices(xspress3mini, zebra): yield from arm_zebra(zebra) -def is_deadtime_optimised( +def deadtime_is_transmission_optimised( direction, deadtime, deadtime_threshold, transmission ) -> tuple[bool, bool]: flip_direction = False @@ -96,6 +96,15 @@ def deadtime_calc_new_transmission(direction, transmission, increment): return transmission +def do_device_optimise_iteration( + attenuator: Attenuator, zebra: Zebra, xspress3mini: Xspress3Mini, transmission +): + """Set transmission, set number of images on xspress3mini, arm xspress3mini and zebra""" + yield from bps.abs_set(attenuator, transmission, group="set_transmission") + yield from bps.abs_set(xspress3mini.set_num_images, 1, wait=True) + arm_devices(xspress3mini, zebra) + + def deadtime_optimisation( attenuator, xspress3mini, zebra, transmission, increment, deadtime_threshold ): @@ -103,10 +112,7 @@ def deadtime_optimisation( LOGGER.info(f"Target deadtime is {deadtime_threshold}") while True: - # TODO: loads of these statements (first 4 lines at least) are the same as in total counts - add to seperate function for neatness - yield from bps.abs_set(attenuator, transmission, group="set_transmission") - yield from bps.abs_set(xspress3mini.set_num_images, 1, wait=True) - arm_devices(xspress3mini, zebra) + do_device_optimise_iteration(attenuator, zebra, xspress3mini, transmission) total_time = xspress3mini.channel_1.total_time.get() reset_ticks = xspress3mini.channel_1.reset_ticks.get() @@ -119,7 +125,7 @@ def deadtime_optimisation( LOGGER.info(f"Deadtime is now at {deadtime}") - is_optimised, flip_direction = is_deadtime_optimised( + is_optimised, flip_direction = deadtime_is_transmission_optimised( direction, deadtime, deadtime_threshold, transmission ) @@ -156,15 +162,7 @@ def total_counts_optimisation( f"Setting transmission to {transmission} for attenuation optimisation cycle {cycle}" ) - yield from bps.abs_set( - attenuator, - transmission, - group="set_transmission", - ) - - yield from bps.abs_set(xspress3mini.set_num_images, 1, wait=True) - - yield from arm_devices(xspress3mini, zebra) + do_device_optimise_iteration(attenuator, zebra, xspress3mini, transmission) data = np.array((yield from bps.rd(xspress3mini.dt_corrected_latest_mca))) total_count = sum(data[int(low_roi) : int(high_roi)]) From 2cc800b0685e8c246caa3a19c4e66500a8b5f857 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Thu, 22 Jun 2023 14:49:05 +0100 Subject: [PATCH 15/36] minor restructure --- .../optimise_attenuation_plan.py | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/src/artemis/experiment_plans/optimise_attenuation_plan.py b/src/artemis/experiment_plans/optimise_attenuation_plan.py index 2ba16c015..1f7ff0c99 100644 --- a/src/artemis/experiment_plans/optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/optimise_attenuation_plan.py @@ -7,7 +7,6 @@ from artemis.device_setup_plans.setup_zebra import arm_zebra from artemis.log import LOGGER -from artemis.parameters import beamline_parameters from artemis.parameters.beamline_parameters import get_beamline_parameters @@ -46,6 +45,27 @@ def create_devices(): i03.attenuator() +def check_parameters( + target, upper_limit, lower_limit, default_high_roi, default_low_roi +): + if target < lower_limit or target > upper_limit: + raise ( + ValueError( + f"Target {target} is outside of lower and upper bounds: {lower_limit} to {upper_limit}" + ) + ) + + if upper_limit < lower_limit: + raise ValueError( + f"Upper limit {upper_limit} must be greater than lower limit {lower_limit}" + ) + + if default_high_roi < default_low_roi: + raise ValueError( + f"Upper roi {default_high_roi} must be greater than lower roi {default_low_roi}" + ) + + def is_counts_within_target(total_count, lower_limit, upper_limit) -> bool: if lower_limit <= total_count and total_count <= upper_limit: return True @@ -73,7 +93,7 @@ def deadtime_is_transmission_optimised( flip_direction = True return False, flip_direction else: - # TODO: is this number the 10% cap? + # The 0.9 is hardcoded in GDA if transmission >= 0.9: return True, flip_direction else: @@ -162,7 +182,9 @@ def total_counts_optimisation( f"Setting transmission to {transmission} for attenuation optimisation cycle {cycle}" ) - do_device_optimise_iteration(attenuator, zebra, xspress3mini, transmission) + yield from do_device_optimise_iteration( + attenuator, zebra, xspress3mini, transmission + ) data = np.array((yield from bps.rd(xspress3mini.dt_corrected_latest_mca))) total_count = sum(data[int(low_roi) : int(high_roi)]) @@ -188,30 +210,8 @@ def total_counts_optimisation( return optimised_transmission -def check_parameters( - target, upper_limit, lower_limit, default_high_roi, default_low_roi -): - if target < lower_limit or target > upper_limit: - raise ( - ValueError( - f"Target {target} is outside of lower and upper bounds: {lower_limit} to {upper_limit}" - ) - ) - - if upper_limit < lower_limit: - raise ValueError( - f"Upper limit {upper_limit} must be greater than lower limit {lower_limit}" - ) - - if default_high_roi < default_low_roi: - raise ValueError( - f"Upper roi {default_high_roi} must be greater than lower roi {default_low_roi}" - ) - - def optimise_attenuation_plan( collection_time, # Comes from self.parameters.acquisitionTime in fluorescence_spectrum.py - params: beamline_parameters, xspress3mini: Xspress3Mini, zebra: Zebra, attenuator: Attenuator, From 8682b2bd321a197396f4d321f103accad4876556 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Thu, 22 Jun 2023 15:11:05 +0100 Subject: [PATCH 16/36] Add overall test for deadtime optimise --- .../optimise_attenuation_plan.py | 8 ++- .../tests/test_optimise_attenuation_plan.py | 68 +++++++++++++++++-- 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/src/artemis/experiment_plans/optimise_attenuation_plan.py b/src/artemis/experiment_plans/optimise_attenuation_plan.py index 1f7ff0c99..3b05c1efe 100644 --- a/src/artemis/experiment_plans/optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/optimise_attenuation_plan.py @@ -132,7 +132,9 @@ def deadtime_optimisation( LOGGER.info(f"Target deadtime is {deadtime_threshold}") while True: - do_device_optimise_iteration(attenuator, zebra, xspress3mini, transmission) + yield from do_device_optimise_iteration( + attenuator, zebra, xspress3mini, transmission + ) total_time = xspress3mini.channel_1.total_time.get() reset_ticks = xspress3mini.channel_1.reset_ticks.get() @@ -141,7 +143,7 @@ def deadtime_optimisation( LOGGER.info(f"Current reset ticks = {reset_ticks}") if total_time != reset_ticks: - deadtime = 1 - abs(float({total_time} - reset_ticks)) / float({total_time}) + deadtime = 1 - abs(total_time - reset_ticks) / (total_time) LOGGER.info(f"Deadtime is now at {deadtime}") @@ -247,7 +249,7 @@ def optimise_attenuation_plan( high_roi = default_high_roi # Hardcode this for now: - optimisation_type = "total_counts" + optimisation_type = "deadtime" yield from bps.abs_set( xspress3mini.acquire_time, collection_time, wait=True diff --git a/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py b/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py index 425994b5c..ddf23cecd 100644 --- a/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py @@ -4,7 +4,9 @@ import pytest from bluesky.run_engine import RunEngine from dodal.beamlines import i03 -from dodal.devices.xspress3_mini.xspress3_mini import DetectorState +from dodal.devices.attenuator import Attenuator +from dodal.devices.xspress3_mini.xspress3_mini import DetectorState, Xspress3Mini +from dodal.devices.zebra import Zebra from ophyd.status import Status from artemis.experiment_plans import optimise_attenuation_plan @@ -13,13 +15,14 @@ PlaceholderParams, arm_devices, check_parameters, + deadtime_is_transmission_optimised, is_counts_within_target, total_counts_optimisation, ) from artemis.parameters.beamline_parameters import get_beamline_parameters -def fake_create_devices(): +def fake_create_devices() -> tuple[Zebra, Xspress3Mini, Attenuator]: zebra = i03.zebra(fake_with_ophyd_sim=True) zebra.wait_for_connection() xspress3mini = i03.xspress3mini(fake_with_ophyd_sim=True) @@ -64,6 +67,7 @@ def test_total_count_optimise(mock_arm_zebra, RE: RunEngine): upper_limit, max_cycles, increment, + deadtime_threshold, ) = PlaceholderParams.from_beamline_params(get_beamline_parameters()) # Same as plan target @@ -73,7 +77,7 @@ def test_total_count_optimise(mock_arm_zebra, RE: RunEngine): transmission_list = [transmission] # Mock a calculation where the dt_corrected_latest_mca array data - # is randomly created based on the transmission value + # is created based on the transmission value def mock_set_transmission(_): data = np.ones(shape=2048) * (transmission_list[0] + 1) total_count = sum(data[int(default_low_roi) : int(default_high_roi)]) @@ -88,7 +92,63 @@ def mock_set_transmission(_): RE( optimise_attenuation_plan.optimise_attenuation_plan( - 5, 1, xspress3mini, zebra, attenuator, 0, 0 + 5, xspress3mini, zebra, attenuator, 0, 0 + ) + ) + + +def test_deadtime_optimise(RE: RunEngine): + """Test the overall deadtime optimisation""" + + zebra, xspress3mini, attenuator = fake_create_devices() + + # Mimic some of the logic to track the transmission and set realistic data + ( + optimisation_type, + default_low_roi, + default_high_roi, + transmission, + target, + lower_limit, + upper_limit, + max_cycles, + increment, + deadtime_threshold, + ) = PlaceholderParams.from_beamline_params(get_beamline_parameters()) + + """ Similar to test_total_count, mimic the set transmission. For now, just assume total time is constant and increasing the transmission will increase the + reset ticks, thus decreasing the deadtime""" + transmission_list = [transmission] + direction_list = [True] + total_time = 8e7 + + # Put realistic values into PV's + xspress3mini.channel_1.total_time.sim_put(8e7) + xspress3mini.channel_1.reset_ticks.sim_put(151276) + + def mock_set_transmission(_): + # Update reset ticks, calc new deadtime, increment transmission. + reset_ticks = 151276 * transmission_list[0] + deadtime = 1 - abs(float(total_time - reset_ticks)) / float(total_time) + + _, direction_flip = deadtime_is_transmission_optimised( + direction_list[0], deadtime, deadtime_threshold, transmission + ) + if direction_flip: + direction_list[0] = not direction_list[0] + + if direction_list[0]: + transmission_list[0] *= increment + else: + transmission_list[0] /= increment + + return get_good_status() + + attenuator.desired_transmission.set = mock_set_transmission + force_fake_devices_to_arm(xspress3mini, zebra) + RE( + optimise_attenuation_plan.optimise_attenuation_plan( + 5, xspress3mini, zebra, attenuator, 0, 0 ) ) From a379f2a4dd85337fbf803f8db35f6685588f72f8 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Thu, 22 Jun 2023 15:43:04 +0100 Subject: [PATCH 17/36] initialise deadtime to 0 --- src/artemis/experiment_plans/optimise_attenuation_plan.py | 2 +- .../experiment_plans/tests/test_optimise_attenuation_plan.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/artemis/experiment_plans/optimise_attenuation_plan.py b/src/artemis/experiment_plans/optimise_attenuation_plan.py index 3b05c1efe..8a0b03ba1 100644 --- a/src/artemis/experiment_plans/optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/optimise_attenuation_plan.py @@ -141,7 +141,7 @@ def deadtime_optimisation( LOGGER.info(f"Current total time = {total_time}") LOGGER.info(f"Current reset ticks = {reset_ticks}") - + deadtime = 0 if total_time != reset_ticks: deadtime = 1 - abs(total_time - reset_ticks) / (total_time) diff --git a/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py b/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py index ddf23cecd..01bc82f2b 100644 --- a/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py @@ -129,7 +129,9 @@ def test_deadtime_optimise(RE: RunEngine): def mock_set_transmission(_): # Update reset ticks, calc new deadtime, increment transmission. reset_ticks = 151276 * transmission_list[0] - deadtime = 1 - abs(float(total_time - reset_ticks)) / float(total_time) + deadtime = 0 + if total_time != reset_ticks: + deadtime = 1 - abs(float(total_time - reset_ticks)) / float(total_time) _, direction_flip = deadtime_is_transmission_optimised( direction_list[0], deadtime, deadtime_threshold, transmission From 12acb95860775fbb114985fe5ecac3c8c8ba520d Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Thu, 22 Jun 2023 16:25:11 +0100 Subject: [PATCH 18/36] update dodal url --- setup.cfg | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/setup.cfg b/setup.cfg index 18ac81d25..6d11309eb 100644 --- a/setup.cfg +++ b/setup.cfg @@ -36,7 +36,11 @@ install_requires = xarray doct databroker +<<<<<<< HEAD dodal @ git+https://github.com/DiamondLightSource/python-dodal.git@b49b2dffa27e44eebf42e8c7e35a375da42654cd +======= + dodal @ git+https://github.com/DiamondLightSource/python-dodal.git@736d93002f692e2e319ddb43fec40ca3019d2918 +>>>>>>> fba920b (update dodal url) [options.extras_require] dev = From fe4599813492995f8ba7b2de34601c9fdbcbd8f4 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Thu, 22 Jun 2023 16:37:04 +0100 Subject: [PATCH 19/36] set optimisation type as plan parameter instead --- src/artemis/experiment_plans/optimise_attenuation_plan.py | 3 ++- .../experiment_plans/tests/test_optimise_attenuation_plan.py | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/artemis/experiment_plans/optimise_attenuation_plan.py b/src/artemis/experiment_plans/optimise_attenuation_plan.py index 8a0b03ba1..a3f7ea41e 100644 --- a/src/artemis/experiment_plans/optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/optimise_attenuation_plan.py @@ -214,6 +214,7 @@ def total_counts_optimisation( def optimise_attenuation_plan( collection_time, # Comes from self.parameters.acquisitionTime in fluorescence_spectrum.py + optimisation_type, xspress3mini: Xspress3Mini, zebra: Zebra, attenuator: Attenuator, @@ -221,7 +222,7 @@ def optimise_attenuation_plan( high_roi=None, ): ( - optimisation_type, + _, # This is optimisation type. Beter for testing if this is a parameter of the plan instead default_low_roi, default_high_roi, initial_transmission, diff --git a/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py b/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py index 01bc82f2b..1b36da4cf 100644 --- a/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py @@ -92,7 +92,7 @@ def mock_set_transmission(_): RE( optimise_attenuation_plan.optimise_attenuation_plan( - 5, xspress3mini, zebra, attenuator, 0, 0 + 5, "total_counts", xspress3mini, zebra, attenuator, 0, 0 ) ) @@ -150,7 +150,7 @@ def mock_set_transmission(_): force_fake_devices_to_arm(xspress3mini, zebra) RE( optimise_attenuation_plan.optimise_attenuation_plan( - 5, xspress3mini, zebra, attenuator, 0, 0 + 5, "deadtime", xspress3mini, zebra, attenuator, 0, 0 ) ) From 9ad51c89dc7073cb1ee4e3f1271662ddced588e3 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Fri, 23 Jun 2023 09:23:06 +0100 Subject: [PATCH 20/36] remove hardcoded optimisation type from plan --- src/artemis/experiment_plans/optimise_attenuation_plan.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/artemis/experiment_plans/optimise_attenuation_plan.py b/src/artemis/experiment_plans/optimise_attenuation_plan.py index a3f7ea41e..04a0d129b 100644 --- a/src/artemis/experiment_plans/optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/optimise_attenuation_plan.py @@ -249,9 +249,6 @@ def optimise_attenuation_plan( if high_roi is None or high_roi == 0: high_roi = default_high_roi - # Hardcode this for now: - optimisation_type = "deadtime" - yield from bps.abs_set( xspress3mini.acquire_time, collection_time, wait=True ) # Don't necessarily need to wait here From 154613cabbc1b3c39a4c83f441c5494a55e9501f Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Fri, 23 Jun 2023 13:26:52 +0100 Subject: [PATCH 21/36] more unit tests --- .../optimise_attenuation_plan.py | 5 ---- .../tests/test_optimise_attenuation_plan.py | 29 +++++++++++++++++-- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/artemis/experiment_plans/optimise_attenuation_plan.py b/src/artemis/experiment_plans/optimise_attenuation_plan.py index 04a0d129b..b3d485e40 100644 --- a/src/artemis/experiment_plans/optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/optimise_attenuation_plan.py @@ -55,11 +55,6 @@ def check_parameters( ) ) - if upper_limit < lower_limit: - raise ValueError( - f"Upper limit {upper_limit} must be greater than lower limit {lower_limit}" - ) - if default_high_roi < default_low_roi: raise ValueError( f"Upper roi {default_high_roi} must be greater than lower roi {default_low_roi}" diff --git a/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py b/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py index 1b36da4cf..5fae78531 100644 --- a/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py @@ -15,6 +15,8 @@ PlaceholderParams, arm_devices, check_parameters, + create_devices, + deadtime_calc_new_transmission, deadtime_is_transmission_optimised, is_counts_within_target, total_counts_optimisation, @@ -97,7 +99,11 @@ def mock_set_transmission(_): ) -def test_deadtime_optimise(RE: RunEngine): +@pytest.mark.parametrize( + "high_roi, low_roi", + [(0, 0), (0, 2048)], +) +def test_deadtime_optimise(high_roi, low_roi, RE: RunEngine): """Test the overall deadtime optimisation""" zebra, xspress3mini, attenuator = fake_create_devices() @@ -150,7 +156,7 @@ def mock_set_transmission(_): force_fake_devices_to_arm(xspress3mini, zebra) RE( optimise_attenuation_plan.optimise_attenuation_plan( - 5, "deadtime", xspress3mini, zebra, attenuator, 0, 0 + 5, "deadtime", xspress3mini, zebra, attenuator, high_roi, low_roi ) ) @@ -210,3 +216,22 @@ def test_arm_devices_runs_correct_functions(RE: RunEngine): RE(arm_devices(xspress3mini, zebra)) xspress3mini.arm.assert_called_once() optimise_attenuation_plan.arm_zebra.assert_called_once() + + +def test_deadtime_calc_new_transmission_gets_correct_value(): + assert deadtime_calc_new_transmission(True, 0.05, 2) == 0.1 + assert deadtime_calc_new_transmission(False, 0.05, 2) == 0.025 + assert deadtime_calc_new_transmission(True, 1, 2) == 1 + + +def test_deadtime_calc_new_transmission_raises_error_on_low_ransmission(): + with pytest.raises(AttenuationOptimisationFailedException): + deadtime_calc_new_transmission(False, 1e-6, 2) + + +def test_create_new_devices(): + with patch("artemis.experiment_plans.optimise_attenuation_plan.i03") as i03: + create_devices() + i03.zebra.assert_called() + i03.xspress3mini.assert_called() + i03.attenuator.assert_called() From f792b22afd460c0bc7f7670639285d7f6d82e777 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Fri, 23 Jun 2023 15:35:43 +0100 Subject: [PATCH 22/36] fix arming devices --- src/artemis/experiment_plans/optimise_attenuation_plan.py | 2 +- .../tests/test_optimise_attenuation_plan.py | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/artemis/experiment_plans/optimise_attenuation_plan.py b/src/artemis/experiment_plans/optimise_attenuation_plan.py index b3d485e40..bff90e1e5 100644 --- a/src/artemis/experiment_plans/optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/optimise_attenuation_plan.py @@ -117,7 +117,7 @@ def do_device_optimise_iteration( """Set transmission, set number of images on xspress3mini, arm xspress3mini and zebra""" yield from bps.abs_set(attenuator, transmission, group="set_transmission") yield from bps.abs_set(xspress3mini.set_num_images, 1, wait=True) - arm_devices(xspress3mini, zebra) + yield from arm_devices(xspress3mini, zebra) def deadtime_optimisation( diff --git a/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py b/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py index 5fae78531..0faeb5dd2 100644 --- a/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py @@ -103,7 +103,8 @@ def mock_set_transmission(_): "high_roi, low_roi", [(0, 0), (0, 2048)], ) -def test_deadtime_optimise(high_roi, low_roi, RE: RunEngine): +@patch("artemis.experiment_plans.optimise_attenuation_plan.arm_zebra") +def test_deadtime_optimise(mock_arm_zebra, high_roi, low_roi, RE: RunEngine): """Test the overall deadtime optimisation""" zebra, xspress3mini, attenuator = fake_create_devices() @@ -153,7 +154,8 @@ def mock_set_transmission(_): return get_good_status() attenuator.desired_transmission.set = mock_set_transmission - force_fake_devices_to_arm(xspress3mini, zebra) + # Force xspress3mini to pass arming + xspress3mini.detector_state.sim_put(DetectorState.ACQUIRE.value) RE( optimise_attenuation_plan.optimise_attenuation_plan( 5, "deadtime", xspress3mini, zebra, attenuator, high_roi, low_roi From c069735df08ebf45a788ebbfd50758c0ebea0589 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Wed, 28 Jun 2023 10:57:42 +0100 Subject: [PATCH 23/36] dodal url --- setup.cfg | 4 ---- 1 file changed, 4 deletions(-) diff --git a/setup.cfg b/setup.cfg index 6d11309eb..18ac81d25 100644 --- a/setup.cfg +++ b/setup.cfg @@ -36,11 +36,7 @@ install_requires = xarray doct databroker -<<<<<<< HEAD dodal @ git+https://github.com/DiamondLightSource/python-dodal.git@b49b2dffa27e44eebf42e8c7e35a375da42654cd -======= - dodal @ git+https://github.com/DiamondLightSource/python-dodal.git@736d93002f692e2e319ddb43fec40ca3019d2918 ->>>>>>> fba920b (update dodal url) [options.extras_require] dev = From c0ddb7ab64352a98f3cab40e5c5a557d163a262e Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Fri, 30 Jun 2023 10:21:31 +0100 Subject: [PATCH 24/36] Added comments and made direction an Enum --- .../optimise_attenuation_plan.py | 49 ++++++++++++++++--- 1 file changed, 43 insertions(+), 6 deletions(-) diff --git a/src/artemis/experiment_plans/optimise_attenuation_plan.py b/src/artemis/experiment_plans/optimise_attenuation_plan.py index bff90e1e5..9bbdba344 100644 --- a/src/artemis/experiment_plans/optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/optimise_attenuation_plan.py @@ -1,3 +1,5 @@ +from enum import Enum + import bluesky.plan_stubs as bps import numpy as np from dodal.beamlines import i03 @@ -14,6 +16,11 @@ class AttenuationOptimisationFailedException(Exception): pass +class Direction(Enum): + POSITIVE = "positive" + NEGATIVE = "negative" + + class PlaceholderParams: """placeholder for the actual params needed for this function""" @@ -80,10 +87,31 @@ def arm_devices(xspress3mini, zebra): def deadtime_is_transmission_optimised( - direction, deadtime, deadtime_threshold, transmission + direction: Direction, deadtime, deadtime_threshold, transmission ) -> tuple[bool, bool]: + """Compares the deadtime to the deadtime_threshold and determines behavior for the next optimisation iteration + + If deadtime is lower than the threshold or greater than 0.9, returns (True, flip_direction). + Marks the flip direction as positive if the deadtime has gone over the threshold. Raises error AttenuationOptimisationFailedException if + transmission goes too low + + Args: + direction: Enum + Enum taking values of either POSITIVE or NEGATIVE, which determines whether the transmission should be increased or decreased in the next iteration + + deadtime: + Current deadtime value + + Returns: + boolean: marking whether or not attenuation is optimised + + flip_direction: Boolean + Set to true if the deadtime goes above its threshold while direction is positive. This makes deadtime decrease on the next iteration. Otherwise + set to false. + """ + flip_direction = False - if direction: + if direction.value == Direction.POSITIVE: if deadtime >= deadtime_threshold: flip_direction = True return False, flip_direction @@ -97,8 +125,8 @@ def deadtime_is_transmission_optimised( return False, flip_direction -def deadtime_calc_new_transmission(direction, transmission, increment): - if direction: +def deadtime_calc_new_transmission(direction: Direction, transmission, increment): + if direction.value == Direction.POSITIVE: transmission *= increment if transmission > 0.999: transmission = 1 @@ -123,7 +151,7 @@ def do_device_optimise_iteration( def deadtime_optimisation( attenuator, xspress3mini, zebra, transmission, increment, deadtime_threshold ): - direction = True + direction = Direction.POSITIVE LOGGER.info(f"Target deadtime is {deadtime_threshold}") while True: @@ -137,6 +165,15 @@ def deadtime_optimisation( LOGGER.info(f"Current total time = {total_time}") LOGGER.info(f"Current reset ticks = {reset_ticks}") deadtime = 0 + + """ Deadtime is the time after each event during which the detector cannot record another event. + The reset ticks PV gives the (absolute) time at which the last event was processed, so the difference between the total time and the + reset ticks time gives the deadtime. Then divide by total time to get it as a percentage. + + This percentage can then be used to calculate the real counts. Eg Real counts = observed counts / (deadtime fraction) + + """ + if total_time != reset_ticks: deadtime = 1 - abs(total_time - reset_ticks) / (total_time) @@ -151,7 +188,7 @@ def deadtime_optimisation( break if flip_direction: - direction = not direction + direction = Direction.NEGATIVE transmission = deadtime_calc_new_transmission( direction, transmission, increment From 9d73376dfde2e6610247fcea5e5bfbc95e18a1e2 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Fri, 30 Jun 2023 10:24:14 +0100 Subject: [PATCH 25/36] corrected comments --- src/artemis/experiment_plans/optimise_attenuation_plan.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/artemis/experiment_plans/optimise_attenuation_plan.py b/src/artemis/experiment_plans/optimise_attenuation_plan.py index 9bbdba344..41e84e099 100644 --- a/src/artemis/experiment_plans/optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/optimise_attenuation_plan.py @@ -167,7 +167,7 @@ def deadtime_optimisation( deadtime = 0 """ Deadtime is the time after each event during which the detector cannot record another event. - The reset ticks PV gives the (absolute) time at which the last event was processed, so the difference between the total time and the + The reset ticks PV stops ticking while the detector is unable to process events, so the difference between the total time and the reset ticks time gives the deadtime. Then divide by total time to get it as a percentage. This percentage can then be used to calculate the real counts. Eg Real counts = observed counts / (deadtime fraction) From 5ad5e3a803a25584b9fa1f35ac97ac4d0e6ca8e1 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Fri, 30 Jun 2023 10:48:32 +0100 Subject: [PATCH 26/36] Minor tidy up and further documenting --- .../optimise_attenuation_plan.py | 104 +++++++++++++++--- .../tests/test_optimise_attenuation_plan.py | 2 +- 2 files changed, 90 insertions(+), 16 deletions(-) diff --git a/src/artemis/experiment_plans/optimise_attenuation_plan.py b/src/artemis/experiment_plans/optimise_attenuation_plan.py index 41e84e099..5efc65e8c 100644 --- a/src/artemis/experiment_plans/optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/optimise_attenuation_plan.py @@ -149,12 +149,47 @@ def do_device_optimise_iteration( def deadtime_optimisation( - attenuator, xspress3mini, zebra, transmission, increment, deadtime_threshold + attenuator: Attenuator, + xspress3mini: Xspress3Mini, + zebra: Zebra, + transmission: float, + increment: float, + deadtime_threshold: float, + max_cycles: int, ): + """Optimises the attenuation for the Xspress3Mini based on the detector deadtime + + Deadtime is the time after each event during which the detector cannot record another event. This loop adjusts the transmission of the attenuator + and checks the deadtime until the deadtime is below the accepted threshold. To protect the sample, the deadtime has a maximum value of 10% + + Args: + attenuator: Attenuator Ophyd device + + xspress3mini: Xspress3Mini ophyd device + + zebra: Zebra Ophyd device + + transmission: Float + The intial transmission value to use for the optimising + + increment: Float + The factor to increase / decrease the transmission each cycle + + deadtime_threshold: Float + The maximum acceptable percentage deadtime + + max_cycles: int + The maximum number of iterations before an error is thrown + + Returns: + optimised_transmission: float + The final transmission value which produces an acceptable deadtime + """ + direction = Direction.POSITIVE LOGGER.info(f"Target deadtime is {deadtime_threshold}") - while True: + for cycle in range(0, max_cycles): yield from do_device_optimise_iteration( attenuator, zebra, xspress3mini, transmission ) @@ -167,7 +202,7 @@ def deadtime_optimisation( deadtime = 0 """ Deadtime is the time after each event during which the detector cannot record another event. - The reset ticks PV stops ticking while the detector is unable to process events, so the difference between the total time and the + The reset ticks PV stops ticking while the detector is unable to process events, so the difference between the total time and the reset ticks time gives the deadtime. Then divide by total time to get it as a percentage. This percentage can then be used to calculate the real counts. Eg Real counts = observed counts / (deadtime fraction) @@ -194,21 +229,59 @@ def deadtime_optimisation( direction, transmission, increment ) + if cycle == max_cycles - 1: + raise AttenuationOptimisationFailedException( + f"Unable to optimise attenuation after maximum cycles.\ + Deadtime did not get lower than threshold: {deadtime_threshold} in maximum cycles {max_cycles}" + ) + return optimised_transmission def total_counts_optimisation( - max_cycles, - transmission, - attenuator, - xspress3mini, - zebra, - low_roi, - high_roi, - lower_limit, - upper_limit, - target_count, + attenuator: Attenuator, + xspress3mini: Xspress3Mini, + zebra: Zebra, + transmission: float, + low_roi: int, + high_roi: int, + lower_limit: float, + upper_limit: float, + target_count: float, + max_cycles: int, ): + """Optimises the attenuation for the Xspress3Mini based on the total counts + + This loop adjusts the transmission of the attenuator and checks the total counts of the detector until the total counts as in the acceptable range, + defined by the lower and upper limit. To protect the sample, the transmission has a maximum value of 10%. + + Args: + attenuator: Attenuator Ophyd device + + xspress3mini: Xspress3Mini ophyd device + + zebra: Zebra Ophyd device + + transmission: Float + The intial transmission value to use for the optimising + + low_roi: Float + Lower region of interest at which to include in the counts + + high_roi: Float + Upper region of interest at which to include in the counts + + target_count: int + The ideal number of target counts - used to calculate the transmission for the subsequent iteration. + + max_cycles: int + The maximum number of iterations before an error is thrown + + Returns: + optimised_transmission: float + The final transmission value which produces an acceptable total_count value + """ + LOGGER.info("Using total count optimisation") for cycle in range(0, max_cycles): @@ -292,16 +365,16 @@ def optimise_attenuation_plan( ) optimised_transmission = yield from total_counts_optimisation( - max_cycles, - initial_transmission, attenuator, xspress3mini, zebra, + initial_transmission, low_roi, high_roi, lower_limit, upper_limit, target, + max_cycles, ) elif optimisation_type == "deadtime": @@ -315,6 +388,7 @@ def optimise_attenuation_plan( initial_transmission, increment, deadtime_threshold, + max_cycles, ) yield from bps.abs_set(attenuator, optimised_transmission, group="set_transmission") diff --git a/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py b/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py index 0faeb5dd2..4dda46d44 100644 --- a/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py @@ -205,7 +205,7 @@ def test_exception_raised_after_max_cycles_reached(RE: RunEngine): with pytest.raises(AttenuationOptimisationFailedException): RE( total_counts_optimisation( - 1, 10, attenuator, xspress3mini, zebra, 0, 1, 0, 1, 5 + attenuator, xspress3mini, zebra, 1, 0, 1, 0, 1, 5, 10 ) ) From a338a1435c833c46d097d8b24c26e9b627161850 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Fri, 30 Jun 2023 14:59:23 +0100 Subject: [PATCH 27/36] Refactor of deadtime optimisation --- .../optimise_attenuation_plan.py | 129 +++++++++++------- .../tests/test_optimise_attenuation_plan.py | 26 +++- 2 files changed, 102 insertions(+), 53 deletions(-) diff --git a/src/artemis/experiment_plans/optimise_attenuation_plan.py b/src/artemis/experiment_plans/optimise_attenuation_plan.py index 5efc65e8c..6a1c8bc00 100644 --- a/src/artemis/experiment_plans/optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/optimise_attenuation_plan.py @@ -86,10 +86,17 @@ def arm_devices(xspress3mini, zebra): yield from arm_zebra(zebra) +def get_new_direction(direction: Direction, deadtime, deadtime_threshold): + if direction == Direction.POSITIVE: + if deadtime >= deadtime_threshold: + direction = Direction.NEGATIVE + return direction + + def deadtime_is_transmission_optimised( direction: Direction, deadtime, deadtime_threshold, transmission -) -> tuple[bool, bool]: - """Compares the deadtime to the deadtime_threshold and determines behavior for the next optimisation iteration +) -> bool: + """Compares the deadtime to the deadtime_threshold and checks against upper and lower bounds. If deadtime is lower than the threshold or greater than 0.9, returns (True, flip_direction). Marks the flip direction as positive if the deadtime has gone over the threshold. Raises error AttenuationOptimisationFailedException if @@ -104,35 +111,53 @@ def deadtime_is_transmission_optimised( Returns: boolean: marking whether or not attenuation is optimised - - flip_direction: Boolean - Set to true if the deadtime goes above its threshold while direction is positive. This makes deadtime decrease on the next iteration. Otherwise - set to false. """ - flip_direction = False - if direction.value == Direction.POSITIVE: - if deadtime >= deadtime_threshold: - flip_direction = True - return False, flip_direction - else: - # The 0.9 is hardcoded in GDA - if transmission >= 0.9: - return True, flip_direction + if direction == Direction.POSITIVE: + if deadtime <= deadtime_threshold: + return False else: if deadtime <= deadtime_threshold: - return True, flip_direction - return False, flip_direction + return True + return False -def deadtime_calc_new_transmission(direction: Direction, transmission, increment): - if direction.value == Direction.POSITIVE: +def calculate_new_direction(direction: Direction, deadtime, deadtime_threshold): + if direction == Direction.POSITIVE: + if deadtime > deadtime_threshold: + direction = Direction.NEGATIVE + return direction + + +def deadtime_calc_new_transmission( + direction: Direction, + transmission: float, + increment: float, + upper_transmission_limit: float, + lower_transmission_limit: float, +) -> float: + """Calculate the new transmission value based on the current direction and increment. Raise error if transmission is too low. + + Args: + direction (Direction: If positive, increase transmission by a factor of the increment. If negative, divide it + transmission (float): Current transmission value + increment (float): Factor to multiply or divide transmission by + upper_transmission_limit (float): Maximum allowed transmission, in order to protect sample. + lower_transmission_limit (float): Minimum expected transmission. Raise an error if transmission goes lower. + + Raises: + AttenuationOptimisationFailedException: _description_ + + Returns: + transmission (float): New transmission value + """ + if direction == Direction.POSITIVE: transmission *= increment - if transmission > 0.999: - transmission = 1 + if transmission > upper_transmission_limit: + transmission = upper_transmission_limit else: transmission /= increment - if transmission < 1.0e-6: + if transmission < lower_transmission_limit: raise AttenuationOptimisationFailedException( "Calculated transmission is below expected limit" ) @@ -153,6 +178,8 @@ def deadtime_optimisation( xspress3mini: Xspress3Mini, zebra: Zebra, transmission: float, + upper_transmission_limit: float, + lower_transmission_limit: float, increment: float, deadtime_threshold: float, max_cycles: int, @@ -160,29 +187,29 @@ def deadtime_optimisation( """Optimises the attenuation for the Xspress3Mini based on the detector deadtime Deadtime is the time after each event during which the detector cannot record another event. This loop adjusts the transmission of the attenuator - and checks the deadtime until the deadtime is below the accepted threshold. To protect the sample, the deadtime has a maximum value of 10% + and checks the deadtime until the deadtime is below the accepted threshold. To protect the sample, the transmission has a maximum value Args: - attenuator: Attenuator Ophyd device + attenuator: (Attenuator) Ophyd device - xspress3mini: Xspress3Mini ophyd device + xspress3mini: (Xspress3Mini) ophyd device - zebra: Zebra Ophyd device + zebra: (Zebra) Ophyd device - transmission: Float + transmission: (float) The intial transmission value to use for the optimising - increment: Float + increment: (float) The factor to increase / decrease the transmission each cycle - deadtime_threshold: Float + deadtime_threshold: (float) The maximum acceptable percentage deadtime - max_cycles: int + max_cycles: (int) The maximum number of iterations before an error is thrown Returns: - optimised_transmission: float + optimised_transmission: (float) The final transmission value which produces an acceptable deadtime """ @@ -202,11 +229,10 @@ def deadtime_optimisation( deadtime = 0 """ Deadtime is the time after each event during which the detector cannot record another event. - The reset ticks PV stops ticking while the detector is unable to process events, so the difference between the total time and the - reset ticks time gives the deadtime. Then divide by total time to get it as a percentage. - + The reset ticks PV stops ticking while the detector is unable to process events, so the absolute difference between the total time and the + reset ticks time gives the deadtime. Divide by total time to get it as a percentage. + This percentage can then be used to calculate the real counts. Eg Real counts = observed counts / (deadtime fraction) - """ if total_time != reset_ticks: @@ -214,27 +240,32 @@ def deadtime_optimisation( LOGGER.info(f"Deadtime is now at {deadtime}") - is_optimised, flip_direction = deadtime_is_transmission_optimised( - direction, deadtime, deadtime_threshold, transmission - ) - - if is_optimised: + # Check if new deadtime is OK + if deadtime <= deadtime_threshold or transmission == upper_transmission_limit: + if transmission == upper_transmission_limit: + LOGGER.warning( + f"Deadtime {deadtime} is above threshold {deadtime_threshold} at maximum transmission {upper_transmission_limit}. Using maximum transmission\ + as optimised value." + ) optimised_transmission = transmission break - if flip_direction: - direction = Direction.NEGATIVE - - transmission = deadtime_calc_new_transmission( - direction, transmission, increment - ) - if cycle == max_cycles - 1: raise AttenuationOptimisationFailedException( f"Unable to optimise attenuation after maximum cycles.\ Deadtime did not get lower than threshold: {deadtime_threshold} in maximum cycles {max_cycles}" ) + direction = calculate_new_direction(direction, deadtime, deadtime_threshold) + + transmission = deadtime_calc_new_transmission( + direction, + transmission, + increment, + upper_transmission_limit, + lower_transmission_limit, + ) + return optimised_transmission @@ -325,6 +356,8 @@ def optimise_attenuation_plan( attenuator: Attenuator, low_roi=None, high_roi=None, + upper_transmission_limit=0.9, + lower_transmission_limit=1.0e-6, ): ( _, # This is optimisation type. Beter for testing if this is a parameter of the plan instead @@ -386,6 +419,8 @@ def optimise_attenuation_plan( xspress3mini, zebra, initial_transmission, + upper_transmission_limit, + lower_transmission_limit, increment, deadtime_threshold, max_cycles, diff --git a/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py b/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py index 4dda46d44..0038a35c0 100644 --- a/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py @@ -12,6 +12,7 @@ from artemis.experiment_plans import optimise_attenuation_plan from artemis.experiment_plans.optimise_attenuation_plan import ( AttenuationOptimisationFailedException, + Direction, PlaceholderParams, arm_devices, check_parameters, @@ -196,7 +197,7 @@ def test_is_counts_within_target_is_false(total_count, lower_limit, upper_limit) assert is_counts_within_target(total_count, lower_limit, upper_limit) is False -def test_exception_raised_after_max_cycles_reached(RE: RunEngine): +def test_total_count_exception_raised_after_max_cycles_reached(RE: RunEngine): zebra, xspress3mini, attenuator = fake_create_devices() optimise_attenuation_plan.is_counts_within_target = MagicMock(return_value=False) optimise_attenuation_plan.arm_zebra = MagicMock() @@ -220,15 +221,28 @@ def test_arm_devices_runs_correct_functions(RE: RunEngine): optimise_attenuation_plan.arm_zebra.assert_called_once() -def test_deadtime_calc_new_transmission_gets_correct_value(): - assert deadtime_calc_new_transmission(True, 0.05, 2) == 0.1 - assert deadtime_calc_new_transmission(False, 0.05, 2) == 0.025 - assert deadtime_calc_new_transmission(True, 1, 2) == 1 +@pytest.mark.parametrize( + "direction, transmission, increment, upper_limit, lower_limit, new_transmission", + [ + (Direction.POSITIVE, 0.5, 2, 0.9, 1e-6, 0.9), + (Direction.POSITIVE, 0.1, 2, 0.9, 1e-6, 0.2), + (Direction.NEGATIVE, 0.8, 2, 0.9, 1e-6, 0.4), + ], +) +def test_deadtime_calc_new_transmission_gets_correct_value( + direction, transmission, increment, upper_limit, lower_limit, new_transmission +): + assert ( + deadtime_calc_new_transmission( + direction, transmission, increment, upper_limit, lower_limit + ) + == new_transmission + ) def test_deadtime_calc_new_transmission_raises_error_on_low_ransmission(): with pytest.raises(AttenuationOptimisationFailedException): - deadtime_calc_new_transmission(False, 1e-6, 2) + deadtime_calc_new_transmission(Direction.NEGATIVE, 1e-6, 2, 1, 1e-6) def test_create_new_devices(): From 92625d9834c2a6d91828250c64b2e088a7a43523 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Fri, 30 Jun 2023 15:47:59 +0100 Subject: [PATCH 28/36] Decompose unit tests --- .../optimise_attenuation_plan.py | 29 +++-- .../tests/test_optimise_attenuation_plan.py | 112 ++++++++++-------- 2 files changed, 82 insertions(+), 59 deletions(-) diff --git a/src/artemis/experiment_plans/optimise_attenuation_plan.py b/src/artemis/experiment_plans/optimise_attenuation_plan.py index 6a1c8bc00..dc264ce64 100644 --- a/src/artemis/experiment_plans/optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/optimise_attenuation_plan.py @@ -173,6 +173,23 @@ def do_device_optimise_iteration( yield from arm_devices(xspress3mini, zebra) +def is_deadtime_optimised( + deadtime: float, + deadtime_threshold: float, + transmission: float, + upper_transmission_limit: float, +) -> bool: + if deadtime <= deadtime_threshold or transmission == upper_transmission_limit: + if transmission == upper_transmission_limit: + LOGGER.warning( + f"Deadtime {deadtime} is above threshold {deadtime_threshold} at maximum transmission {upper_transmission_limit}. Using maximum transmission\ + as optimised value." + ) + return True + else: + return False + + def deadtime_optimisation( attenuator: Attenuator, xspress3mini: Xspress3Mini, @@ -232,7 +249,7 @@ def deadtime_optimisation( The reset ticks PV stops ticking while the detector is unable to process events, so the absolute difference between the total time and the reset ticks time gives the deadtime. Divide by total time to get it as a percentage. - This percentage can then be used to calculate the real counts. Eg Real counts = observed counts / (deadtime fraction) + This percentage can then be used to calculate the real counts. """ if total_time != reset_ticks: @@ -241,12 +258,10 @@ def deadtime_optimisation( LOGGER.info(f"Deadtime is now at {deadtime}") # Check if new deadtime is OK - if deadtime <= deadtime_threshold or transmission == upper_transmission_limit: - if transmission == upper_transmission_limit: - LOGGER.warning( - f"Deadtime {deadtime} is above threshold {deadtime_threshold} at maximum transmission {upper_transmission_limit}. Using maximum transmission\ - as optimised value." - ) + + if is_deadtime_optimised( + deadtime, deadtime_threshold, transmission, upper_transmission_limit + ): optimised_transmission = transmission break diff --git a/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py b/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py index 0038a35c0..686bf93cb 100644 --- a/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py @@ -15,13 +15,16 @@ Direction, PlaceholderParams, arm_devices, + calculate_new_direction, check_parameters, create_devices, deadtime_calc_new_transmission, - deadtime_is_transmission_optimised, + deadtime_optimisation, is_counts_within_target, + is_deadtime_optimised, total_counts_optimisation, ) +from artemis.log import LOGGER from artemis.parameters.beamline_parameters import get_beamline_parameters @@ -54,6 +57,7 @@ def get_good_status(): return status +@pytest.mark.skip(reason="Flakey test which is refactored in another PR") @patch("artemis.experiment_plans.optimise_attenuation_plan.arm_zebra") def test_total_count_optimise(mock_arm_zebra, RE: RunEngine): """Test the overall total count algorithm""" @@ -100,68 +104,72 @@ def mock_set_transmission(_): ) +"""LOGIC TO TEST HERE: + +check direction flip function +check deadtime is calcualed and overall function returns value + +""" + + @pytest.mark.parametrize( - "high_roi, low_roi", - [(0, 0), (0, 2048)], + "deadtime, deadtime_threshold, transmission, upper_transmission_limit, result", + [(1, 1, 0.5, 1, True), (1, 0.5, 0.9, 1, False)], ) -@patch("artemis.experiment_plans.optimise_attenuation_plan.arm_zebra") -def test_deadtime_optimise(mock_arm_zebra, high_roi, low_roi, RE: RunEngine): - """Test the overall deadtime optimisation""" +def test_is_deadtime_optimised_returns_correct_value( + deadtime, deadtime_threshold, transmission, upper_transmission_limit, result +): + assert ( + is_deadtime_optimised( + deadtime, deadtime_threshold, transmission, upper_transmission_limit + ) + == result + ) - zebra, xspress3mini, attenuator = fake_create_devices() - # Mimic some of the logic to track the transmission and set realistic data - ( - optimisation_type, - default_low_roi, - default_high_roi, - transmission, - target, - lower_limit, - upper_limit, - max_cycles, - increment, - deadtime_threshold, - ) = PlaceholderParams.from_beamline_params(get_beamline_parameters()) +def test_is_deadtime_is_optimised_logs_warning_when_upper_transmission_limit_is_reached(): + LOGGER.warning = MagicMock() + is_deadtime_optimised(0.5, 0.4, 0.9, 0.9) + LOGGER.warning.assert_called_once() - """ Similar to test_total_count, mimic the set transmission. For now, just assume total time is constant and increasing the transmission will increase the - reset ticks, thus decreasing the deadtime""" - transmission_list = [transmission] - direction_list = [True] - total_time = 8e7 - # Put realistic values into PV's - xspress3mini.channel_1.total_time.sim_put(8e7) - xspress3mini.channel_1.reset_ticks.sim_put(151276) +@pytest.mark.parametrize( + "old_direction, deadtime, deadtime_threshold, new_direction", + [ + (Direction.POSITIVE, 0.1, 0.9, Direction.POSITIVE), + (Direction.NEGATIVE, 0.5, 0.4, Direction.NEGATIVE), + ], +) +def test_calculate_new_direction_gives_correct_value( + old_direction, deadtime, deadtime_threshold, new_direction +): + assert ( + calculate_new_direction(old_direction, deadtime, deadtime_threshold) + == new_direction + ) - def mock_set_transmission(_): - # Update reset ticks, calc new deadtime, increment transmission. - reset_ticks = 151276 * transmission_list[0] - deadtime = 0 - if total_time != reset_ticks: - deadtime = 1 - abs(float(total_time - reset_ticks)) / float(total_time) - - _, direction_flip = deadtime_is_transmission_optimised( - direction_list[0], deadtime, deadtime_threshold, transmission - ) - if direction_flip: - direction_list[0] = not direction_list[0] - if direction_list[0]: - transmission_list[0] *= increment - else: - transmission_list[0] /= increment +@patch( + "artemis.experiment_plans.optimise_attenuation_plan.do_device_optimise_iteration" +) +def test_deadtime_optimisation_calculates_deadtime_correctly( + mock_do_device_optimise_iteration, RE: RunEngine +): + zebra, xspress3mini, attenuator = fake_create_devices() - return get_good_status() + xspress3mini.channel_1.total_time.sim_put(100) + xspress3mini.channel_1.reset_ticks.sim_put(101) + is_deadtime_optimised.return_value = True - attenuator.desired_transmission.set = mock_set_transmission - # Force xspress3mini to pass arming - xspress3mini.detector_state.sim_put(DetectorState.ACQUIRE.value) - RE( - optimise_attenuation_plan.optimise_attenuation_plan( - 5, "deadtime", xspress3mini, zebra, attenuator, high_roi, low_roi + with patch( + "artemis.experiment_plans.optimise_attenuation_plan.is_deadtime_optimised" + ) as mock_is_deadtime_optimised: + RE( + deadtime_optimisation( + attenuator, xspress3mini, zebra, 0.5, 0.9, 1e-6, 1.2, 0.01, 2 + ) ) - ) + mock_is_deadtime_optimised.assert_called_with(0.99, 0.01, 0.5, 0.9) @pytest.mark.parametrize( From b9acee6a57513752eb6908f9e8969e5c76247023 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Fri, 30 Jun 2023 15:52:37 +0100 Subject: [PATCH 29/36] dodal url --- .../tests/test_optimise_attenuation_plan.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py b/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py index 686bf93cb..cff299e96 100644 --- a/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/tests/test_optimise_attenuation_plan.py @@ -104,14 +104,6 @@ def mock_set_transmission(_): ) -"""LOGIC TO TEST HERE: - -check direction flip function -check deadtime is calcualed and overall function returns value - -""" - - @pytest.mark.parametrize( "deadtime, deadtime_threshold, transmission, upper_transmission_limit, result", [(1, 1, 0.5, 1, True), (1, 0.5, 0.9, 1, False)], From 4d8e756d87a7b8abe5d834bf9185678c682dca0a Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Fri, 30 Jun 2023 16:36:16 +0100 Subject: [PATCH 30/36] fix tests --- src/artemis/experiment_plans/tests/test_grid_detection_plan.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/artemis/experiment_plans/tests/test_grid_detection_plan.py b/src/artemis/experiment_plans/tests/test_grid_detection_plan.py index b14463f0b..3799a39d5 100644 --- a/src/artemis/experiment_plans/tests/test_grid_detection_plan.py +++ b/src/artemis/experiment_plans/tests/test_grid_detection_plan.py @@ -128,7 +128,7 @@ def test_create_devices(create_device: MagicMock): create_device.assert_has_calls( [ call(Smargon, "smargon", "-MO-SGON-01:", True, False), - call(OAV, "oav", "", True, False), + call(OAV, "oav", "-DI-OAV-01:", True, False), call( device=Backlight, name="backlight", From 76c9209852cbc83b83ec50a7bd73d2d5ce7195a2 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Wed, 28 Jun 2023 13:19:10 +0100 Subject: [PATCH 31/36] update dodal url --- .../optimise_attenuation_plan.py | 29 ------------------- 1 file changed, 29 deletions(-) diff --git a/src/artemis/experiment_plans/optimise_attenuation_plan.py b/src/artemis/experiment_plans/optimise_attenuation_plan.py index dc264ce64..25e633f8d 100644 --- a/src/artemis/experiment_plans/optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/optimise_attenuation_plan.py @@ -93,35 +93,6 @@ def get_new_direction(direction: Direction, deadtime, deadtime_threshold): return direction -def deadtime_is_transmission_optimised( - direction: Direction, deadtime, deadtime_threshold, transmission -) -> bool: - """Compares the deadtime to the deadtime_threshold and checks against upper and lower bounds. - - If deadtime is lower than the threshold or greater than 0.9, returns (True, flip_direction). - Marks the flip direction as positive if the deadtime has gone over the threshold. Raises error AttenuationOptimisationFailedException if - transmission goes too low - - Args: - direction: Enum - Enum taking values of either POSITIVE or NEGATIVE, which determines whether the transmission should be increased or decreased in the next iteration - - deadtime: - Current deadtime value - - Returns: - boolean: marking whether or not attenuation is optimised - """ - - if direction == Direction.POSITIVE: - if deadtime <= deadtime_threshold: - return False - else: - if deadtime <= deadtime_threshold: - return True - return False - - def calculate_new_direction(direction: Direction, deadtime, deadtime_threshold): if direction == Direction.POSITIVE: if deadtime > deadtime_threshold: From bd024a0db78df928cdb262851c40291fb13a2df2 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Mon, 3 Jul 2023 17:04:22 +0100 Subject: [PATCH 32/36] adjust comments and wait for optimised transmission --- .../optimise_attenuation_plan.py | 48 ++++++++++++------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/src/artemis/experiment_plans/optimise_attenuation_plan.py b/src/artemis/experiment_plans/optimise_attenuation_plan.py index 25e633f8d..c267cdd82 100644 --- a/src/artemis/experiment_plans/optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/optimise_attenuation_plan.py @@ -110,14 +110,24 @@ def deadtime_calc_new_transmission( """Calculate the new transmission value based on the current direction and increment. Raise error if transmission is too low. Args: - direction (Direction: If positive, increase transmission by a factor of the increment. If negative, divide it - transmission (float): Current transmission value - increment (float): Factor to multiply or divide transmission by - upper_transmission_limit (float): Maximum allowed transmission, in order to protect sample. - lower_transmission_limit (float): Minimum expected transmission. Raise an error if transmission goes lower. + direction (Direction): + If positive, increase transmission by a factor of the increment. If negative, divide it + + transmission (float): + Current transmission value + + increment (float): + Factor to multiply or divide transmission by + + upper_transmission_limit (float): + Maximum allowed transmission, in order to protect sample. + + lower_transmission_limit (float): + Minimum expected transmission. Raise an error if transmission goes lower. Raises: - AttenuationOptimisationFailedException: _description_ + AttenuationOptimisationFailedException: + This error is thrown if the transmission goes below the expected value Returns: transmission (float): New transmission value @@ -188,7 +198,7 @@ def deadtime_optimisation( The intial transmission value to use for the optimising increment: (float) - The factor to increase / decrease the transmission each cycle + The factor to increase / decrease the transmission by each iteration deadtime_threshold: (float) The maximum acceptable percentage deadtime @@ -273,29 +283,29 @@ def total_counts_optimisation( defined by the lower and upper limit. To protect the sample, the transmission has a maximum value of 10%. Args: - attenuator: Attenuator Ophyd device + attenuator: (Attenuator) Ophyd device - xspress3mini: Xspress3Mini ophyd device + xspress3mini: (Xspress3Mini) ophyd device - zebra: Zebra Ophyd device + zebra: (Zebra) Ophyd device - transmission: Float + transmission: (float) The intial transmission value to use for the optimising - low_roi: Float + low_roi: (float) Lower region of interest at which to include in the counts - high_roi: Float + high_roi: (float) Upper region of interest at which to include in the counts - target_count: int + target_count: (int) The ideal number of target counts - used to calculate the transmission for the subsequent iteration. - max_cycles: int + max_cycles: (int) The maximum number of iterations before an error is thrown Returns: - optimised_transmission: float + optimised_transmission: (float) The final transmission value which produces an acceptable total_count value """ @@ -346,7 +356,7 @@ def optimise_attenuation_plan( lower_transmission_limit=1.0e-6, ): ( - _, # This is optimisation type. Beter for testing if this is a parameter of the plan instead + _, # This is optimisation type. While we can get it from GDA, it's better for testing if this is a parameter of the plan instead default_low_roi, default_high_roi, initial_transmission, @@ -412,6 +422,8 @@ def optimise_attenuation_plan( max_cycles, ) - yield from bps.abs_set(attenuator, optimised_transmission, group="set_transmission") + yield from bps.abs_set( + attenuator, optimised_transmission, group="set_transmission", wait=True + ) return optimised_transmission From 9c1e6e67be028bd9e0415c4b94786800ebf8d1ea Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Tue, 4 Jul 2023 16:09:35 +0100 Subject: [PATCH 33/36] (#774) Add pydantic explicitly to artemis --- setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.cfg b/setup.cfg index c8aa9fdc3..d4dc4e054 100644 --- a/setup.cfg +++ b/setup.cfg @@ -37,6 +37,7 @@ install_requires = doct databroker dodal @ git+https://github.com/DiamondLightSource/python-dodal.git@6c9b4a29b412da6ef62553d86ae705236baac77d + pydantic<2.0 # See https://github.com/DiamondLightSource/python-artemis/issues/774 [options.extras_require] dev = From 29f11668c011ce3a4b1a8456502ddcf8660049eb Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Tue, 4 Jul 2023 16:13:58 +0100 Subject: [PATCH 34/36] Update dodal --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 2efc1f450..b91559a7e 100644 --- a/setup.cfg +++ b/setup.cfg @@ -36,7 +36,7 @@ install_requires = xarray doct databroker - dodal @ git+https://github.com/DiamondLightSource/python-dodal.git@187da1fe34f0da7b88f7821b6c0b935c377ab9a7 + dodal @ git+https://github.com/DiamondLightSource/python-dodal.git@d6529d2d9d1883d87bbd167c1ba3787eb578f7f4 [options.extras_require] dev = From a3a7d5b29053eeed8506a6eca82f67e0bb35e640 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Tue, 4 Jul 2023 16:56:28 +0100 Subject: [PATCH 35/36] (#754) Fix review comment --- setup.cfg | 3 ++- src/artemis/experiment_plans/optimise_attenuation_plan.py | 7 ------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/setup.cfg b/setup.cfg index c8aa9fdc3..74623804f 100644 --- a/setup.cfg +++ b/setup.cfg @@ -36,7 +36,8 @@ install_requires = xarray doct databroker - dodal @ git+https://github.com/DiamondLightSource/python-dodal.git@6c9b4a29b412da6ef62553d86ae705236baac77d + dodal @ git+https://github.com/DiamondLightSource/python-dodal.git@4337a5f797dd7e75122f05aad946ae71fe4bcf46 + [options.extras_require] dev = diff --git a/src/artemis/experiment_plans/optimise_attenuation_plan.py b/src/artemis/experiment_plans/optimise_attenuation_plan.py index c267cdd82..2e6c4a473 100644 --- a/src/artemis/experiment_plans/optimise_attenuation_plan.py +++ b/src/artemis/experiment_plans/optimise_attenuation_plan.py @@ -86,13 +86,6 @@ def arm_devices(xspress3mini, zebra): yield from arm_zebra(zebra) -def get_new_direction(direction: Direction, deadtime, deadtime_threshold): - if direction == Direction.POSITIVE: - if deadtime >= deadtime_threshold: - direction = Direction.NEGATIVE - return direction - - def calculate_new_direction(direction: Direction, deadtime, deadtime_threshold): if direction == Direction.POSITIVE: if deadtime > deadtime_threshold: From 93e800e5ae4c8d4e0c58e90df5e193e63962bfac Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Tue, 4 Jul 2023 17:24:43 +0100 Subject: [PATCH 36/36] (#754) Fix tests --- .../experiment_plans/tests/test_grid_detection_plan.py | 2 +- src/artemis/system_tests/test_main_system.py | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/artemis/experiment_plans/tests/test_grid_detection_plan.py b/src/artemis/experiment_plans/tests/test_grid_detection_plan.py index 3799a39d5..b14463f0b 100644 --- a/src/artemis/experiment_plans/tests/test_grid_detection_plan.py +++ b/src/artemis/experiment_plans/tests/test_grid_detection_plan.py @@ -128,7 +128,7 @@ def test_create_devices(create_device: MagicMock): create_device.assert_has_calls( [ call(Smargon, "smargon", "-MO-SGON-01:", True, False), - call(OAV, "oav", "-DI-OAV-01:", True, False), + call(OAV, "oav", "", True, False), call( device=Backlight, name="backlight", diff --git a/src/artemis/system_tests/test_main_system.py b/src/artemis/system_tests/test_main_system.py index 4040e13c6..cd40a39b4 100644 --- a/src/artemis/system_tests/test_main_system.py +++ b/src/artemis/system_tests/test_main_system.py @@ -433,7 +433,12 @@ def test_log_on_invalid_json_params(test_env: ClientAndRunEngine): assert response.get("exception_type") == "ValidationError" -def test_warn_exception_during_plan_causes_warning_in_log(caplog, test_env): +@pytest.mark.skip( + reason="See https://github.com/DiamondLightSource/python-artemis/issues/777" +) +def test_warn_exception_during_plan_causes_warning_in_log( + caplog, test_env: ClientAndRunEngine +): test_env.client.put(START_ENDPOINT, data=TEST_PARAMS) test_env.mock_run_engine.error = WarningException("D'Oh") response_json = wait_for_run_engine_status(test_env.client)