From 18b9eb50039d810392eb507de80a93dea8045ac9 Mon Sep 17 00:00:00 2001 From: David Perl Date: Mon, 7 Aug 2023 11:42:10 +0100 Subject: [PATCH 1/8] separate out sample env setup --- .../device_setup_plans/manipulate_sample.py | 63 ++++++++++ src/artemis/device_setup_plans/setup_zebra.py | 4 + .../experiment_plans/fast_grid_scan_plan.py | 28 +---- .../experiment_plans/rotation_scan_plan.py | 111 +++++++++--------- 4 files changed, 123 insertions(+), 83 deletions(-) create mode 100644 src/artemis/device_setup_plans/manipulate_sample.py diff --git a/src/artemis/device_setup_plans/manipulate_sample.py b/src/artemis/device_setup_plans/manipulate_sample.py new file mode 100644 index 000000000..79d2df126 --- /dev/null +++ b/src/artemis/device_setup_plans/manipulate_sample.py @@ -0,0 +1,63 @@ +from __future__ import annotations + +import bluesky.plan_stubs as bps +from dodal.devices.attenuator import Attenuator +from dodal.devices.backlight import Backlight +from dodal.devices.detector_motion import DetectorMotion +from dodal.devices.smargon import Smargon + +from artemis.log import LOGGER + +LOWER_DETECTOR_SHUTTER_AFTER_SCAN = True + + +def setup_sample_environment( + detector_motion: DetectorMotion, + backlight: Backlight, + attenuator: Attenuator, + transmission_fraction: float, + detector_distance: float, + group="setup_senv", +): + """Move out the backlight, retract the detector shutter, and set the attenuator to + transmission.""" + + yield from bps.abs_set(detector_motion.shutter, 1, group=group) + yield from bps.abs_set(detector_motion.z, detector_distance, group=group) + yield from bps.abs_set(backlight.pos, backlight.OUT, group=group) + yield from bps.abs_set(attenuator, transmission_fraction, group=group) + + +def cleanup_sample_environment( + detector_motion: DetectorMotion, + group="cleanup_senv", +): + """Put the detector shutter back down""" + + yield from bps.abs_set( + detector_motion.shutter, + int(not LOWER_DETECTOR_SHUTTER_AFTER_SCAN), + group=group, + ) + + +def move_x_y_z( + smargon: Smargon, + x: float | None = None, + y: float | None = None, + z: float | None = None, + wait=False, + group="move_x_y_z", +): + """Move the x, y, and z axes of the given smargon to the specified position. All + axes are optional.""" + + LOGGER.info(f"Moving smargon to x, y, z: {(x,y,z)}") + if x: + yield from bps.abs_set(smargon.x, x, group=group) + if y: + yield from bps.abs_set(smargon.y, y, group=group) + if z: + yield from bps.abs_set(smargon.z, z, group=group) + if wait: + yield from bps.wait(group) diff --git a/src/artemis/device_setup_plans/setup_zebra.py b/src/artemis/device_setup_plans/setup_zebra.py index 66b07f60e..274969b9e 100644 --- a/src/artemis/device_setup_plans/setup_zebra.py +++ b/src/artemis/device_setup_plans/setup_zebra.py @@ -108,3 +108,7 @@ def set_zebra_shutter_to_manual( if wait: yield from bps.wait(group) + + +def make_trigger_safe(zebra: Zebra, group="make_zebra_safe", wait=False): + yield from bps.abs_set(zebra.inputs.soft_in_1, 0, wait=wait) diff --git a/src/artemis/experiment_plans/fast_grid_scan_plan.py b/src/artemis/experiment_plans/fast_grid_scan_plan.py index 21bde8625..0e87e7e8c 100755 --- a/src/artemis/experiment_plans/fast_grid_scan_plan.py +++ b/src/artemis/experiment_plans/fast_grid_scan_plan.py @@ -27,6 +27,7 @@ from dodal.devices.fast_grid_scan import set_fast_grid_scan_params import artemis.log +from artemis.device_setup_plans.manipulate_sample import move_x_y_z from artemis.device_setup_plans.read_hardware_for_setup import read_hardware_for_ispyb from artemis.device_setup_plans.setup_zebra import ( set_zebra_shutter_to_manual, @@ -119,28 +120,6 @@ def set_aperture(): yield from set_aperture() -@bpp.set_run_key_decorator("move_xyz") -@bpp.run_decorator(md={"subplan_name": "move_xyz"}) -def move_xyz( - sample_motors, - xray_centre_motor_position: np.ndarray, - md={ - "plan_name": "move_xyz", - }, -): - """Move 'sample motors' to a specific motor position (e.g. a position obtained - from gridscan processing results)""" - artemis.log.LOGGER.info(f"Moving Smargon x, y, z to: {xray_centre_motor_position}") - yield from bps.mv( - sample_motors.x, - xray_centre_motor_position[0], - sample_motors.y, - xray_centre_motor_position[1], - sample_motors.z, - xray_centre_motor_position[2], - ) - - def wait_for_fgs_valid(fgs_motors: FastGridScan, timeout=0.5): artemis.log.LOGGER.info("Waiting for valid fgs_params") SLEEP_PER_CHECK = 0.1 @@ -254,10 +233,7 @@ def run_gridscan_and_move( # once we have the results, go to the appropriate position artemis.log.LOGGER.info("Moving to centre of mass.") with TRACER.start_span("move_to_result"): - yield from move_xyz( - fgs_composite.sample_motors, - xray_centre, - ) + yield from move_x_y_z(fgs_composite.sample_motors, *xray_centre) def get_plan( diff --git a/src/artemis/experiment_plans/rotation_scan_plan.py b/src/artemis/experiment_plans/rotation_scan_plan.py index 1692249f7..cb551a911 100644 --- a/src/artemis/experiment_plans/rotation_scan_plan.py +++ b/src/artemis/experiment_plans/rotation_scan_plan.py @@ -15,10 +15,16 @@ from ophyd.device import Device from ophyd.epics_motor import EpicsMotor +from artemis.device_setup_plans.manipulate_sample import ( + cleanup_sample_environment, + move_x_y_z, + setup_sample_environment, +) from artemis.device_setup_plans.read_hardware_for_setup import read_hardware_for_ispyb from artemis.device_setup_plans.setup_zebra import ( arm_zebra, disarm_zebra, + make_trigger_safe, setup_zebra_for_rotation, ) from artemis.external_interaction.callbacks.rotation.rotation_callback_collection import ( @@ -55,46 +61,6 @@ def create_devices() -> dict[str, Device]: TIME_TO_VELOCITY_S = 0.15 -def setup_sample_environment( - detector_motion: DetectorMotion, - backlight: Backlight, - attenuator: Attenuator, - transmission: float, - group="setup_senv", -): - yield from bps.abs_set(detector_motion.shutter, 1, group=group) - yield from bps.abs_set(backlight.pos, backlight.OUT, group=group) - yield from bps.abs_set(attenuator, transmission, group=group) - - -def cleanup_sample_environment( - zebra: Zebra, - detector_motion: DetectorMotion, - group="cleanup_senv", -): - yield from bps.abs_set(zebra.inputs.soft_in_1, 0, group=group) - yield from bps.abs_set(detector_motion.shutter, 0, group=group) - - -def move_x_y_z( - smargon: Smargon, - x: float | None, - y: float | None, - z: float | None, - wait=False, - group="move_x_y_z", -): - if x: - LOGGER.info(f"x: {x}, y: {y}, z: {z}") - yield from bps.abs_set(smargon.x, x, group=group) - if y: - yield from bps.abs_set(smargon.y, y, group=group) - if z: - yield from bps.abs_set(smargon.z, z, group=group) - if wait: - yield from bps.wait(group) - - def move_to_start_w_buffer( axis: EpicsMotor, start_angle: float, @@ -150,19 +116,41 @@ def set_speed(axis: EpicsMotor, image_width, exposure_time, wait=True): ) +@bpp.set_run_key_decorator("rotation_scan_setup") +@bpp.run_decorator(md={"subplan_name": "rotation_scan_setup"}) +def rotation_scan_experiment_setup( + params: RotationInternalParameters, + smargon: Smargon, + backlight: Backlight, + attenuator: Attenuator, + detector_motion: DetectorMotion, + **kwargs, +): + expt_params: RotationScanParams = params.experiment_params + + LOGGER.info("moving to start x, y, z if necessary") + yield from move_x_y_z( + smargon, expt_params.x, expt_params.y, expt_params.z, wait=True + ) + + transmission = params.artemis_params.ispyb_params.transmission_fraction + yield from setup_sample_environment( + detector_motion, backlight, attenuator, transmission + ) + + @bpp.set_run_key_decorator("rotation_scan_main") @bpp.run_decorator(md={"subplan_name": "rotation_scan_main"}) def rotation_scan_plan( params: RotationInternalParameters, smargon: Smargon, zebra: Zebra, - backlight: Backlight, - attenuator: Attenuator, - detector_motion: DetectorMotion, **kwargs, ): """A plan to collect diffraction images from a sample continuously rotating about - a fixed axis - for now this axis is limited to omega.""" + a fixed axis - for now this axis is limited to omega. Only does the scan itself, no + setup tasks.""" + detector_params: DetectorParams = params.artemis_params.detector_params expt_params: RotationScanParams = params.experiment_params @@ -172,11 +160,6 @@ def rotation_scan_plan( exposure_time_s = detector_params.exposure_time shutter_time_s = expt_params.shutter_opening_time_s - LOGGER.info("moving to start x, y, z if necessary") - yield from move_x_y_z( - smargon, expt_params.x, expt_params.y, expt_params.z, wait=True - ) - speed_for_rotation_deg_s = image_width_deg / exposure_time_s LOGGER.info(f"calculated speed: {speed_for_rotation_deg_s} deg/s") @@ -196,10 +179,6 @@ def rotation_scan_plan( f" for {shutter_time_s} s at {speed_for_rotation_deg_s} deg/s" ) - transmission = params.artemis_params.ispyb_params.transmission_fraction - yield from setup_sample_environment( - detector_motion, backlight, attenuator, transmission - ) LOGGER.info(f"moving omega to beginning, start_angle={start_angle_deg}") yield from move_to_start_w_buffer( smargon.omega, start_angle_deg, acceleration_offset @@ -257,9 +236,12 @@ def rotation_scan_plan( def cleanup_plan( zebra: Zebra, smargon: Smargon, detector_motion: DetectorMotion, **kwargs ): - yield from cleanup_sample_environment(zebra, detector_motion) - yield from bps.abs_set(smargon.omega.velocity, DEFAULT_MAX_VELOCITY) - yield from bpp.finalize_wrapper(disarm_zebra(zebra), bps.wait("cleanup_senv")) + yield from cleanup_sample_environment(detector_motion, group="cleanup") + yield from bps.abs_set( + smargon.omega.velocity, DEFAULT_MAX_VELOCITY, group="cleanup" + ) + yield from make_trigger_safe(zebra, group="cleanup") + yield from bpp.finalize_wrapper(disarm_zebra(zebra), bps.wait("cleanup")) def get_plan(parameters: RotationInternalParameters): @@ -282,7 +264,22 @@ def rotation_scan_plan_with_stage_and_cleanup( @bpp.stage_decorator([eiger]) @bpp.finalize_decorator(lambda: cleanup_plan(**devices)) - def rotation_with_cleanup_and_stage(params): + def rotation_with_cleanup_and_stage(params: RotationInternalParameters): + LOGGER.info("setting up sample environment...") + yield from setup_sample_environment( + devices["detector_motion"], + devices["backlight"], + devices["attenuator"], + params.artemis_params.ispyb_params.transmission_fraction, + params.artemis_params.detector_params.detector_distance, + ) + LOGGER.info("moving to position (if specified)") + yield from move_x_y_z( + devices["smargon"], + params.experiment_params.x, + params.experiment_params.y, + params.experiment_params.z, + ) LOGGER.info("setting up and staging eiger...") yield from rotation_scan_plan(params, **devices) From 19d2a1fba2a036f74f319c622ca6aada1772183f Mon Sep 17 00:00:00 2001 From: David Perl Date: Mon, 7 Aug 2023 11:42:19 +0100 Subject: [PATCH 2/8] fix tests --- .../experiment_plans/tests/conftest.py | 68 ++++++++++++++++--- .../tests/test_fast_grid_scan_plan.py | 59 +++++++++++----- .../tests/test_rotation_scan_plan.py | 60 ++++++++++++---- 3 files changed, 148 insertions(+), 39 deletions(-) diff --git a/src/artemis/experiment_plans/tests/conftest.py b/src/artemis/experiment_plans/tests/conftest.py index 391e69de8..0fe76b8f1 100644 --- a/src/artemis/experiment_plans/tests/conftest.py +++ b/src/artemis/experiment_plans/tests/conftest.py @@ -6,9 +6,13 @@ from bluesky.utils import Msg from dodal.beamlines import i03 from dodal.devices.aperturescatterguard import AperturePositions +from dodal.devices.attenuator import Attenuator +from dodal.devices.backlight import Backlight +from dodal.devices.detector_motion import DetectorMotion from dodal.devices.eiger import EigerDetector from dodal.devices.smargon import Smargon from dodal.devices.zebra import Zebra +from ophyd.epics_motor import EpicsMotor from ophyd.status import Status from artemis.experiment_plans.fast_grid_scan_plan import FGSComposite @@ -31,6 +35,15 @@ ) +def mock_set(motor: EpicsMotor, val): + motor.user_readback.sim_put(val) + return Status(done=True, success=True) + + +def patch_motor(motor): + return patch.object(motor, "set", partial(mock_set, motor)) + + @pytest.fixture def test_fgs_params(): return FGSInternalParameters(**raw_params_from_file()) @@ -67,13 +80,6 @@ def smargon() -> Smargon: smargon.z.user_setpoint._use_limits = False smargon.omega.user_setpoint._use_limits = False - def mock_set(motor, val): - motor.user_readback.sim_put(val) - return Status(done=True, success=True) - - def patch_motor(motor): - return patch.object(motor, "set", partial(mock_set, motor)) - with patch_motor(smargon.omega), patch_motor(smargon.x), patch_motor( smargon.y ), patch_motor(smargon.z): @@ -92,7 +98,11 @@ def backlight(): @pytest.fixture def detector_motion(): - return i03.detector_motion(fake_with_ophyd_sim=True) + det = i03.detector_motion(fake_with_ophyd_sim=True) + det.z.user_setpoint._use_limits = False + + with patch_motor(det.z): + yield det @pytest.fixture @@ -122,7 +132,12 @@ def flux(): @pytest.fixture def attenuator(): - return i03.attenuator(fake_with_ophyd_sim=True) + with patch( + "dodal.devices.attenuator.await_value", + return_value=Status(done=True, success=True), + autospec=True, + ): + yield i03.attenuator(fake_with_ophyd_sim=True) @pytest.fixture @@ -165,6 +180,7 @@ def fake_create_devices( eiger: EigerDetector, smargon: Smargon, zebra: Zebra, + detetctor_motion: DetectorMotion, ): eiger.stage = MagicMock() eiger.unstage = MagicMock() @@ -178,15 +194,45 @@ def fake_create_devices( smargon.omega.set = mock_omega_sets devices = { - "eiger": i03.eiger(fake_with_ophyd_sim=True), + "eiger": eiger, "smargon": smargon, "zebra": zebra, - "detector_motion": i03.detector_motion(fake_with_ophyd_sim=True), + "detector_motion": detetctor_motion, "backlight": i03.backlight(fake_with_ophyd_sim=True), } return devices +@pytest.fixture() +def fake_create_rotation_devices( + eiger: EigerDetector, + smargon: Smargon, + zebra: Zebra, + detector_motion: DetectorMotion, + backlight: Backlight, + attenuator: Attenuator, +): + eiger.stage = MagicMock() + eiger.unstage = MagicMock() + mock_omega_sets = MagicMock(return_value=Status(done=True, success=True)) + + mock_arm_disarm = MagicMock( + side_effect=zebra.pc.arm.armed.set, return_value=Status(done=True, success=True) + ) + zebra.pc.arm.set = mock_arm_disarm + smargon.omega.velocity.set = mock_omega_sets + smargon.omega.set = mock_omega_sets + + return { + "eiger": eiger, + "smargon": smargon, + "zebra": zebra, + "detector_motion": detector_motion, + "backlight": backlight, + "attenuator": attenuator, + } + + @pytest.fixture def fake_fgs_composite(smargon: Smargon, test_fgs_params: InternalParameters): fake_composite = FGSComposite( 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 c4cc80ebf..02a8b6893 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 @@ -1,5 +1,5 @@ import types -from unittest.mock import ANY, MagicMock, call, patch +from unittest.mock import MagicMock, call, patch import bluesky.plan_stubs as bps import numpy as np @@ -127,9 +127,9 @@ def standalone_read_hardware_for_ispyb(und, syn, slits, attn, fl): "dodal.devices.aperturescatterguard.ApertureScatterguard._safe_move_within_datacollection_range" ) @patch("artemis.experiment_plans.fast_grid_scan_plan.run_gridscan", autospec=True) -@patch("artemis.experiment_plans.fast_grid_scan_plan.move_xyz", autospec=True) +@patch("artemis.experiment_plans.fast_grid_scan_plan.move_x_y_z", autospec=True) def test_results_adjusted_and_passed_to_move_xyz( - move_xyz: MagicMock, + move_x_y_z: MagicMock, run_gridscan: MagicMock, move_aperture: MagicMock, fake_fgs_composite: FGSComposite, @@ -187,35 +187,62 @@ def test_results_adjusted_and_passed_to_move_xyz( ) ) - call_large = call( + ap_call_large = call( *(fake_fgs_composite.aperture_scatterguard.aperture_positions.LARGE) ) - call_medium = call( + ap_call_medium = call( *(fake_fgs_composite.aperture_scatterguard.aperture_positions.MEDIUM) ) move_aperture.assert_has_calls( - [call_large, call_large, call_medium], any_order=True + [ap_call_large, ap_call_large, ap_call_medium], any_order=True + ) + + mv_call_large = call( + fake_fgs_composite.sample_motors, 0.05, 0.15000000000000002, 0.25 + ) + mv_call_medium = call( + fake_fgs_composite.sample_motors, 0.05, 0.15000000000000002, 0.25 + ) + move_x_y_z.assert_has_calls( + [mv_call_large, mv_call_large, mv_call_medium], any_order=True ) -@patch("bluesky.plan_stubs.mv", autospec=True) +@patch("bluesky.plan_stubs.abs_set", autospec=True) def test_results_passed_to_move_motors( - bps_mv: MagicMock, + bps_abs_set: MagicMock, test_fgs_params: FGSInternalParameters, fake_fgs_composite: FGSComposite, RE: RunEngine, ): - from artemis.experiment_plans.fast_grid_scan_plan import move_xyz + from artemis.device_setup_plans.manipulate_sample import move_x_y_z set_up_logging_handlers(logging_level="INFO", dev_mode=True) RE.subscribe(VerbosePlanExecutionLoggingCallback()) motor_position = test_fgs_params.experiment_params.grid_position_to_motor_position( np.array([1, 2, 3]) ) - RE(move_xyz(fake_fgs_composite.sample_motors, motor_position)) - bps_mv.assert_called_once_with( - ANY, motor_position[0], ANY, motor_position[1], ANY, motor_position[2] + RE(move_x_y_z(fake_fgs_composite.sample_motors, *motor_position)) + bps_abs_set.assert_has_calls( + [ + call( + fake_fgs_composite.sample_motors.x, + motor_position[0], + group="move_x_y_z", + ), + call( + fake_fgs_composite.sample_motors.y, + motor_position[1], + group="move_x_y_z", + ), + call( + fake_fgs_composite.sample_motors.z, + motor_position[2], + group="move_x_y_z", + ), + ], + any_order=True, ) @@ -223,7 +250,7 @@ def test_results_passed_to_move_motors( "dodal.devices.aperturescatterguard.ApertureScatterguard._safe_move_within_datacollection_range", ) @patch("artemis.experiment_plans.fast_grid_scan_plan.run_gridscan", autospec=True) -@patch("artemis.experiment_plans.fast_grid_scan_plan.move_xyz", autospec=True) +@patch("artemis.experiment_plans.fast_grid_scan_plan.move_x_y_z", autospec=True) @patch("bluesky.plan_stubs.rd") def test_individual_plans_triggered_once_and_only_once_in_composite_run( rd: MagicMock, @@ -262,7 +289,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] + array_arg = move_xyz.call_args.args[1:4] np.testing.assert_allclose(array_arg, np.array([0.05, 0.15, 0.25])) move_xyz.assert_called_once() @@ -272,7 +299,7 @@ def test_individual_plans_triggered_once_and_only_once_in_composite_run( autospec=True, ) @patch("artemis.experiment_plans.fast_grid_scan_plan.run_gridscan", autospec=True) -@patch("artemis.experiment_plans.fast_grid_scan_plan.move_xyz", autospec=True) +@patch("artemis.experiment_plans.fast_grid_scan_plan.move_x_y_z", autospec=True) @patch("bluesky.plan_stubs.rd") def test_logging_within_plan( rd: MagicMock, @@ -311,7 +338,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] + array_arg = move_xyz.call_args.args[1:4] np.testing.assert_array_almost_equal(array_arg, np.array([0.05, 0.15, 0.25])) move_xyz.assert_called_once() diff --git a/src/artemis/experiment_plans/tests/test_rotation_scan_plan.py b/src/artemis/experiment_plans/tests/test_rotation_scan_plan.py index e789b965f..cc5a22ccb 100644 --- a/src/artemis/experiment_plans/tests/test_rotation_scan_plan.py +++ b/src/artemis/experiment_plans/tests/test_rotation_scan_plan.py @@ -58,7 +58,7 @@ TEST_SHUTTER_OPENING_DEGREES = 2.5 -def do_rotation_plan_for_tests( +def do_rotation_main_plan_for_tests( run_engine, callbacks, sim_und, @@ -99,6 +99,43 @@ def do_rotation_plan_for_tests( ) +@pytest.fixture +def run_full_rotation_plan( + RE: RunEngine, + test_rotation_params: RotationInternalParameters, + fake_create_rotation_devices, + attenuator: Attenuator, + mock_rotation_subscriptions: RotationCallbackCollection, + synchrotron: Synchrotron, + s4_slit_gaps: S4SlitGaps, + undulator: Undulator, + flux: Flux, +): + with ( + patch( + "bluesky.preprocessors.__read_and_stash_a_motor", + fake_read, + ), + patch( + "artemis.experiment_plans.rotation_scan_plan.create_devices", + lambda: fake_create_rotation_devices, + ), + patch( + "artemis.experiment_plans.rotation_scan_plan.RotationCallbackCollection.from_params", + lambda _: mock_rotation_subscriptions, + ), + patch("dodal.beamlines.i03.undulator", lambda: undulator), + patch("dodal.beamlines.i03.synchrotron", lambda: synchrotron), + patch("dodal.beamlines.i03.s4_slit_gaps", lambda: s4_slit_gaps), + patch("dodal.beamlines.i03.flux", lambda: flux), + patch("dodal.beamlines.i03.attenuator", lambda: attenuator), + ): + RE(get_plan(test_rotation_params)) + + fake_create_rotation_devices["test_rotation_params"] = test_rotation_params + return fake_create_rotation_devices + + def setup_and_run_rotation_plan_for_tests( RE: RunEngine, test_params: RotationInternalParameters, @@ -155,7 +192,7 @@ def side_set_w_return(obj, *args): zebra.pc.arm.arm_set.set = mock_arm with patch("bluesky.plan_stubs.wait", autospec=True): - do_rotation_plan_for_tests( + do_rotation_main_plan_for_tests( RE, mock_rotation_subscriptions, undulator, @@ -340,14 +377,13 @@ def test_rotation_plan_zebra_settings(setup_and_run_rotation_plan_for_tests_stan assert zebra.pc.pulse_start.get() == expt_params.shutter_opening_time_s -def test_rotation_plan_smargon_settings(setup_and_run_rotation_plan_for_tests_standard): - smargon: Smargon = setup_and_run_rotation_plan_for_tests_standard["smargon"] - params: RotationInternalParameters = setup_and_run_rotation_plan_for_tests_standard[ - "test_rotation_params" - ] +def test_full_rotation_plan_smargon_settings( + run_full_rotation_plan, +): + smargon: Smargon = run_full_rotation_plan["smargon"] + params: RotationInternalParameters = run_full_rotation_plan["test_rotation_params"] expt_params = params.experiment_params - omega_vel_set: MagicMock = smargon.omega.velocity.set omega_set: MagicMock = smargon.omega.set rotation_speed = ( expt_params.image_width / params.artemis_params.detector_params.exposure_time @@ -358,11 +394,11 @@ def test_rotation_plan_smargon_settings(setup_and_run_rotation_plan_for_tests_st assert smargon.x.user_readback.get() == expt_params.x assert smargon.y.user_readback.get() == expt_params.y assert smargon.z.user_readback.get() == expt_params.z - assert omega_vel_set.call_count == 3 - omega_vel_set.assert_has_calls( - [call(DEFAULT_MAX_VELOCITY), call(rotation_speed), call(DEFAULT_MAX_VELOCITY)] + assert omega_set.call_count == 6 + omega_set.assert_has_calls( + [call(DEFAULT_MAX_VELOCITY), call(rotation_speed), call(DEFAULT_MAX_VELOCITY)], + any_order=True, ) - assert omega_set.call_count == 2 def test_rotation_plan_smargon_doesnt_move_xyz_if_not_given_in_params( From e4336a0506008a9cad0df9cd717a1226cebe8fd1 Mon Sep 17 00:00:00 2001 From: David Perl Date: Mon, 7 Aug 2023 12:27:44 +0100 Subject: [PATCH 3/8] fix tests --- src/artemis/experiment_plans/tests/conftest.py | 4 ++-- .../tests/test_rotation_scan_plan.py | 13 ++++++++----- .../callbacks/ispyb_callback_base.py | 5 ++++- .../callbacks/rotation/ispyb_callback.py | 5 ----- .../callbacks/rotation/zocalo_callback.py | 3 ++- 5 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/artemis/experiment_plans/tests/conftest.py b/src/artemis/experiment_plans/tests/conftest.py index 0fe76b8f1..509b502d3 100644 --- a/src/artemis/experiment_plans/tests/conftest.py +++ b/src/artemis/experiment_plans/tests/conftest.py @@ -180,7 +180,7 @@ def fake_create_devices( eiger: EigerDetector, smargon: Smargon, zebra: Zebra, - detetctor_motion: DetectorMotion, + detector_motion: DetectorMotion, ): eiger.stage = MagicMock() eiger.unstage = MagicMock() @@ -197,7 +197,7 @@ def fake_create_devices( "eiger": eiger, "smargon": smargon, "zebra": zebra, - "detector_motion": detetctor_motion, + "detector_motion": detector_motion, "backlight": i03.backlight(fake_with_ophyd_sim=True), } return devices diff --git a/src/artemis/experiment_plans/tests/test_rotation_scan_plan.py b/src/artemis/experiment_plans/tests/test_rotation_scan_plan.py index cc5a22ccb..92a8ef9a5 100644 --- a/src/artemis/experiment_plans/tests/test_rotation_scan_plan.py +++ b/src/artemis/experiment_plans/tests/test_rotation_scan_plan.py @@ -92,9 +92,6 @@ def do_rotation_main_plan_for_tests( expt_params, sim_sgon, sim_zeb, - sim_bl, - sim_att, - sim_det, ) ) @@ -482,15 +479,20 @@ def test_cleanup_happens( @pytest.mark.s03 @patch("bluesky.plan_stubs.wait") @patch("artemis.external_interaction.callbacks.rotation.nexus_callback.NexusWriter") +@patch( + "artemis.external_interaction.callbacks.rotation.rotation_callback_collection.RotationZocaloHandlerCallback" +) def test_ispyb_deposition_in_plan( bps_wait, nexus_writer, - fake_create_devices, + zocalo_callback, + fake_create_rotation_devices, RE, test_rotation_params: RotationInternalParameters, fetch_comment, fetch_datacollection_attribute, undulator, + attenuator, synchrotron, s4_slit_gaps, flux, @@ -512,12 +514,13 @@ def test_ispyb_deposition_in_plan( with ( patch( "artemis.experiment_plans.rotation_scan_plan.create_devices", - lambda: fake_create_devices, + lambda: fake_create_rotation_devices, ), patch("dodal.beamlines.i03.undulator", return_value=undulator), patch("dodal.beamlines.i03.synchrotron", return_value=synchrotron), patch("dodal.beamlines.i03.s4_slit_gaps", return_value=s4_slit_gaps), patch("dodal.beamlines.i03.flux", return_value=flux), + patch("dodal.beamlines.i03.attenuator", return_value=attenuator), patch( "bluesky.preprocessors.__read_and_stash_a_motor", fake_read, diff --git a/src/artemis/external_interaction/callbacks/ispyb_callback_base.py b/src/artemis/external_interaction/callbacks/ispyb_callback_base.py index 0e210e0ea..74bd2c8af 100644 --- a/src/artemis/external_interaction/callbacks/ispyb_callback_base.py +++ b/src/artemis/external_interaction/callbacks/ispyb_callback_base.py @@ -80,5 +80,8 @@ def stop(self, doc: dict): LOGGER.debug("ISPyB handler received stop document.") exit_status = doc.get("exit_status") reason = doc.get("reason") - self.ispyb.end_deposition(exit_status, reason) set_dcgid_tag(None) + try: + self.ispyb.end_deposition(exit_status, reason) + except Exception: + LOGGER.info(f"Failed to finalise ISPyB deposition on stop document: {doc}") diff --git a/src/artemis/external_interaction/callbacks/rotation/ispyb_callback.py b/src/artemis/external_interaction/callbacks/rotation/ispyb_callback.py index 9eebeaf1e..4b6e6e4bd 100644 --- a/src/artemis/external_interaction/callbacks/rotation/ispyb_callback.py +++ b/src/artemis/external_interaction/callbacks/rotation/ispyb_callback.py @@ -3,7 +3,6 @@ from artemis.external_interaction.callbacks.ispyb_callback_base import ( BaseISPyBHandlerCallback, ) -from artemis.external_interaction.exceptions import ISPyBDepositionNotMade from artemis.external_interaction.ispyb.store_in_ispyb import StoreRotationInIspyb from artemis.log import set_dcgid_tag from artemis.parameters.plan_specific.fgs_internal_params import FGSInternalParameters @@ -42,8 +41,4 @@ def event(self, doc: dict): def stop(self, doc: dict): if doc.get("run_start") == self.uid_to_finalize_on: - if self.ispyb_ids == (None, None): - raise ISPyBDepositionNotMade( - "ISPyB deposition was not initialised at run start!" - ) super().stop(doc) diff --git a/src/artemis/external_interaction/callbacks/rotation/zocalo_callback.py b/src/artemis/external_interaction/callbacks/rotation/zocalo_callback.py index 4428816fe..b4435f2fe 100644 --- a/src/artemis/external_interaction/callbacks/rotation/zocalo_callback.py +++ b/src/artemis/external_interaction/callbacks/rotation/zocalo_callback.py @@ -22,6 +22,7 @@ def __init__( ): self.ispyb: RotationISPyBHandlerCallback = ispyb_handler self.zocalo_interactor = ZocaloInteractor(zocalo_environment) + self.run_uid = None def start(self, doc: dict): LOGGER.info("Zocalo handler received start document.") @@ -29,7 +30,7 @@ def start(self, doc: dict): self.run_uid = doc.get("uid") def stop(self, doc: dict): - if doc.get("run_start") == self.run_uid: + if self.run_uid and doc.get("run_start") == self.run_uid: LOGGER.info( f"Zocalo handler received stop document, for run {doc.get('run_start')}." ) From 90db10b0f40918ae644161fd19c11172cc4c9cba Mon Sep 17 00:00:00 2001 From: David Perl Date: Mon, 7 Aug 2023 13:05:55 +0100 Subject: [PATCH 4/8] remove unused function --- .../experiment_plans/rotation_scan_plan.py | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/src/artemis/experiment_plans/rotation_scan_plan.py b/src/artemis/experiment_plans/rotation_scan_plan.py index cb551a911..03e62f496 100644 --- a/src/artemis/experiment_plans/rotation_scan_plan.py +++ b/src/artemis/experiment_plans/rotation_scan_plan.py @@ -116,29 +116,6 @@ def set_speed(axis: EpicsMotor, image_width, exposure_time, wait=True): ) -@bpp.set_run_key_decorator("rotation_scan_setup") -@bpp.run_decorator(md={"subplan_name": "rotation_scan_setup"}) -def rotation_scan_experiment_setup( - params: RotationInternalParameters, - smargon: Smargon, - backlight: Backlight, - attenuator: Attenuator, - detector_motion: DetectorMotion, - **kwargs, -): - expt_params: RotationScanParams = params.experiment_params - - LOGGER.info("moving to start x, y, z if necessary") - yield from move_x_y_z( - smargon, expt_params.x, expt_params.y, expt_params.z, wait=True - ) - - transmission = params.artemis_params.ispyb_params.transmission_fraction - yield from setup_sample_environment( - detector_motion, backlight, attenuator, transmission - ) - - @bpp.set_run_key_decorator("rotation_scan_main") @bpp.run_decorator(md={"subplan_name": "rotation_scan_main"}) def rotation_scan_plan( From d234408e40838992bb8f763143794d707294ff02 Mon Sep 17 00:00:00 2001 From: David Perl Date: Mon, 7 Aug 2023 13:07:17 +0100 Subject: [PATCH 5/8] fix linting --- src/artemis/experiment_plans/rotation_scan_plan.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/artemis/experiment_plans/rotation_scan_plan.py b/src/artemis/experiment_plans/rotation_scan_plan.py index 03e62f496..fb4c76835 100644 --- a/src/artemis/experiment_plans/rotation_scan_plan.py +++ b/src/artemis/experiment_plans/rotation_scan_plan.py @@ -5,8 +5,6 @@ import bluesky.plan_stubs as bps import bluesky.preprocessors as bpp from dodal.beamlines import i03 -from dodal.devices.attenuator import Attenuator -from dodal.devices.backlight import Backlight from dodal.devices.detector import DetectorParams from dodal.devices.detector_motion import DetectorMotion from dodal.devices.eiger import DetectorParams, EigerDetector From 1f93bb883ece5ab5fc01e4e3f335761a5fd32e44 Mon Sep 17 00:00:00 2001 From: David Perl Date: Mon, 7 Aug 2023 13:54:26 +0100 Subject: [PATCH 6/8] fix small things --- src/artemis/experiment_plans/rotation_scan_plan.py | 5 ++++- .../experiment_plans/tests/test_fast_grid_scan_plan.py | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/artemis/experiment_plans/rotation_scan_plan.py b/src/artemis/experiment_plans/rotation_scan_plan.py index fb4c76835..7298e8d71 100644 --- a/src/artemis/experiment_plans/rotation_scan_plan.py +++ b/src/artemis/experiment_plans/rotation_scan_plan.py @@ -176,6 +176,7 @@ def rotation_scan_plan( LOGGER.info("wait for any previous moves...") # wait for all the setup tasks at once yield from bps.wait("setup_senv") + yield from bps.wait("move_x_y_z") yield from bps.wait("move_to_start") yield from bps.wait("setup_zebra") @@ -254,10 +255,12 @@ def rotation_with_cleanup_and_stage(params: RotationInternalParameters): params.experiment_params.x, params.experiment_params.y, params.experiment_params.z, + group="move_x_y_z", ) - LOGGER.info("setting up and staging eiger...") + yield from rotation_scan_plan(params, **devices) + LOGGER.info("setting up and staging eiger...") yield from rotation_with_cleanup_and_stage(params) yield from rotation_scan_plan_with_stage_and_cleanup(parameters) 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 02a8b6893..0408d970d 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 @@ -199,10 +199,10 @@ def test_results_adjusted_and_passed_to_move_xyz( ) mv_call_large = call( - fake_fgs_composite.sample_motors, 0.05, 0.15000000000000002, 0.25 + fake_fgs_composite.sample_motors, 0.05, pytest.approx(0.15), 0.25 ) mv_call_medium = call( - fake_fgs_composite.sample_motors, 0.05, 0.15000000000000002, 0.25 + fake_fgs_composite.sample_motors, 0.05, pytest.approx(0.15), 0.25 ) move_x_y_z.assert_has_calls( [mv_call_large, mv_call_large, mv_call_medium], any_order=True From 1c53fe13acac4d6c9cf3b73e080c1da21265969b Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Wed, 9 Aug 2023 16:31:05 +0100 Subject: [PATCH 7/8] (#834) Wait for final move to centre to complete --- src/artemis/experiment_plans/fast_grid_scan_plan.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/artemis/experiment_plans/fast_grid_scan_plan.py b/src/artemis/experiment_plans/fast_grid_scan_plan.py index 0e87e7e8c..e07d0c7f3 100755 --- a/src/artemis/experiment_plans/fast_grid_scan_plan.py +++ b/src/artemis/experiment_plans/fast_grid_scan_plan.py @@ -233,7 +233,7 @@ def run_gridscan_and_move( # once we have the results, go to the appropriate position artemis.log.LOGGER.info("Moving to centre of mass.") with TRACER.start_span("move_to_result"): - yield from move_x_y_z(fgs_composite.sample_motors, *xray_centre) + yield from move_x_y_z(fgs_composite.sample_motors, *xray_centre, wait=True) def get_plan( From 6a8be31ff1ccccdfb38a93f4eef2a3b20d290fe5 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Wed, 9 Aug 2023 16:55:29 +0100 Subject: [PATCH 8/8] (#834) Add expected wait to tests --- .../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 0408d970d..5edaf8ee3 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 @@ -199,10 +199,10 @@ def test_results_adjusted_and_passed_to_move_xyz( ) mv_call_large = call( - fake_fgs_composite.sample_motors, 0.05, pytest.approx(0.15), 0.25 + fake_fgs_composite.sample_motors, 0.05, pytest.approx(0.15), 0.25, wait=True ) mv_call_medium = call( - fake_fgs_composite.sample_motors, 0.05, pytest.approx(0.15), 0.25 + fake_fgs_composite.sample_motors, 0.05, pytest.approx(0.15), 0.25, wait=True ) move_x_y_z.assert_has_calls( [mv_call_large, mv_call_large, mv_call_medium], any_order=True