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

I24 ssx: Use updated PMAC ProgramRunner #501

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ dependencies = [
"ophyd == 1.9.0",
"ophyd-async >= 0.3a5",
"bluesky >= 1.13.0a4",
"dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@3f59ce090dd5c5f94f761b4b80a61cbd34d90564",
"dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@main",
]


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,10 @@
https://confluence.diamond.ac.uk/display/MXTech/Dynamics+and+fixed+targets.

Args:
parameters (FixedTargerParameters): The collection paramelters
parameters (FixedTargerParameters): The collection parameters.

Returns:
The estimated collection time, in s.
"""
buffer = 30
pump_setting = parameters.pump_repeat
Expand Down Expand Up @@ -278,54 +281,42 @@
def get_prog_num(
Copy link
Contributor

Choose a reason for hiding this comment

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

Could: I feel like the test against this could be improved a bit given we're editing the function quite a lot. It doesn't test all combinations and uses integers rather than enums

chip_type: ChipType, map_type: MappingType, pump_repeat: PumpProbeSetting
) -> int:
logger.info("Get Program Number")
if pump_repeat == PumpProbeSetting.NoPP:
if chip_type in [ChipType.Oxford, ChipType.OxfordInner]:
logger.info(
f"Pump_repeat: {str(pump_repeat)} \tOxford Chip: {str(chip_type)}"
)
if map_type == MappingType.NoMap:
logger.info("Map type 0 = None")
logger.info("Program number: 11")
return 11
elif map_type == MappingType.Lite:
logger.info("Map type 1 = Mapping Lite")
logger.info("Program number: 12")
return 12
elif map_type == MappingType.Full:
logger.info("Map type 2 = Full Mapping")
logger.info("Program number: 13") # once fixed return 13
msg = "Mapping Type FULL is broken as of 11.09.17"
logger.error(msg)
raise ValueError(msg)
Comment on lines -295 to -300
Copy link
Contributor

Choose a reason for hiding this comment

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

Should: This has been broken for 7 years? I feel like at this point we should remove it and then re-introduce when it actually does get added back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made a separate issue for it because there's a lot of code that raises this error if full mapping is selected, plus the edm bits to be removed, see #515 . Link added in code

else:
logger.debug(f"Unknown Mapping Type; map_type = {map_type}")
return 0
elif chip_type == ChipType.Custom:
logger.info(
f"Pump_repeat: {str(pump_repeat)} \tCustom Chip: {str(chip_type)}"
)
logger.info("Program number: 11")
return 11
elif chip_type == ChipType.Minichip:
logger.info(
f"Pump_repeat: {str(pump_repeat)} \tMini Oxford Chip: {str(chip_type)}"
)
logger.info("Program number: 11")
return 11
else:
logger.debug(f"Unknown chip_type, chip_tpe = {chip_type}")
return 0
elif pump_repeat in [
pp.value for pp in PumpProbeSetting if pp != PumpProbeSetting.NoPP
]:
logger.info(f"Pump_repeat: {str(pump_repeat)} \t Chip Type: {str(chip_type)}")
logger.info("Map Type = Mapping Lite with Pump Probe")
"""Get the motion program number based on the experiment parameters set by \
the user.
Any pump probe experiment will return program number 14 (assumes lite mapping).
For non pump probe experiments, the program number depends on the chip and map type:
- Custom, Mini and PSI chips, as well as Oxford chips with no map return 11
- Oxford chips with lite mapping return 12
- Oxford chips with full mapping should return 13. Currently disabled, will \
raise an error.
"""
logger.info("Get Program Number for the motion program.")
logger.info(f"Pump_repeat: {str(pump_repeat)} \t Chip Type: {str(chip_type)}")
if pump_repeat != PumpProbeSetting.NoPP:
logger.info("Assuming Map type = Mapping Lite.")
logger.info("Program number: 14")
return 14
else:
logger.warning(f"Unknown pump_repeat, pump_repeat = {pump_repeat}")
return 0

if chip_type not in [ChipType.Oxford, ChipType.OxfordInner]:
logger.info("Program number: 11")
return 11

if map_type == MappingType.NoMap:
logger.info(f"Map type: {str(map_type)}")
logger.info("Program number: 11")
return 11
if map_type == MappingType.Lite:
logger.info(f"Map type: {str(map_type)}")
logger.info("Program number: 12")
return 12
if map_type == MappingType.Full:
# TODO See https://github.com/DiamondLightSource/mx-bluesky/issues/515
logger.info(f"Map type: {str(map_type)}")
logger.info("Program number: 13")
# TODO once reinstated return 13
msg = "Full mapping is broken and currently disabled."
logger.error(msg)
raise ValueError(msg)


@log.log_on_entry
Expand Down Expand Up @@ -659,10 +650,6 @@
yield from bps.trigger(pmac.to_xyz_zero)
sleep(2.0)

prog_num = get_prog_num(
parameters.chip.chip_type, parameters.map_type, parameters.pump_repeat
)

# Now ready for data collection. Open fast shutter (zebra gate)
logger.info("Opening fast shutter.")
yield from open_fast_shutter(zebra)
Expand All @@ -681,11 +668,30 @@
wavelength,
)

timeout_time = calculate_collection_timeout(parameters)
logger.info(f"Run PMAC with program number {prog_num}")
yield from bps.abs_set(pmac.run_program, prog_num, timeout_time, wait=True)
yield from kickoff_and_complete_collection(pmac, parameters)

Check warning on line 671 in src/mx_bluesky/beamlines/i24/serial/fixed_target/i24ssx_Chip_Collect_py3v1.py

View check run for this annotation

Codecov / codecov/patch

src/mx_bluesky/beamlines/i24/serial/fixed_target/i24ssx_Chip_Collect_py3v1.py#L671

Added line #L671 was not covered by tests


def kickoff_and_complete_collection(pmac: PMAC, parameters: FixedTargetParameters):
prog_num = get_prog_num(
parameters.chip.chip_type, parameters.map_type, parameters.pump_repeat
)
yield from bps.abs_set(pmac.program_number, prog_num, group="setup_pmac")
# Calculate approx collection time
total_collection_time = calculate_collection_timeout(parameters)
logger.info(f"Estimated collection time: {total_collection_time}s.")
yield from bps.abs_set(
pmac.collection_time, total_collection_time, group="setup_pmac"
)
yield from bps.wait(group="setup_pmac") # Make sure the soft signals are set

@bpp.run_decorator(md={"subplan_name": "run_ft_collection"})
def run_collection():
logger.info(f"Kick off PMAC with program number {prog_num}.")
yield from bps.kickoff(pmac.run_program, wait=True)
yield from bps.complete(pmac.run_program, wait=True)
logger.info("Collection completed without errors.")

logger.debug("Collection completed without errors.")
yield from run_collection()


@log.log_on_entry
Expand Down
21 changes: 21 additions & 0 deletions tests/unit_tests/beamlines/i24/serial/fixed_target/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,24 @@ def dummy_params_without_pp():
"checker_pattern": False,
}
return FixedTargetParameters(**params)


@pytest.fixture
def dummy_params_with_pp():
oxford_defaults = get_chip_format(ChipType.Oxford)
params = {
"visit": "foo",
"directory": "bar",
"filename": "chip",
"exposure_time_s": 0.01,
"detector_distance_mm": 100,
"detector_name": "eiger",
"num_exposures": 1,
"chip": oxford_defaults.model_dump(),
"map_type": 1,
"pump_repeat": 3,
"checker_pattern": False,
"laser_dwell_s": 0.02,
"laser_delay_s": 0.05,
}
return FixedTargetParameters(**params)
Original file line number Diff line number Diff line change
@@ -1,18 +1,25 @@
import asyncio
from unittest.mock import ANY, MagicMock, call, mock_open, patch

import bluesky.plan_stubs as bps
import pytest
from bluesky.utils import FailedStatus
from dodal.devices.hutch_shutter import HutchShutter
from dodal.devices.i24.pmac import PMAC
from dodal.devices.zebra import Zebra
from ophyd_async.core import get_mock_put
from ophyd_async.core import callback_on_mock_put, get_mock_put, set_mock_value

from mx_bluesky.beamlines.i24.serial.fixed_target.ft_utils import MappingType
from mx_bluesky.beamlines.i24.serial.fixed_target.ft_utils import (
ChipType,
MappingType,
PumpProbeSetting,
)
from mx_bluesky.beamlines.i24.serial.fixed_target.i24ssx_Chip_Collect_py3v1 import (
datasetsizei24,
finish_i24,
get_chip_prog_values,
get_prog_num,
kickoff_and_complete_collection,
load_motion_program_data,
run_aborted_plan,
start_i24,
Expand Down Expand Up @@ -57,17 +64,25 @@ def test_get_chip_prog_values(dummy_params_without_pp):
@pytest.mark.parametrize(
"chip_type, map_type, pump_repeat, expected_prog",
[
(0, 0, 0, 11), # Oxford chip, full chip, no pump
(0, 1, 0, 12), # Oxford chip, map generated by mapping lite, no pump
(2, "", 0, 11), # Custom chip, map type not needed(full assumed), no pump
(0, "", 2, 14), # Oxford chip, assumes mapping lite, pump 2
(3, "", 0, 11), # Minichip, no map type, no pump probe
(ChipType.Oxford, MappingType.NoMap, PumpProbeSetting.NoPP, 11),
(ChipType.Oxford, MappingType.Lite, PumpProbeSetting.NoPP, 12),
(ChipType.OxfordInner, MappingType.Lite, PumpProbeSetting.NoPP, 12),
(ChipType.Custom, MappingType.Lite, PumpProbeSetting.NoPP, 11),
(ChipType.Minichip, MappingType.NoMap, PumpProbeSetting.NoPP, 11),
(ChipType.Oxford, MappingType.Lite, PumpProbeSetting.Short2, 14),
(ChipType.Minichip, MappingType.NoMap, PumpProbeSetting.Repeat5, 14),
(ChipType.Custom, MappingType.Lite, PumpProbeSetting.Medium1, 14),
],
)
def test_get_prog_number(chip_type, map_type, pump_repeat, expected_prog):
assert get_prog_num(chip_type, map_type, pump_repeat) == expected_prog


def test_get_prog_number_raises_error_for_disabled_map_setting():
with pytest.raises(ValueError):
get_prog_num(ChipType.Oxford, MappingType.Full, PumpProbeSetting.NoPP)


@pytest.mark.parametrize(
"map_type, pump_repeat, checker, expected_calls",
[
Expand Down Expand Up @@ -247,3 +262,46 @@ async def test_tidy_up_after_collection_plan(
fake_caput.assert_has_calls([call(ANY, 0), call(ANY, "Done")])

mock_finish.assert_called_once()


@patch(
"mx_bluesky.beamlines.i24.serial.fixed_target.i24ssx_Chip_Collect_py3v1.calculate_collection_timeout"
)
async def test_kick_off_and_complete_collection(
fake_collection_time, pmac, dummy_params_with_pp, RE, done_status
):
pmac.run_program.kickoff = MagicMock(return_value=done_status)
pmac.run_program.complete = MagicMock(return_value=done_status)

async def go_high_then_low():
set_mock_value(pmac.scanstatus, 1)
await asyncio.sleep(0.1)
set_mock_value(pmac.scanstatus, 0)

callback_on_mock_put(
pmac.pmac_string,
lambda *args, **kwargs: asyncio.create_task(go_high_then_low()), # type: ignore
)
fake_collection_time.return_value = 2.0
res = RE(kickoff_and_complete_collection(pmac, dummy_params_with_pp))

assert await pmac.program_number.get_value() == 14
assert await pmac.collection_time.get_value() == 2.0

pmac.run_program.kickoff.assert_called_once()
pmac.run_program.complete.assert_called_once()

assert res.exit_status == "success"


@patch(
"mx_bluesky.beamlines.i24.serial.fixed_target.i24ssx_Chip_Collect_py3v1.calculate_collection_timeout"
)
async def test_kickoff_and_complete_fails_if_scan_status_pv_does_not_change(
fake_collection_time, pmac, dummy_params_without_pp, RE
):
fake_collection_time.return_value = 1.0
pmac.run_program.KICKOFF_TIMEOUT = 0.1
set_mock_value(pmac.scanstatus, 0)
with pytest.raises(FailedStatus):
RE(kickoff_and_complete_collection(pmac, dummy_params_without_pp))
Loading