Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for box_size_um not included in robot_load_then_centre_plan. #763

Merged
merged 2 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ dependencies = [
"ophyd == 1.9.0",
"ophyd-async >= 0.8a5",
"bluesky >= 1.13",
"dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@main",
"dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@cb12746cb3f658a6d4571a0cd870133a17980bb7",
]


Expand Down
7 changes: 6 additions & 1 deletion src/mx_bluesky/common/parameters/gridscan.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,13 @@ class GridCommon(
DiffractionExperimentWithSample,
OptionalGonioAngleStarts,
):
"""Parameters used in every MX diffraction experiment using grids. This model should be used by plans which have no knowledge of the grid specifications - i.e before automatic grid detection has completed"""
"""
Parameters used in every MX diffraction experiment using grids. This model should
be used by plans which have no knowledge of the grid specifications - i.e before
automatic grid detection has completed
"""

box_size_um: float = Field(default=GridscanParamConstants.BOX_WIDTH_UM)
grid_width_um: float = Field(default=GridscanParamConstants.WIDTH_UM)
exposure_time_s: float = Field(default=GridscanParamConstants.EXPOSURE_TIME_S)

Expand Down
4 changes: 1 addition & 3 deletions src/mx_bluesky/hyperion/parameters/gridscan.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@
PandAGridScanParams,
ZebraGridScanParams,
)
from pydantic import Field

from mx_bluesky.common.parameters.constants import GridscanParamConstants
from mx_bluesky.common.parameters.gridscan import (
GridCommon,
SpecifiedThreeDGridScan,
Expand Down Expand Up @@ -146,4 +144,4 @@ class PinTipCentreThenXrayCentre(
class GridScanWithEdgeDetect(
GridCommonWithHyperionDetectorParams, WithHyperionUDCFeatures
):
box_size_um: float = Field(default=GridscanParamConstants.BOX_WIDTH_UM)
pass
7 changes: 4 additions & 3 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -527,10 +527,11 @@ def mirror_voltages():

@pytest.fixture
def undulator_dcm(RE, sim_run_engine, dcm):
undulator_dcm = i03.undulator_dcm(fake_with_ophyd_sim=True)
undulator_dcm = i03.undulator_dcm(
fake_with_ophyd_sim=True,
daq_configuration_path="tests/test_data/test_daq_configuration",
)
set_up_dcm(undulator_dcm.dcm_ref(), sim_run_engine)
undulator_dcm.roll_energy_table_path = "tests/test_data/test_daq_configuration/lookup/BeamLineEnergy_DCM_Roll_converter.txt"
undulator_dcm.pitch_energy_table_path = "tests/test_data/test_daq_configuration/lookup/BeamLineEnergy_DCM_Pitch_converter.txt"
yield undulator_dcm
beamline_utils.clear_devices()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,6 @@
"omega_start_deg": 0.0,
"grid_width_um": 290.6,
"transmission_frac": 1.0,
"visit": "cm31105-4"
"visit": "cm31105-4",
"box_size_um": 20
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
"demand_energy_ev": 11100,
"run_number": 0,
"features": {"use_panda_for_gridscan": false, "compare_cpu_and_gpu_zocalo": true},
"panda_runup_distance_mm": 0.17
"panda_runup_distance_mm": 0.17,
"box_size_um": 20
},
"multi_rotation_scan": {
"comment": "Rotation",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,6 @@
"tip_offset_um": 108.9,
"grid_width_um": 290.6,
"transmission_frac": 1.0,
"visit": "cm31105-4"
"visit": "cm31105-4",
"box_size_um": 20
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@
"sample_id": 12345,
"sample_puck": 40,
"sample_pin": 3,
"comment": "Descriptive comment."
"comment": "Descriptive comment.",
"box_size_um": 20
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
#######################
# #
# 5.5mm CPMU 20/11/22 #
# #
#######################
# Used to convert from energy to gap. Constructed from tables for 3rd, 5th and 7th harmonic.
# It is important that at the point of change from one harmonic to another that there is
# point for the same energy from both harmomics to prevent invalid interpolation.
# run reloadLookupTables() when done
Units eV mm
5700 5.4606
5760 5.5
6000 5.681
6500 6.045
7000 6.404
7500 6.765
8000 7.124
8500 7.491
9000 7.872
9500 8.258
9700 8.424
9700 5.542
10000 5.675
10500 5.895
11000 6.113
11500 6.328
12000 6.545
12500 6.758
12700 6.83
13000 6.98
13443 7.168
13443 5.5
13500 5.517
14000 5.674
14500 5.831
15000 5.987
15500 6.139
16000 6.294
16500 6.447
17000 6.603
17320 6.697
17320 5.5
17500 5.552
18000 5.674
18500 5.794
19000 5.912
19500 6.037
20000 6.157
20500 6.277
20939 6.378
20939 5.5
21000 5.517
21500 5.577
22000 5.674
22500 5.773
23000 5.871
23500 5.97
24000 6.072
24500 6.167
25000 6.264
29 changes: 27 additions & 2 deletions tests/unit_tests/hyperion/experiment_plans/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ def robot_load_composite(
backlight,
detector_motion,
flux,
ophyd_pin_tip_detection,
pin_tip_detection_with_found_pin,
zocalo,
synchrotron,
sample_shutter,
Expand All @@ -305,6 +305,7 @@ def robot_load_composite(
set_mock_value(dcm.energy_in_kev.user_readback, 11.105)
smargon.stub_offsets.set = MagicMock(return_value=NullStatus())
aperture_scatterguard.set = MagicMock(return_value=NullStatus())
set_mock_value(smargon.omega.max_velocity, 131)
return RobotLoadThenCentreComposite(
xbpm_feedback=xbpm_feedback,
attenuator=attenuator,
Expand All @@ -316,7 +317,7 @@ def robot_load_composite(
zebra_fast_grid_scan=fast_grid_scan,
flux=flux,
oav=oav,
pin_tip_detection=ophyd_pin_tip_detection,
pin_tip_detection=pin_tip_detection_with_found_pin,
smargon=smargon,
synchrotron=synchrotron,
s4_slit_gaps=s4_slit_gaps,
Expand Down Expand Up @@ -376,3 +377,27 @@ def msg_maches_run(msg: Msg):
return msg.run == run_name

sim_run_engine.add_handler("open_run", fire_event, msg_maches_run)


@pytest.fixture
def grid_detection_callback_with_detected_grid():
with patch(
"mx_bluesky.hyperion.experiment_plans.grid_detect_then_xray_centre_plan.GridDetectionCallback",
autospec=True,
) as callback:
callback.return_value.get_grid_parameters.return_value = {
"transmission_frac": 1.0,
"exposure_time_s": 0,
"x_start_um": 0,
"y_start_um": 0,
"y2_start_um": 0,
"z_start_um": 0,
"z2_start_um": 0,
"x_steps": 10,
"y_steps": 10,
"z_steps": 10,
"x_step_size_um": 0.1,
"y_step_size_um": 0.1,
"z_step_size_um": 0.1,
}
yield callback
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,15 @@
from typing import Any
from unittest.mock import AsyncMock, MagicMock, call, patch

import numpy
import pytest
from bluesky.protocols import Location
from bluesky.run_engine import RunEngine
from bluesky.simulators import RunEngineSimulator, assert_message_and_return_remaining
from bluesky.utils import Msg
from dodal.devices.i03.beamstop import BeamstopPositions
from dodal.devices.oav.oav_parameters import OAVParameters
from dodal.devices.oav.pin_image_recognition import PinTipDetection
from dodal.devices.synchrotron import SynchrotronMode
from ophyd.sim import NullStatus
from ophyd_async.testing import set_mock_value
from pydantic import ValidationError

Expand Down Expand Up @@ -50,14 +49,6 @@
GOOD_TEST_LOAD_CENTRE_COLLECT_MULTI_ROTATION = "tests/test_data/parameter_json_files/good_test_load_centre_collect_params_multi_rotation.json"


def find_a_pin(pin_tip_detection):
def set_good_position():
set_mock_value(pin_tip_detection.triggered_tip, numpy.array([100, 110]))
return NullStatus()

return set_good_position


@pytest.fixture
def composite(
robot_load_composite,
Expand Down Expand Up @@ -143,30 +134,6 @@ def load_centre_collect_with_top_n_params():
return LoadCentreCollect(**params)


@pytest.fixture
def grid_detection_callback_with_detected_grid():
with patch(
"mx_bluesky.hyperion.experiment_plans.grid_detect_then_xray_centre_plan.GridDetectionCallback",
autospec=True,
) as callback:
callback.return_value.get_grid_parameters.return_value = {
"transmission_frac": 1.0,
"exposure_time_s": 0,
"x_start_um": 0,
"y_start_um": 0,
"y2_start_um": 0,
"z_start_um": 0,
"z2_start_um": 0,
"x_steps": 10,
"y_steps": 10,
"z_steps": 10,
"x_step_size_um": 0.1,
"y_step_size_um": 0.1,
"z_step_size_um": 0.1,
}
yield callback


def test_can_serialize_load_centre_collect_params(load_centre_collect_params):
load_centre_collect_params.model_dump_json()

Expand Down Expand Up @@ -632,3 +599,23 @@ def test_load_centre_collect_creates_storage_directory_if_not_present(
[call("/tmp/dls/i03/data/2024/cm31105-4/auto/123458/", exist_ok=True)],
any_order=True,
)


def test_box_size_passed_through_to_gridscan(
composite: LoadCentreCollectComposite,
load_centre_collect_params: LoadCentreCollect,
oav_parameters_for_rotation: OAVParameters,
RE: RunEngine,
):
load_centre_collect_params.robot_load_then_centre.box_size_um = 25
with patch(
"mx_bluesky.hyperion.experiment_plans.pin_centre_then_xray_centre_plan.detect_grid_and_do_gridscan"
) as mock_detect_grid:
Comment on lines +611 to +613
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I normally prefer using the patch decorator, unless there's a specific reason you can't (which I don't think there is in this case?) as it leads to less indentation and I feel like keeps the test tidier but I get that's probably personal preference

with pytest.raises(AssertionError, match="Flyscan result event not received.*"):
RE(
load_centre_collect_full(
composite, load_centre_collect_params, oav_parameters_for_rotation
)
)
detect_grid_call = mock_detect_grid.mock_calls[0]
assert detect_grid_call.args[1].box_size_um == 25
Original file line number Diff line number Diff line change
Expand Up @@ -532,3 +532,23 @@ def test_robot_load_then_centre_fails_with_exception_when_no_beamstop(
sim_run_engine.simulate_plan(
robot_load_then_centre(robot_load_composite, robot_load_then_centre_params)
)


def test_box_size_passed_through_to_gridscan(
robot_load_composite: RobotLoadThenCentreComposite,
robot_load_then_centre_params: RobotLoadThenCentre,
grid_detection_callback_with_detected_grid: MagicMock,
RE: RunEngine,
):
robot_load_then_centre_params.box_size_um = 25
with patch(
"mx_bluesky.hyperion.experiment_plans.pin_centre_then_xray_centre_plan.detect_grid_and_do_gridscan"
) as mock_detect_grid:
RE(
robot_load_then_centre(
robot_load_composite,
robot_load_then_centre_params,
)
)
detect_grid_call = mock_detect_grid.mock_calls[0]
assert detect_grid_call.args[1].box_size_um == 25
53 changes: 47 additions & 6 deletions tests/unit_tests/hyperion/test_main_system.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,9 @@ def wait_for_run_engine_status(

def check_status_in_response(response_object, expected_result: Status):
response_json = json.loads(response_object.data)
assert response_json["status"] == expected_result.value
assert response_json["status"] == expected_result.value, (
f"{response_json['status']} != {expected_result.value}: {response_json.get('message')}"
)


def test_start_gives_success(test_env: ClientAndRunEngine):
Expand Down Expand Up @@ -290,14 +292,53 @@ def test_when_started_n_returnstatus_interrupted_bc_RE_aborted_thn_error_reptd(
assert response_json["exception_type"] == "Exception"


def test_start_with_json_file_gives_success(test_env: ClientAndRunEngine):
@pytest.mark.parametrize(
"endpoint, test_file",
[
[
START_ENDPOINT,
"tests/test_data/parameter_json_files/good_test_parameters.json",
],
[
"/grid_detect_then_xray_centre/start",
"tests/test_data/parameter_json_files/good_test_grid_with_edge_detect_parameters"
".json",
Comment on lines +304 to +305
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Having the .json on a separate line made me think it was a new arg at first glance. I think it's more readable together, even if it is longer than the line length

],
[
"/rotation_scan/start",
"tests/test_data/parameter_json_files/good_test_rotation_scan_parameters"
".json",
],
[
"/pin_tip_centre_then_xray_centre/start",
"tests/test_data/parameter_json_files/good_test_pin_centre_then_xray_centre_parameters"
".json",
],
[
"/robot_load_then_centre/start",
"tests/test_data/parameter_json_files/good_test_robot_load_and_centre_params"
".json",
],
[
"/multi_rotation_scan/start",
"tests/test_data/parameter_json_files/good_test_multi_rotation_scan_parameters"
".json",
],
[
"/load_centre_collect_full/start",
"tests/test_data/parameter_json_files/good_test_load_centre_collect_params"
".json",
],
],
)
def test_start_with_json_file_gives_success(
test_env: ClientAndRunEngine, endpoint: str, test_file: str
):
test_env.mock_run_engine.RE_takes_time = False

with open(
"tests/test_data/parameter_json_files/good_test_parameters.json"
) as test_params_file:
with open(test_file) as test_params_file:
test_params = test_params_file.read()
response = test_env.client.put(START_ENDPOINT, data=test_params)
response = test_env.client.put(endpoint, data=test_params)
check_status_in_response(response, Status.SUCCESS)


Expand Down
Loading