From e012ea934f903268030572cfe7d8f643b1d4ea08 Mon Sep 17 00:00:00 2001 From: Richard Gildea Date: Mon, 23 Jan 2023 13:05:31 +0000 Subject: [PATCH 01/14] Zocalo no longer returns reversed coords See python-dlstbx#196 --- .../external_interaction/unit_tests/test_zocalo_interaction.py | 2 +- src/artemis/external_interaction/zocalo/zocalo_interaction.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/artemis/external_interaction/unit_tests/test_zocalo_interaction.py b/src/artemis/external_interaction/unit_tests/test_zocalo_interaction.py index ac1c6d9a0..74ecfe3cd 100644 --- a/src/artemis/external_interaction/unit_tests/test_zocalo_interaction.py +++ b/src/artemis/external_interaction/unit_tests/test_zocalo_interaction.py @@ -134,4 +134,4 @@ def test_when_message_recieved_from_zocalo_then_point_returned( return_value = future.result() assert type(return_value) == Point3D - assert return_value == Point3D(*reversed(centre_of_mass_coords)) + assert return_value == Point3D(*centre_of_mass_coords) diff --git a/src/artemis/external_interaction/zocalo/zocalo_interaction.py b/src/artemis/external_interaction/zocalo/zocalo_interaction.py index 2b4eee46a..b644ee59a 100644 --- a/src/artemis/external_interaction/zocalo/zocalo_interaction.py +++ b/src/artemis/external_interaction/zocalo/zocalo_interaction.py @@ -99,7 +99,7 @@ def receive_result( transport.ack(header) received_group_id = recipe_parameters["dcgid"] if received_group_id == str(data_collection_group_id): - result_received.put(Point3D(*reversed(message[0]["centre_of_mass"]))) + result_received.put(Point3D(*message[0]["centre_of_mass"])) else: artemis.log.LOGGER.warning( f"Warning: results for {received_group_id} received but expected \ From 03ca0e295c059ff136f74f6c9b980ae91a837f55 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Tue, 24 Jan 2023 16:14:39 +0000 Subject: [PATCH 02/14] (#438) Reset FGS position counter on start up --- src/artemis/devices/fast_grid_scan.py | 8 ++------ src/artemis/devices/system_tests/test_gridscan_system.py | 8 ++++++-- src/artemis/experiment_plans/fast_grid_scan_plan.py | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/artemis/devices/fast_grid_scan.py b/src/artemis/devices/fast_grid_scan.py index 77cbb3edc..519d55885 100644 --- a/src/artemis/devices/fast_grid_scan.py +++ b/src/artemis/devices/fast_grid_scan.py @@ -1,7 +1,6 @@ import threading import time from dataclasses import dataclass -from typing import List from bluesky.plan_stubs import mv from dataclasses_json import dataclass_json @@ -14,7 +13,6 @@ Signal, ) from ophyd.status import DeviceStatus, StatusBase -from ophyd.utils.epics_pvs import set_and_wait from artemis.devices.motors import XYZLimitBundle from artemis.devices.status import await_value @@ -244,10 +242,6 @@ def scan(): threading.Thread(target=scan, daemon=True).start() return st - def stage(self) -> List[object]: - set_and_wait(self.position_counter, 0) - return super().stage() - def complete(self) -> DeviceStatus: return GridScanCompleteStatus(self) @@ -284,4 +278,6 @@ def set_fast_grid_scan_params(scan: FastGridScan, params: GridScanParams): params.z1_start, scan.z2_start, params.z2_start, + scan.position_counter, + 0, ) diff --git a/src/artemis/devices/system_tests/test_gridscan_system.py b/src/artemis/devices/system_tests/test_gridscan_system.py index a0e1eefb8..12f42abdf 100644 --- a/src/artemis/devices/system_tests/test_gridscan_system.py +++ b/src/artemis/devices/system_tests/test_gridscan_system.py @@ -6,6 +6,7 @@ GridScanParams, set_fast_grid_scan_params, ) +from artemis.experiment_plans.fast_grid_scan_plan import wait_for_fgs_valid @pytest.fixture() @@ -29,6 +30,10 @@ def test_when_program_data_set_and_staged_then_expected_images_correct( def test_given_valid_params_when_kickoff_then_completion_status_increases_and_finishes( fast_grid_scan: FastGridScan, ): + def set_and_wait_plan(fast_grid_scan: FastGridScan): + yield from set_fast_grid_scan_params(fast_grid_scan, GridScanParams(3, 3)) + yield from wait_for_fgs_valid(fast_grid_scan) + prev_current, prev_fraction = None, None def progress_watcher(*args, **kwargs): @@ -44,8 +49,7 @@ def progress_watcher(*args, **kwargs): assert 0 < prev_fraction < 1 RE = RunEngine() - RE(set_fast_grid_scan_params(fast_grid_scan, GridScanParams(3, 3))) - fast_grid_scan.stage() + RE(set_and_wait_plan(fast_grid_scan)) assert fast_grid_scan.position_counter.get() == 0 # S03 currently is giving 2* the number of expected images (see #13) diff --git a/src/artemis/experiment_plans/fast_grid_scan_plan.py b/src/artemis/experiment_plans/fast_grid_scan_plan.py index 30fa9780d..a2260799b 100644 --- a/src/artemis/experiment_plans/fast_grid_scan_plan.py +++ b/src/artemis/experiment_plans/fast_grid_scan_plan.py @@ -110,7 +110,7 @@ def run_gridscan( yield from set_fast_grid_scan_params(fgs_motors, parameters.grid_scan_params) yield from wait_for_fgs_valid(fgs_motors) - @bpp.stage_decorator([eiger, fgs_motors]) + @bpp.stage_decorator([eiger]) def do_fgs(): yield from bps.wait() # Wait for all moves to complete yield from bps.kickoff(fgs_motors) From db53f66fd6389d29deb0e2aa3122fccece7d7287 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Tue, 24 Jan 2023 18:05:18 +0000 Subject: [PATCH 03/14] (#451) Initial ideas for creating devices before running plans --- src/artemis/__main__.py | 4 +- src/artemis/devices/eiger.py | 10 ++-- .../experiment_plans/experiment_registry.py | 7 ++- .../experiment_plans/fast_grid_scan_plan.py | 53 +++++++++++++------ .../fgs/tests/test_fgs_callback_collection.py | 5 +- src/artemis/parameters.py | 15 ++++++ src/artemis/system_tests/test_fgs_plan.py | 16 +++--- 7 files changed, 75 insertions(+), 35 deletions(-) diff --git a/src/artemis/__main__.py b/src/artemis/__main__.py index 81e9178de..3e9ef47f1 100644 --- a/src/artemis/__main__.py +++ b/src/artemis/__main__.py @@ -69,6 +69,8 @@ def __init__(self, RE: RunEngine) -> None: self.RE = RE if VERBOSE_EVENT_LOGGING: RE.subscribe(VerbosePlanExecutionLoggingCallback()) + for plan in PLAN_REGISTRY: + PLAN_REGISTRY[plan]["setup"]() def start( self, experiment: Callable, parameters: FullParameters @@ -144,7 +146,7 @@ def put(self, experiment: str, action: Actions): status_and_message = StatusAndMessage(Status.FAILED, f"{action} not understood") if action == Actions.START.value: try: - plan = PLAN_REGISTRY.get(experiment) + plan = PLAN_REGISTRY.get(experiment).get("run") if plan is None: raise PlanNotFound( f"Experiment plan '{experiment}' not found in registry." diff --git a/src/artemis/devices/eiger.py b/src/artemis/devices/eiger.py index c6673dcea..73b4d12b7 100644 --- a/src/artemis/devices/eiger.py +++ b/src/artemis/devices/eiger.py @@ -28,14 +28,10 @@ class EigerDetector(Device): filewriters_finished: StatusBase - def __init__( - self, detector_params: DetectorParams, name="Eiger Detector", *args, **kwargs - ): - super().__init__(name=name, *args, **kwargs) - self.detector_params = detector_params - self.check_detector_variables_set() + detector_params = None - def check_detector_variables_set(self): + def set_detector_parameters(self, detector_params: DetectorParams): + self.detector_params = detector_params if self.detector_params is None: raise Exception("Parameters for scan must be specified") diff --git a/src/artemis/experiment_plans/experiment_registry.py b/src/artemis/experiment_plans/experiment_registry.py index 71cde6dd1..2940647ae 100644 --- a/src/artemis/experiment_plans/experiment_registry.py +++ b/src/artemis/experiment_plans/experiment_registry.py @@ -2,7 +2,12 @@ from artemis.experiment_plans import fast_grid_scan_plan -PLAN_REGISTRY: Dict[str, Callable] = {"fast_grid_scan": fast_grid_scan_plan.get_plan} +PLAN_REGISTRY: Dict[str, Dict[str, Callable]] = { + "fast_grid_scan": { + "setup": fast_grid_scan_plan.create_devices, + "run": fast_grid_scan_plan.get_plan, + } +} class PlanNotFound(Exception): diff --git a/src/artemis/experiment_plans/fast_grid_scan_plan.py b/src/artemis/experiment_plans/fast_grid_scan_plan.py index 30fa9780d..c6a219f55 100644 --- a/src/artemis/experiment_plans/fast_grid_scan_plan.py +++ b/src/artemis/experiment_plans/fast_grid_scan_plan.py @@ -19,10 +19,18 @@ from artemis.devices.undulator import Undulator from artemis.exceptions import WarningException from artemis.external_interaction.callbacks import FGSCallbackCollection -from artemis.parameters import ISPYB_PLAN_NAME, SIM_BEAMLINE, FullParameters +from artemis.parameters import ( + ISPYB_PLAN_NAME, + SIM_BEAMLINE, + FullParameters, + get_beamline_prefixes, +) from artemis.tracing import TRACER from artemis.utils import Point3D +fast_grid_scan_composite: FGSComposite = None +eiger: EigerDetector = None + def read_hardware_for_ispyb( undulator: Undulator, @@ -164,35 +172,44 @@ def gridscan_with_subscriptions(fgs_composite, detector, params): ) -def get_plan( - parameters: FullParameters, subscriptions: FGSCallbackCollection -) -> Callable: - """Create the plan to run the grid scan based on provided parameters. - - Args: - parameters (FullParameters): The parameters to run the scan. - - Returns: - Generator: The plan for the gridscan - """ - artemis.log.LOGGER.info("Fetching composite plan") +def create_devices(): + """Creates the devices required for the plan and connect to them""" + global fast_grid_scan_composite, eiger + prefixes = get_beamline_prefixes() + artemis.log.LOGGER.info( + f"Creating devices for {prefixes.beamline_prefix} and {prefixes.insertion_prefix}" + ) fast_grid_scan_composite = FGSComposite( - insertion_prefix=parameters.insertion_prefix, + insertion_prefix=prefixes.insertion_prefix, name="fgs", - prefix=parameters.beamline, + prefix=prefixes.beamline_prefix, ) # Note, eiger cannot be currently waited on, see #166 eiger = EigerDetector( - parameters.detector_params, name="eiger", - prefix=f"{parameters.beamline}-EA-EIGER-01:", + prefix=f"{prefixes.beamline_prefix}-EA-EIGER-01:", ) artemis.log.LOGGER.info("Connecting to EPICS devices...") fast_grid_scan_composite.wait_for_connection() artemis.log.LOGGER.info("Connected.") + +def get_plan( + parameters: FullParameters, + subscriptions: FGSCallbackCollection, +) -> Callable: + """Create the plan to run the grid scan based on provided parameters. + + Args: + parameters (FullParameters): The parameters to run the scan. + + Returns: + Generator: The plan for the gridscan + """ + eiger.set_detector_parameters(parameters.detector_params) + @bpp.finalize_decorator(lambda: tidy_up_plans(fast_grid_scan_composite)) def run_gridscan_and_move_and_tidy(fgs_composite, detector, params, comms): yield from run_gridscan_and_move(fgs_composite, detector, params, comms) @@ -217,4 +234,6 @@ def run_gridscan_and_move_and_tidy(fgs_composite, detector, params, comms): parameters = FullParameters(beamline=args.beamline) subscriptions = FGSCallbackCollection.from_params(parameters) + create_devices() + RE(get_plan(parameters, subscriptions)) diff --git a/src/artemis/external_interaction/callbacks/fgs/tests/test_fgs_callback_collection.py b/src/artemis/external_interaction/callbacks/fgs/tests/test_fgs_callback_collection.py index eadf20767..77dab0e20 100644 --- a/src/artemis/external_interaction/callbacks/fgs/tests/test_fgs_callback_collection.py +++ b/src/artemis/external_interaction/callbacks/fgs/tests/test_fgs_callback_collection.py @@ -16,6 +16,7 @@ from artemis.parameters import ( ISPYB_PLAN_NAME, SIM_BEAMLINE, + SIM_INSERTION_PREFIX, DetectorParams, FullParameters, ) @@ -150,9 +151,7 @@ def test_communicator_in_composite_run( callbacks.zocalo_handler.xray_centre_motor_position = Point3D(1, 2, 3) fast_grid_scan_composite = FGSComposite( - insertion_prefix=params.insertion_prefix, - name="fgs", - prefix=params.beamline, + insertion_prefix=SIM_INSERTION_PREFIX, name="fgs", prefix=SIM_BEAMLINE ) # this is where it's currently getting stuck: # fast_grid_scan_composite.fast_grid_scan.is_invalid = lambda: False diff --git a/src/artemis/parameters.py b/src/artemis/parameters.py index f4c0790e7..5c6f06349 100644 --- a/src/artemis/parameters.py +++ b/src/artemis/parameters.py @@ -1,5 +1,6 @@ import copy from dataclasses import dataclass, field +from os import environ from dataclasses_json import dataclass_json @@ -19,6 +20,20 @@ def default_field(obj): return field(default_factory=lambda: copy.deepcopy(obj)) +@dataclass +class BeamlinePrefixes: + beamline_prefix: str + insertion_prefix: str + + +def get_beamline_prefixes(): + beamline = environ.get("BEAMLINE") + if beamline is None: + return BeamlinePrefixes(SIM_BEAMLINE, SIM_INSERTION_PREFIX) + if beamline == "i03": + return BeamlinePrefixes("BL03I", "SR03I") + + @dataclass_json @dataclass class FullParameters: diff --git a/src/artemis/system_tests/test_fgs_plan.py b/src/artemis/system_tests/test_fgs_plan.py index 89cb8e0e0..6ff67ad45 100644 --- a/src/artemis/system_tests/test_fgs_plan.py +++ b/src/artemis/system_tests/test_fgs_plan.py @@ -12,7 +12,12 @@ run_gridscan, ) from artemis.external_interaction.callbacks import FGSCallbackCollection -from artemis.parameters import SIM_BEAMLINE, DetectorParams, FullParameters +from artemis.parameters import ( + SIM_BEAMLINE, + SIM_INSERTION_PREFIX, + DetectorParams, + FullParameters, +) @pytest.fixture() @@ -45,7 +50,6 @@ def eiger() -> EigerDetector: params = FullParameters() -params.beamline = SIM_BEAMLINE @pytest.fixture @@ -56,9 +60,9 @@ def RE(): @pytest.fixture def fgs_composite(): fast_grid_scan_composite = FGSComposite( - insertion_prefix=params.insertion_prefix, + insertion_prefix=SIM_INSERTION_PREFIX, name="fgs", - prefix=params.beamline, + prefix=SIM_BEAMLINE, ) return fast_grid_scan_composite @@ -120,7 +124,7 @@ def test_full_plan_tidies_at_end( fgs_composite: FGSComposite, ): callbacks = FGSCallbackCollection.from_params(FullParameters()) - RE(get_plan(params, callbacks)) + RE(get_plan(fgs_composite, eiger, params, callbacks)) set_shutter_to_manual.assert_called_once() @@ -143,6 +147,6 @@ def test_full_plan_tidies_at_end_when_plan_fails( callbacks = FGSCallbackCollection.from_params(FullParameters()) run_gridscan_and_move.side_effect = Exception() with pytest.raises(Exception): - RE(get_plan(params, callbacks)) + RE(get_plan(fgs_composite, eiger, params, callbacks)) set_shutter_to_manual.assert_called_once() # tidy_plans.assert_called_once() From d38cb136516366a23d424b0cb249a4c35cba86bd Mon Sep 17 00:00:00 2001 From: David Perl Date: Wed, 25 Jan 2023 09:24:55 +0000 Subject: [PATCH 04/14] improve eiger detectorparams type safety --- src/artemis/devices/eiger.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/artemis/devices/eiger.py b/src/artemis/devices/eiger.py index 73b4d12b7..4a8aec236 100644 --- a/src/artemis/devices/eiger.py +++ b/src/artemis/devices/eiger.py @@ -1,4 +1,5 @@ from enum import Enum +from typing import Optional from ophyd import Component, Device, EpicsSignalRO, StatusBase from ophyd.areadetector.cam import EigerDetectorCam @@ -28,7 +29,7 @@ class EigerDetector(Device): filewriters_finished: StatusBase - detector_params = None + detector_params: Optional[DetectorParams] = None def set_detector_parameters(self, detector_params: DetectorParams): self.detector_params = detector_params @@ -84,6 +85,7 @@ def disable_roi_mode(self): self.change_roi_mode(False) def change_roi_mode(self, enable: bool): + assert self.detector_params is not None detector_dimensions = ( self.detector_params.detector_size_constants.roi_size_pixels if enable @@ -102,6 +104,7 @@ def change_roi_mode(self, enable: bool): self.log.error("Failed to switch to ROI mode") def set_cam_pvs(self) -> Status: + assert self.detector_params is not None status = self.cam.acquire_time.set(self.detector_params.exposure_time) status &= self.cam.acquire_period.set(self.detector_params.exposure_time) status &= self.cam.num_exposures.set(1) @@ -125,6 +128,7 @@ def set_odin_pvs(self): return odin_status def set_mx_settings_pvs(self) -> Status: + assert self.detector_params is not None beam_x_pixels, beam_y_pixels = self.detector_params.get_beam_position_pixels( self.detector_params.detector_distance ) @@ -155,6 +159,7 @@ def set_detector_threshold(self, energy: float, tolerance: float = 0.1) -> Statu return status def set_num_triggers_and_captures(self) -> Status: + assert self.detector_params is not None status = self.cam.num_images.set(1) status &= self.cam.num_triggers.set(self.detector_params.num_images) status &= self.odin.file_writer.num_capture.set(self.detector_params.num_images) From 500f0e5dde3b7b48e056333a20b3bd25d8b7aa55 Mon Sep 17 00:00:00 2001 From: David Perl Date: Wed, 25 Jan 2023 09:42:10 +0000 Subject: [PATCH 05/14] add eiger init classmethod with params and fix most eiger unit tests --- src/artemis/devices/eiger.py | 26 ++++++++++++++++++++ src/artemis/devices/unit_tests/test_eiger.py | 16 ++++++------ src/artemis/parameters.py | 16 ++---------- 3 files changed, 36 insertions(+), 22 deletions(-) diff --git a/src/artemis/devices/eiger.py b/src/artemis/devices/eiger.py index 4a8aec236..f56363de3 100644 --- a/src/artemis/devices/eiger.py +++ b/src/artemis/devices/eiger.py @@ -10,6 +10,20 @@ from artemis.devices.status import await_value from artemis.log import LOGGER +DETECTOR_PARAM_DEFAULTS = { + "current_energy": 100, + "exposure_time": 0.1, + "directory": "/tmp", + "prefix": "file_name", + "run_number": 0, + "detector_distance": 100.0, + "omega_start": 0.0, + "omega_increment": 0.0, + "num_images": 2000, + "use_roi_mode": False, + "det_dist_to_beam_converter_path": "src/artemis/devices/unit_tests/test_lookup_table.txt", +} + class EigerTriggerMode(Enum): INTERNAL_SERIES = 0 @@ -31,6 +45,18 @@ class EigerDetector(Device): detector_params: Optional[DetectorParams] = None + @classmethod + def with_params( + cls, + params: DetectorParams = DetectorParams(**DETECTOR_PARAM_DEFAULTS), + name: str = "EigerDetector", + *args, + **kwargs, + ): + det = cls(name=name, *args, **kwargs) + det.set_detector_parameters(params) + return det + def set_detector_parameters(self, detector_params: DetectorParams): self.detector_params = detector_params if self.detector_params is None: diff --git a/src/artemis/devices/unit_tests/test_eiger.py b/src/artemis/devices/unit_tests/test_eiger.py index e77f99745..d20f87628 100644 --- a/src/artemis/devices/unit_tests/test_eiger.py +++ b/src/artemis/devices/unit_tests/test_eiger.py @@ -44,9 +44,9 @@ @pytest.fixture def fake_eiger(): - FakeEigerDetector = make_fake_device(EigerDetector) - fake_eiger: EigerDetector = FakeEigerDetector( - detector_params=TEST_DETECTOR_PARAMS, name="test" + FakeEigerDetector: EigerDetector = make_fake_device(EigerDetector) + fake_eiger: EigerDetector = FakeEigerDetector.with_params( + params=TEST_DETECTOR_PARAMS, name="test" ) return fake_eiger @@ -94,13 +94,13 @@ def test_detector_threshold( ], ) def test_check_detector_variables( - fake_eiger, - detector_params, + fake_eiger: EigerDetector, + detector_params: DetectorParams, detector_size_constants, beam_xy_converter, expected_error_number, ): - fake_eiger.detector_params = detector_params + fake_eiger.set_detector_parameters(detector_params) if detector_params is not None: fake_eiger.detector_params.beam_xy_converter = beam_xy_converter @@ -108,13 +108,13 @@ def test_check_detector_variables( if expected_error_number != 0: with pytest.raises(Exception) as e: - fake_eiger.check_detector_variables_set() + fake_eiger.set_detector_parameters() number_of_errors = str(e.value).count("\n") + 1 assert number_of_errors == expected_error_number else: try: - fake_eiger.check_detector_variables_set() + fake_eiger.set_detector_parameters() except Exception as e: assert False, f"exception was raised {e}" diff --git a/src/artemis/parameters.py b/src/artemis/parameters.py index 5c6f06349..f12fc7b80 100644 --- a/src/artemis/parameters.py +++ b/src/artemis/parameters.py @@ -4,7 +4,7 @@ from dataclasses_json import dataclass_json -from artemis.devices.eiger import DetectorParams +from artemis.devices.eiger import DETECTOR_PARAM_DEFAULTS, DetectorParams from artemis.devices.fast_grid_scan import GridScanParams from artemis.external_interaction.ispyb.ispyb_dataclass import IspybParams from artemis.utils import Point3D @@ -57,19 +57,7 @@ class FullParameters: ) ) detector_params: DetectorParams = default_field( - DetectorParams( - current_energy=100, - exposure_time=0.1, - directory="/tmp", - prefix="file_name", - run_number=0, - detector_distance=100.0, - omega_start=0.0, - omega_increment=0.0, - num_images=2000, - use_roi_mode=False, - det_dist_to_beam_converter_path="src/artemis/devices/unit_tests/test_lookup_table.txt", - ) + DetectorParams(**DETECTOR_PARAM_DEFAULTS) ) ispyb_params: IspybParams = default_field( IspybParams( From bf037137fb753b8583cf8eb8b803855527255d78 Mon Sep 17 00:00:00 2001 From: David Perl Date: Wed, 25 Jan 2023 09:48:16 +0000 Subject: [PATCH 06/14] check experiment and run plan exist separately --- src/artemis/__main__.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/artemis/__main__.py b/src/artemis/__main__.py index 3e9ef47f1..e58e2829f 100644 --- a/src/artemis/__main__.py +++ b/src/artemis/__main__.py @@ -146,11 +146,16 @@ def put(self, experiment: str, action: Actions): status_and_message = StatusAndMessage(Status.FAILED, f"{action} not understood") if action == Actions.START.value: try: - plan = PLAN_REGISTRY.get(experiment).get("run") - if plan is None: + experiment_type = PLAN_REGISTRY.get(experiment) + if experiment_type is None: raise PlanNotFound( f"Experiment plan '{experiment}' not found in registry." ) + plan = experiment_type.get("run") + if plan is None: + raise PlanNotFound( + f"Experiment plan '{experiment}' has no \"run\" method." + ) parameters = FullParameters.from_json(request.data) status_and_message = self.runner.start(plan, parameters) except JSONDecodeError as e: From 977616915e081438b671e6509e680e97428f0c6a Mon Sep 17 00:00:00 2001 From: David Perl Date: Wed, 25 Jan 2023 09:52:56 +0000 Subject: [PATCH 07/14] fix tests --- src/artemis/unit_tests/test_fast_grid_scan_plan.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/artemis/unit_tests/test_fast_grid_scan_plan.py b/src/artemis/unit_tests/test_fast_grid_scan_plan.py index 73590292e..5f7ee0d64 100644 --- a/src/artemis/unit_tests/test_fast_grid_scan_plan.py +++ b/src/artemis/unit_tests/test_fast_grid_scan_plan.py @@ -115,12 +115,12 @@ def test_results_adjusted_and_passed_to_move_xyz( motor_position = params.grid_scan_params.grid_position_to_motor_position( Point3D(0.5, 1.5, 2.5) ) - FakeComposite = make_fake_device(FGSComposite) - FakeEiger = make_fake_device(EigerDetector) + FakeComposite: FGSComposite = make_fake_device(FGSComposite) + FakeEiger: EigerDetector = make_fake_device(EigerDetector) RE( run_gridscan_and_move( FakeComposite("test", name="fgs"), - FakeEiger(params.detector_params), + FakeEiger.with_params(params=params.detector_params, name="test"), params, subscriptions, ) @@ -164,9 +164,9 @@ def test_individual_plans_triggered_once_and_only_once_in_composite_run( ) FakeComposite = make_fake_device(FGSComposite) - FakeEiger = make_fake_device(EigerDetector) + FakeEiger: EigerDetector = make_fake_device(EigerDetector) fake_composite = FakeComposite("test", name="fakecomposite") - fake_eiger = FakeEiger(params.detector_params) + fake_eiger = FakeEiger.with_params(params=params.detector_params, name="test") RE( run_gridscan_and_move( From 282a0ad0fae6462024364b97682e7d002336278b Mon Sep 17 00:00:00 2001 From: David Perl Date: Wed, 25 Jan 2023 10:33:41 +0000 Subject: [PATCH 08/14] test_main_system needs s03 now to load devices --- src/artemis/system_tests/test_main_system.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/artemis/system_tests/test_main_system.py b/src/artemis/system_tests/test_main_system.py index 1ad41b0af..91d176426 100644 --- a/src/artemis/system_tests/test_main_system.py +++ b/src/artemis/system_tests/test_main_system.py @@ -84,16 +84,19 @@ def check_status_in_response(response_object, expected_result: Status): assert response_json["status"] == expected_result.value +@pytest.mark.s03 def test_start_gives_success(test_env: ClientAndRunEngine): response = test_env.client.put(START_ENDPOINT, data=TEST_PARAMS) check_status_in_response(response, Status.SUCCESS) +@pytest.mark.s03 def test_getting_status_return_idle(test_env: ClientAndRunEngine): response = test_env.client.get(STATUS_ENDPOINT) check_status_in_response(response, Status.IDLE) +@pytest.mark.s03 def test_getting_status_after_start_sent_returns_busy( test_env: ClientAndRunEngine, ): @@ -102,6 +105,7 @@ def test_getting_status_after_start_sent_returns_busy( check_status_in_response(response, Status.BUSY) +@pytest.mark.s03 def test_putting_bad_plan_fails(test_env: ClientAndRunEngine): response = test_env.client.put("/bad_plan/start", data=TEST_PARAMS).json assert isinstance(response, dict) @@ -112,12 +116,14 @@ def test_putting_bad_plan_fails(test_env: ClientAndRunEngine): ) +@pytest.mark.s03 def test_sending_start_twice_fails(test_env: ClientAndRunEngine): test_env.client.put(START_ENDPOINT, data=TEST_PARAMS) response = test_env.client.put(START_ENDPOINT, data=TEST_PARAMS) check_status_in_response(response, Status.FAILED) +@pytest.mark.s03 def test_given_started_when_stopped_then_success_and_idle_status( test_env: ClientAndRunEngine, ): @@ -134,6 +140,7 @@ def test_given_started_when_stopped_then_success_and_idle_status( check_status_in_response(response, Status.ABORTING) +@pytest.mark.s03 def test_given_started_when_stopped_and_started_again_then_runs( test_env: ClientAndRunEngine, ): @@ -145,6 +152,7 @@ def test_given_started_when_stopped_and_started_again_then_runs( check_status_in_response(response, Status.BUSY) +@pytest.mark.s03 def test_given_started_when_RE_stops_on_its_own_with_error_then_error_reported( test_env: ClientAndRunEngine, ): From c853ec4a89f2a5ee7eec10652efeb889bb20e438 Mon Sep 17 00:00:00 2001 From: David Perl Date: Wed, 25 Jan 2023 10:35:10 +0000 Subject: [PATCH 09/14] fix detector param variables test --- src/artemis/devices/unit_tests/test_eiger.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/artemis/devices/unit_tests/test_eiger.py b/src/artemis/devices/unit_tests/test_eiger.py index d20f87628..00c4bdd35 100644 --- a/src/artemis/devices/unit_tests/test_eiger.py +++ b/src/artemis/devices/unit_tests/test_eiger.py @@ -100,21 +100,19 @@ def test_check_detector_variables( beam_xy_converter, expected_error_number, ): - fake_eiger.set_detector_parameters(detector_params) - if detector_params is not None: - fake_eiger.detector_params.beam_xy_converter = beam_xy_converter - fake_eiger.detector_params.detector_size_constants = detector_size_constants + detector_params.beam_xy_converter = beam_xy_converter + detector_params.detector_size_constants = detector_size_constants if expected_error_number != 0: with pytest.raises(Exception) as e: - fake_eiger.set_detector_parameters() + fake_eiger.set_detector_parameters(detector_params) number_of_errors = str(e.value).count("\n") + 1 assert number_of_errors == expected_error_number else: try: - fake_eiger.set_detector_parameters() + fake_eiger.set_detector_parameters(detector_params) except Exception as e: assert False, f"exception was raised {e}" From e4eda590201d9ebb69262df0c7e026d6eae91ad1 Mon Sep 17 00:00:00 2001 From: David Perl Date: Wed, 25 Jan 2023 10:45:23 +0000 Subject: [PATCH 10/14] patch plan registry better instead --- src/artemis/system_tests/test_main_system.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/artemis/system_tests/test_main_system.py b/src/artemis/system_tests/test_main_system.py index 91d176426..92ec5ec91 100644 --- a/src/artemis/system_tests/test_main_system.py +++ b/src/artemis/system_tests/test_main_system.py @@ -49,7 +49,11 @@ class ClientAndRunEngine: @pytest.fixture def test_env(): mock_run_engine = MockRunEngine() - app, runner = create_app({"TESTING": True}, mock_run_engine) + with patch.dict( + "artemis.__main__.PLAN_REGISTRY", + {(k, MagicMock()) for k, _ in PLAN_REGISTRY.items()}, + ): + app, runner = create_app({"TESTING": True}, mock_run_engine) runner_thread = threading.Thread(target=runner.wait_on_queue) runner_thread.start() with app.test_client() as client: @@ -84,19 +88,16 @@ def check_status_in_response(response_object, expected_result: Status): assert response_json["status"] == expected_result.value -@pytest.mark.s03 def test_start_gives_success(test_env: ClientAndRunEngine): response = test_env.client.put(START_ENDPOINT, data=TEST_PARAMS) check_status_in_response(response, Status.SUCCESS) -@pytest.mark.s03 def test_getting_status_return_idle(test_env: ClientAndRunEngine): response = test_env.client.get(STATUS_ENDPOINT) check_status_in_response(response, Status.IDLE) -@pytest.mark.s03 def test_getting_status_after_start_sent_returns_busy( test_env: ClientAndRunEngine, ): @@ -105,7 +106,6 @@ def test_getting_status_after_start_sent_returns_busy( check_status_in_response(response, Status.BUSY) -@pytest.mark.s03 def test_putting_bad_plan_fails(test_env: ClientAndRunEngine): response = test_env.client.put("/bad_plan/start", data=TEST_PARAMS).json assert isinstance(response, dict) @@ -116,14 +116,12 @@ def test_putting_bad_plan_fails(test_env: ClientAndRunEngine): ) -@pytest.mark.s03 def test_sending_start_twice_fails(test_env: ClientAndRunEngine): test_env.client.put(START_ENDPOINT, data=TEST_PARAMS) response = test_env.client.put(START_ENDPOINT, data=TEST_PARAMS) check_status_in_response(response, Status.FAILED) -@pytest.mark.s03 def test_given_started_when_stopped_then_success_and_idle_status( test_env: ClientAndRunEngine, ): @@ -140,7 +138,6 @@ def test_given_started_when_stopped_then_success_and_idle_status( check_status_in_response(response, Status.ABORTING) -@pytest.mark.s03 def test_given_started_when_stopped_and_started_again_then_runs( test_env: ClientAndRunEngine, ): @@ -152,7 +149,6 @@ def test_given_started_when_stopped_and_started_again_then_runs( check_status_in_response(response, Status.BUSY) -@pytest.mark.s03 def test_given_started_when_RE_stops_on_its_own_with_error_then_error_reported( test_env: ClientAndRunEngine, ): From 26d6ef56781e9148eeb9f1698ff8688cbbc3a411 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Fri, 27 Jan 2023 10:27:58 +0000 Subject: [PATCH 11/14] (#503) Exceptions inside receive_results are now re-raised --- .../unit_tests/test_zocalo_interaction.py | 38 ++++++++++++++++ .../zocalo/zocalo_interaction.py | 43 +++++++++++++------ 2 files changed, 68 insertions(+), 13 deletions(-) diff --git a/src/artemis/external_interaction/unit_tests/test_zocalo_interaction.py b/src/artemis/external_interaction/unit_tests/test_zocalo_interaction.py index 74ecfe3cd..2348bcbfb 100644 --- a/src/artemis/external_interaction/unit_tests/test_zocalo_interaction.py +++ b/src/artemis/external_interaction/unit_tests/test_zocalo_interaction.py @@ -6,6 +6,7 @@ from typing import Callable, Dict from unittest.mock import MagicMock, patch +import pytest from pytest import mark, raises from zocalo.configuration import Configuration @@ -135,3 +136,40 @@ def test_when_message_recieved_from_zocalo_then_point_returned( assert type(return_value) == Point3D assert return_value == Point3D(*centre_of_mass_coords) + + +@patch("workflows.recipe.wrap_subscribe") +@patch("zocalo.configuration.from_file") +@patch("artemis.external_interaction.zocalo.zocalo_interaction.lookup") +def test_when_exception_caused_by_zocalo_message_then_exception_propagated( + mock_transport_lookup, mock_from_file, mock_wrap_subscribe +): + zc = ZocaloInteractor(environment=SIM_ZOCALO_ENV) + + mock_zc: Configuration = MagicMock() + mock_from_file.return_value = mock_zc + mock_transport = MagicMock() + mock_transport_lookup.return_value = MagicMock() + mock_transport_lookup.return_value.return_value = mock_transport + + with concurrent.futures.ThreadPoolExecutor() as executor: + future = executor.submit(zc.wait_for_result, 0) + + for _ in range(10): + sleep(0.1) + if mock_wrap_subscribe.call_args: + break + + result_func = mock_wrap_subscribe.call_args[0][2] + + failure_exception = Exception("Bad function!") + + mock_recipe_wrapper = MagicMock() + mock_transport.ack.side_effect = failure_exception + with pytest.raises(Exception) as actual_exception: + result_func(mock_recipe_wrapper, {}, []) + assert str(actual_exception.value) == str(failure_exception) + + with pytest.raises(Exception) as actual_exception: + future.result() + assert str(actual_exception.value) == str(failure_exception) diff --git a/src/artemis/external_interaction/zocalo/zocalo_interaction.py b/src/artemis/external_interaction/zocalo/zocalo_interaction.py index b644ee59a..0e14516c1 100644 --- a/src/artemis/external_interaction/zocalo/zocalo_interaction.py +++ b/src/artemis/external_interaction/zocalo/zocalo_interaction.py @@ -1,6 +1,8 @@ import getpass import queue import socket +from time import sleep +from typing import Optional import workflows.recipe import workflows.transport @@ -89,22 +91,28 @@ def wait_for_result( """ transport = self._get_zocalo_connection() result_received: queue.Queue = queue.Queue() + exception: Optional[Exception] = None def receive_result( rw: workflows.recipe.RecipeWrapper, header: dict, message: dict ) -> None: - artemis.log.LOGGER.info(f"Received {message}") - recipe_parameters = rw.recipe_step["parameters"] - artemis.log.LOGGER.info(f"Recipe step parameters: {recipe_parameters}") - transport.ack(header) - received_group_id = recipe_parameters["dcgid"] - if received_group_id == str(data_collection_group_id): - result_received.put(Point3D(*message[0]["centre_of_mass"])) - else: - artemis.log.LOGGER.warning( - f"Warning: results for {received_group_id} received but expected \ - {data_collection_group_id}" - ) + try: + artemis.log.LOGGER.info(f"Received {message}") + recipe_parameters = rw.recipe_step["parameters"] + artemis.log.LOGGER.info(f"Recipe step parameters: {recipe_parameters}") + transport.ack(header) + received_group_id = recipe_parameters["dcgid"] + if received_group_id == str(data_collection_group_id): + result_received.put(Point3D(*message[0]["centre_of_mass"])) + else: + artemis.log.LOGGER.warning( + f"Warning: results for {received_group_id} received but expected \ + {data_collection_group_id}" + ) + except Exception as e: + nonlocal exception + exception = e + raise e workflows.recipe.wrap_subscribe( transport, @@ -115,6 +123,15 @@ def receive_result( ) try: - return result_received.get(timeout=timeout) + time_waited = 0 + while time_waited < timeout: + if result_received.empty(): + if exception is not None: + raise exception + else: + sleep(1) + else: + return result_received.get_nowait() + raise TimeoutError("Timed out waiting for zocalo results") finally: transport.disconnect() From 0db178f38b707642c4295d83eac9221185872a84 Mon Sep 17 00:00:00 2001 From: David Perl <115003895+dperl-dls@users.noreply.github.com> Date: Fri, 27 Jan 2023 11:34:49 +0000 Subject: [PATCH 12/14] add more context to message --- src/artemis/external_interaction/zocalo/zocalo_interaction.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/artemis/external_interaction/zocalo/zocalo_interaction.py b/src/artemis/external_interaction/zocalo/zocalo_interaction.py index 0e14516c1..d1bd953c0 100644 --- a/src/artemis/external_interaction/zocalo/zocalo_interaction.py +++ b/src/artemis/external_interaction/zocalo/zocalo_interaction.py @@ -132,6 +132,6 @@ def receive_result( sleep(1) else: return result_received.get_nowait() - raise TimeoutError("Timed out waiting for zocalo results") + raise TimeoutError(f"No results returned by Zocalo for dcgid {data_collection_group_id} within timeout of {timeout}") finally: transport.disconnect() From 8f8b75c2cfe093d86b5840ec9486cf5280636e5f Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Fri, 27 Jan 2023 12:21:39 +0000 Subject: [PATCH 13/14] (#501) Throw a no diffraction exception on no results --- .../unit_tests/test_zocalo_interaction.py | 80 ++++++++++++++++++- .../zocalo/zocalo_interaction.py | 7 ++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/src/artemis/external_interaction/unit_tests/test_zocalo_interaction.py b/src/artemis/external_interaction/unit_tests/test_zocalo_interaction.py index 2348bcbfb..d52f558b8 100644 --- a/src/artemis/external_interaction/unit_tests/test_zocalo_interaction.py +++ b/src/artemis/external_interaction/unit_tests/test_zocalo_interaction.py @@ -10,7 +10,10 @@ from pytest import mark, raises from zocalo.configuration import Configuration -from artemis.external_interaction.zocalo.zocalo_interaction import ZocaloInteractor +from artemis.external_interaction.zocalo.zocalo_interaction import ( + NoDiffractionFound, + ZocaloInteractor, +) from artemis.parameters import SIM_ZOCALO_ENV from artemis.utils import Point3D @@ -173,3 +176,78 @@ def test_when_exception_caused_by_zocalo_message_then_exception_propagated( with pytest.raises(Exception) as actual_exception: future.result() assert str(actual_exception.value) == str(failure_exception) + + +@patch("workflows.recipe.wrap_subscribe") +@patch("zocalo.configuration.from_file") +@patch("artemis.external_interaction.zocalo.zocalo_interaction.lookup") +def test_when_exception_caused_by_zocalo_message_then_exception_propagated( + mock_transport_lookup, mock_from_file, mock_wrap_subscribe +): + zc = ZocaloInteractor(environment=SIM_ZOCALO_ENV) + + mock_zc: Configuration = MagicMock() + mock_from_file.return_value = mock_zc + mock_transport = MagicMock() + mock_transport_lookup.return_value = MagicMock() + mock_transport_lookup.return_value.return_value = mock_transport + + with concurrent.futures.ThreadPoolExecutor() as executor: + future = executor.submit(zc.wait_for_result, 0) + + for _ in range(10): + sleep(0.1) + if mock_wrap_subscribe.call_args: + break + + result_func = mock_wrap_subscribe.call_args[0][2] + + failure_exception = Exception("Bad function!") + + mock_recipe_wrapper = MagicMock() + mock_transport.ack.side_effect = failure_exception + with pytest.raises(Exception) as actual_exception: + result_func(mock_recipe_wrapper, {}, []) + assert str(actual_exception.value) == str(failure_exception) + + with pytest.raises(Exception) as actual_exception: + future.result() + assert str(actual_exception.value) == str(failure_exception) + + +@patch("workflows.recipe.wrap_subscribe") +@patch("zocalo.configuration.from_file") +@patch("artemis.external_interaction.zocalo.zocalo_interaction.lookup") +def test_when_no_results_returned_then_no_diffraction_exception_raised( + mock_transport_lookup, mock_from_file, mock_wrap_subscribe +): + zc = ZocaloInteractor(environment=SIM_ZOCALO_ENV) + + message = [] + datacollection_grid_id = 7263143 + step_params = {"dcid": "8183741", "dcgid": str(datacollection_grid_id)} + + mock_zc: Configuration = MagicMock() + mock_from_file.return_value = mock_zc + mock_transport = MagicMock() + mock_transport_lookup.return_value = MagicMock() + mock_transport_lookup.return_value.return_value = mock_transport + + with concurrent.futures.ThreadPoolExecutor() as executor: + future = executor.submit(zc.wait_for_result, datacollection_grid_id) + + for _ in range(10): + sleep(0.1) + if mock_wrap_subscribe.call_args: + break + + result_func = mock_wrap_subscribe.call_args[0][2] + + mock_recipe_wrapper = MagicMock() + mock_recipe_wrapper.recipe_step.__getitem__.return_value = step_params + + with pytest.raises(NoDiffractionFound): + result_func(mock_recipe_wrapper, {}, message) + + with pytest.raises(NoDiffractionFound): + future.result() diff --git a/src/artemis/external_interaction/zocalo/zocalo_interaction.py b/src/artemis/external_interaction/zocalo/zocalo_interaction.py index 0e14516c1..bb6f5c6c3 100644 --- a/src/artemis/external_interaction/zocalo/zocalo_interaction.py +++ b/src/artemis/external_interaction/zocalo/zocalo_interaction.py @@ -10,11 +10,16 @@ from workflows.transport import lookup import artemis.log +from artemis.exceptions import WarningException from artemis.utils import Point3D TIMEOUT = 90 +class NoDiffractionFound(WarningException): + pass + + class ZocaloInteractor: zocalo_environment: str = "artemis" @@ -103,6 +108,8 @@ def receive_result( transport.ack(header) received_group_id = recipe_parameters["dcgid"] if received_group_id == str(data_collection_group_id): + if len(message) == 0: + raise NoDiffractionFound() result_received.put(Point3D(*message[0]["centre_of_mass"])) else: artemis.log.LOGGER.warning( From e7cd45023590f30bbf07c07ce40a9084f226a281 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Fri, 27 Jan 2023 12:57:23 +0000 Subject: [PATCH 14/14] (#501) Move to fallback position on no diffraction --- fake_zocalo/__main__.py | 32 ++++++++++++++++++- .../fgs/tests/test_zocalo_handler.py | 7 ++-- .../callbacks/fgs/zocalo_callback.py | 28 +++++++++------- .../system_tests/test_zocalo_system.py | 22 +++++++++++-- 4 files changed, 71 insertions(+), 18 deletions(-) diff --git a/fake_zocalo/__main__.py b/fake_zocalo/__main__.py index f719a0715..4b3380fae 100644 --- a/fake_zocalo/__main__.py +++ b/fake_zocalo/__main__.py @@ -8,6 +8,8 @@ from pika.adapters.blocking_connection import BlockingChannel from pika.spec import BasicProperties +NO_DIFFRACTION_ID = 1 + def load_configuration_file(filename): conf = yaml.safe_load(Path(filename).read_text()) @@ -23,7 +25,7 @@ def main(): config["host"], config["port"], config["vhost"], creds ) - result = { + single_crystal_result = { "environment": {"ID": "6261b482-bef2-49f5-8699-eb274cd3b92e"}, "payload": [{"max_voxel": [1, 2, 3], "centre_of_mass": [1.2, 2.3, 3.4]}], "recipe": { @@ -41,6 +43,27 @@ def main(): "recipe-pointer": 1, } + no_diffraction_result = { + "environment": {"ID": "6261b482-bef2-49f5-8699-eb274cd3b92e"}, + "payload": [], + "recipe": { + "start": [ + [1, [{"max_voxel": [1, 2, 3], "centre_of_mass": [1.2, 2.3, 3.4]}]] + ], + "1": { + "service": "Send XRC results to GDA", + "queue": "xrc.i03", + "exchange": "results", + "parameters": { + "dcid": str(NO_DIFFRACTION_ID), + "dcgid": str(NO_DIFFRACTION_ID), + }, + }, + }, + "recipe-path": [], + "recipe-pointer": 1, + } + def on_request(ch: BlockingChannel, method, props, body): print( f"recieved message: \n properties: \n\n {method} \n\n {props} \n\n{body}\n" @@ -58,6 +81,13 @@ def on_request(ch: BlockingChannel, method, props, body): delivery_mode=2, headers={"workflows-recipe": True, "x-delivery-count": 1}, ) + + if message.get("parameters").get("ispyb_dcid") == NO_DIFFRACTION_ID: + result = no_diffraction_result + else: + result = single_crystal_result + + print(f"Sending results {result}") result_chan = conn.channel() result_chan.basic_publish( "results", "xrc.i03", json.dumps(result), resultprops diff --git a/src/artemis/external_interaction/callbacks/fgs/tests/test_zocalo_handler.py b/src/artemis/external_interaction/callbacks/fgs/tests/test_zocalo_handler.py index d8f37cb5b..49ddd9f24 100644 --- a/src/artemis/external_interaction/callbacks/fgs/tests/test_zocalo_handler.py +++ b/src/artemis/external_interaction/callbacks/fgs/tests/test_zocalo_handler.py @@ -1,4 +1,3 @@ -from math import nan from unittest.mock import MagicMock, call import pytest @@ -7,6 +6,7 @@ FGSCallbackCollection, ) from artemis.external_interaction.callbacks.fgs.tests.conftest import TestData +from artemis.external_interaction.zocalo.zocalo_interaction import NoDiffractionFound from artemis.parameters import FullParameters from artemis.utils import Point3D @@ -113,9 +113,8 @@ def test_GIVEN_no_results_from_zocalo_WHEN_communicator_wait_for_results_called_ callbacks = FGSCallbackCollection.from_params(params) mock_zocalo_functions(callbacks) callbacks.ispyb_handler.ispyb_ids = (0, 0, 100) - expected_centre_grid_coords = Point3D(nan, nan, nan) - callbacks.zocalo_handler.zocalo_interactor.wait_for_result.return_value = ( - expected_centre_grid_coords + callbacks.zocalo_handler.zocalo_interactor.wait_for_result.side_effect = ( + NoDiffractionFound() ) fallback_position = Point3D(1, 2, 3) diff --git a/src/artemis/external_interaction/callbacks/fgs/zocalo_callback.py b/src/artemis/external_interaction/callbacks/fgs/zocalo_callback.py index b23f45536..7127087b0 100644 --- a/src/artemis/external_interaction/callbacks/fgs/zocalo_callback.py +++ b/src/artemis/external_interaction/callbacks/fgs/zocalo_callback.py @@ -1,4 +1,3 @@ -import math import time from typing import Callable @@ -8,7 +7,10 @@ FGSISPyBHandlerCallback, ) from artemis.external_interaction.exceptions import ISPyBDepositionNotMade -from artemis.external_interaction.zocalo.zocalo_interaction import ZocaloInteractor +from artemis.external_interaction.zocalo.zocalo_interaction import ( + NoDiffractionFound, + ZocaloInteractor, +) from artemis.log import LOGGER from artemis.parameters import ISPYB_PLAN_NAME, FullParameters from artemis.utils import Point3D @@ -75,24 +77,28 @@ def wait_for_results(self, fallback_xyz: Point3D) -> Point3D: Point3D: The xray centre position to move to """ datacollection_group_id = self.ispyb.ispyb_ids[2] - raw_results = self.zocalo_interactor.wait_for_result(datacollection_group_id) self.processing_time = time.time() - self.processing_start_time + try: + raw_results = self.zocalo_interactor.wait_for_result( + datacollection_group_id + ) + + # _wait_for_result returns the centre of the grid box, but we want the corner + results = Point3D( + raw_results.x - 0.5, raw_results.y - 0.5, raw_results.z - 0.5 + ) + xray_centre = self.grid_position_to_motor_position(results) + + LOGGER.info(f"Results recieved from zocalo: {xray_centre}") - if any([math.isnan(coord) for coord in raw_results]): + except NoDiffractionFound: # We move back to the centre if results aren't found log_msg = ( f"Zocalo: No diffraction found, using fallback centre {fallback_xyz}" ) xray_centre = fallback_xyz LOGGER.warn(log_msg) - else: - # _wait_for_result returns the centre of the grid box, but we want the corner - results = Point3D( - raw_results.x - 0.5, raw_results.y - 0.5, raw_results.z - 0.5 - ) - xray_centre = self.grid_position_to_motor_position(results) - LOGGER.info(f"Results recieved from zocalo: {xray_centre}") self.ispyb.append_to_comment( f"Zocalo processing took {self.processing_time:.2f} s" ) diff --git a/src/artemis/external_interaction/system_tests/test_zocalo_system.py b/src/artemis/external_interaction/system_tests/test_zocalo_system.py index 44f355e0f..dcede6410 100644 --- a/src/artemis/external_interaction/system_tests/test_zocalo_system.py +++ b/src/artemis/external_interaction/system_tests/test_zocalo_system.py @@ -24,7 +24,7 @@ def test_when_running_start_stop_then_get_expected_returned_results(zocalo_env): zc.zocalo_interactor.run_start(dcid) for dcid in dcids: zc.zocalo_interactor.run_end(dcid) - assert zc.zocalo_interactor.wait_for_result(4) == Point3D(x=3.4, y=2.3, z=1.2) + assert zc.zocalo_interactor.wait_for_result(4) == Point3D(x=1.2, y=2.3, z=3.4) @pytest.mark.s03 @@ -32,10 +32,28 @@ def test_zocalo_callback_calls_append_comment(zocalo_env): params = FullParameters() zc: FGSZocaloCallback = FGSCallbackCollection.from_params(params).zocalo_handler zc.ispyb.append_to_comment = MagicMock() - zc.ispyb.ispyb_ids = ([1, 2], 0, 4) dcids = [1, 2] + zc.ispyb.ispyb_ids = (dcids, 0, 4) for dcid in dcids: zc.zocalo_interactor.run_start(dcid) zc.stop({}) zc.wait_for_results(fallback_xyz=Point3D(0, 0, 0)) assert zc.ispyb.append_to_comment.call_count == 1 + + +@pytest.mark.s03 +def test_given_a_result_with_no_diffraction_when_zocalo_called_then_move_to_fallback( + zocalo_env, +): + params = FullParameters() + NO_DIFFFRACTION_ID = 1 + zc: FGSZocaloCallback = FGSCallbackCollection.from_params(params).zocalo_handler + zc.ispyb.append_to_comment = MagicMock() + dcids = [NO_DIFFFRACTION_ID, NO_DIFFFRACTION_ID] + zc.ispyb.ispyb_ids = (dcids, 0, NO_DIFFFRACTION_ID) + for dcid in dcids: + zc.zocalo_interactor.run_start(dcid) + zc.stop({}) + fallback = Point3D(1, 2, 3) + centre = zc.wait_for_results(fallback_xyz=fallback) + assert centre == fallback