Skip to content

Bugfixes #95

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.

Sign up for GitHub

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

Open
jacob-williamson wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Bugfixes #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2dcdf83
Bugfixes
jacob-williamson Aug 19, 2026
8ae0bd8
Get background time per pdf time from max in queue
jacob-williamson Aug 20, 2026
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
5 changes: 4 additions & 1 deletion src/daq_queuing_service/plugins/converter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@
from daq_queuing_service.task_queue.task import Experiment, Task, TaskWithPosition


class ConverterError(Exception): ...
class ConverterError(Exception):
def __init__(self, original: Exception):
super().__init__(f"{type(original).__name__}: {original}")
self.original = original


class ValidateError(Exception): ...
Expand Down
4 changes: 2 additions & 2 deletions src/daq_queuing_service/plugins/i15_1/backgrounds.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ class BackgroundInfo(BaseModel):
# https://github.com/DiamondLightSource/daq-queuing-service/issues/84
model_config = ConfigDict(frozen=True)
bg_type: BACKGROUND_TYPES
time_per_pdf: int

def add_tiled_id(self, tiled_id: str) -> "TiledBackground":
return TiledBackground(
bg_type=self.bg_type,
tiled_id=tiled_id,
bg_type=self.bg_type, tiled_id=tiled_id, time_per_pdf=self.time_per_pdf
)


Expand Down
23 changes: 19 additions & 4 deletions src/daq_queuing_service/plugins/i15_1/i15_1_converter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ def _construct_blueapi_tasks_from_experiment(
self,
experiment: Experiment,
) -> list[TaskRequest]:
LOGGER.debug(f"Converting to blueapi calls, experiment = {experiment}")
sample_name = experiment.sample.name
# Assume sample name is of form test_8_1 to load from position 8 on puck 1
_, position, puck = sample_name.split("_")
Expand Down Expand Up @@ -155,14 +156,24 @@ def _add_required_background_scans(self, tasks: list[Task]) -> list[Task]:
# This can be made more robust https://github.com/DiamondLightSource/daq-queuing-service/issues/80
new_tasks: list[Task] = []

pdf_times = [
task.experiment.experiment_definition.data["time_per_pdf"]
for task in tasks
if isinstance(task.experiment, Experiment)
and "time_per_pdf" in task.experiment.experiment_definition.data
]
max_time_per_pdf = max(pdf_times) if pdf_times else 10

for task in tasks:
experiment = task.experiment
if (
isinstance(experiment, Experiment)
and experiment.name != BACKGROUND_SCAN
):
instrument_session = experiment.instrument_session
backgrounds = self._get_required_backgrounds(experiment)
backgrounds = self._get_required_backgrounds(
experiment, max_time_per_pdf
)

for background in backgrounds:
if tiled_id := get_background_tiled_id(
Expand Down Expand Up @@ -199,9 +210,11 @@ def _remove_repeated_backgrounds(self, tasks: list[Task]) -> list[Task]:
LOGGER.debug(f"Removing repeated background scan: {task.experiment}")
return new_tasks

def _get_required_backgrounds(self, experiment: Experiment) -> list[BackgroundInfo]:
def _get_required_backgrounds(
self, experiment: Experiment, time_per_pdf: int
) -> list[BackgroundInfo]:
# This should be fleshed out https://github.com/DiamondLightSource/daq-queuing-service/issues/79
return [BackgroundInfo(bg_type="fq")]
return [BackgroundInfo(bg_type="fq", time_per_pdf=time_per_pdf)]

def _add_tiled_background_to_md(
self, params: dict[str, Any], tiled_id: str, background: BackgroundInfo
Expand All @@ -225,6 +238,8 @@ def _construct_background_experiment(
# Need to get sample info for test samples (air, empty capillary etc)
sample=Sample(name="fq_1_1", id="", data={}),
experiment_definition=ExperimentDefinition(
name="background_scan", id="", data={"background": background}
name="background_scan",
id="",
data={"background": background, "time_per_pdf": 10},
),
)
21 changes: 14 additions & 7 deletions src/daq_queuing_service/task_queue/queue.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ def _sync(self):
modified, and also right before a call is popped off the front of the queue.
"""
LOGGER.debug("Syncing")
LOGGER.debug(f"Queue before sync: {self._queue}")
for task_id in list(self._queue):
task = self._tasks[task_id]
if task.status in (Status.COMPLETE, Status.ERROR):
Expand All @@ -162,7 +163,7 @@ def _sync(self):
self._queue_history,
)
except Exception as e:
raise ConverterError(*e.args) from e
raise ConverterError(e) from e

# Update task_registry to match new tasks
# Not needed as long as pre_process modifies in place
Expand Down Expand Up @@ -200,7 +201,7 @@ def _sync(self):
self._queue_history,
)
except Exception as e:
raise ConverterError(*e.args) from e
raise ConverterError(e) from e

self._call_queue.extend(new_calls)

Expand All @@ -215,6 +216,7 @@ def _sync(self):
self._save_contents()
self._broadcast_changes()
self._modifying.notify_all()
LOGGER.debug(f"Queue after sync: {self._queue}")

def _copy_contents(self) -> QueueContents:
return deepcopy(
Expand All @@ -231,13 +233,18 @@ def _save_contents(self):
self._last_good_contents = self._copy_contents()

def _restore_from_contents(self, contents: QueueContents):
self._tasks = TaskRegistry(contents["tasks"])
self._queue = contents["queue"]
self._history = contents["history"]
self._call_queue = contents["call_queue"]
self._call_history = contents["call_history"]
LOGGER.info(f"Restoring to contents: {contents}")

restored = deepcopy(contents)

self._tasks = TaskRegistry(restored["tasks"])
self._queue = restored["queue"]
self._history = restored["history"]
self._call_queue = restored["call_queue"]
self._call_history = restored["call_history"]

def _restore_latest_good_contents(self):
LOGGER.info("Restoring to last good contents")
self._restore_from_contents(self._last_good_contents)

def _broadcast_changes(self):
Expand Down
8 changes: 4 additions & 4 deletions tests/unit_tests/plugins/i15-1/test_get_background_tiled_id.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def test_get_background_tiled_id_makes_expected_searches(
client, search_2, search_3 = mock_tiled_searches
get_background_tiled_id(
client,
BackgroundInfo(bg_type="air"),
BackgroundInfo(bg_type="air", time_per_pdf=10),
instrument_session="cm12345-1",
)
client.search.assert_called_once_with(
Expand All @@ -51,7 +51,7 @@ def test_get_background_tiled_id_makes_expected_searches(
search_3.search.assert_called_once_with(
Eq(
key="start.experiment_definition.metadata.background",
value='{"bg_type":"air"}',
value='{"bg_type":"air","time_per_pdf":10}',
)
)

Expand All @@ -63,7 +63,7 @@ def test_get_background_tiled_returns_most_recent_valid_background(
assert (
get_background_tiled_id(
client,
BackgroundInfo(bg_type="air"),
BackgroundInfo(bg_type="air", time_per_pdf=10),
instrument_session="cm12345-1",
)
== "tiled_id_2"
Expand All @@ -78,7 +78,7 @@ def test_get_background_tiled_id_returns_none_if_no_matching_backgrounds_found(
assert (
get_background_tiled_id(
client,
BackgroundInfo(bg_type="air"),
BackgroundInfo(bg_type="air", time_per_pdf=10),
instrument_session="cm12345-1",
)
is None
Expand Down
81 changes: 66 additions & 15 deletions tests/unit_tests/plugins/i15-1/test_i15_1_converter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
from blueapi.service.model import TaskRequest

from daq_queuing_service.blueapi_interaction.blueapi_call import BlueapiCall
from daq_queuing_service.broadcaster import Broadcaster
from daq_queuing_service.plugins.i15_1.backgrounds import BackgroundInfo
from daq_queuing_service.plugins.i15_1.i15_1_converter import I151Converter
from daq_queuing_service.task_queue.queue import TaskQueue
from daq_queuing_service.task_queue.task import (
Experiment,
ExperimentDefinition,
Expand All @@ -27,6 +29,34 @@ def assert_tasks_equal(task1: Task | TaskWithPosition, task2: Task | TaskWithPos
assert task1 == task2


@pytest.fixture
async def queue_with_i15_1_plugin(background_not_found_in_tiled: None):
queue = TaskQueue(converter=I151Converter(), broadcaster=Broadcaster())
tasks = [
Task(
experiment=Experiment(
name=f"task_{i}",
instrument_session="cm12345-1",
experiment_definition=ExperimentDefinition(
name="",
id="",
data={
"list_of_temperatures": [100 * i, 100 * i + 20],
"time_per_pdf": i,
"settle_time": 5,
"ramp_rate": 10,
},
),
sample=Sample(name=f"sample_{i}_2", id=str(i), data={}),
)
)
for i in range(5)
]
await queue.add_tasks(tasks)
await queue.resume_queue()
return queue


@pytest.fixture(autouse=True)
def background_found_in_tiled():
with patch(
Expand Down Expand Up @@ -256,7 +286,10 @@ def test_if_no_background_found_in_tiled_then_background_scan_added_to_tasks(
"experiment_definition": {
"name": "background_scan",
"id": "",
"data": {"background": {"bg_type": "fq"}},
"data": {
"background": {"bg_type": "fq", "time_per_pdf": 100},
"time_per_pdf": 10,
},
},
},
"id": "",
Expand All @@ -270,11 +303,13 @@ def test_if_no_background_found_in_tiled_then_background_scan_added_to_tasks(
def test_add_required_background_scans_does_not_add_the_same_background_twice(
tasks: list[Task], background_not_found_in_tiled: None
):
bg_1 = BackgroundInfo(bg_type="air")
bg_2 = BackgroundInfo(bg_type="bs")
bg_3 = BackgroundInfo(bg_type="fq")
bg_1 = BackgroundInfo(bg_type="air", time_per_pdf=5)
bg_2 = BackgroundInfo(bg_type="bs", time_per_pdf=10)
bg_3 = BackgroundInfo(bg_type="fq", time_per_pdf=15)

def fake_get_required_background(self: I151Converter, experiment: Experiment):
def fake_get_required_background(
self: I151Converter, experiment: Experiment, max_time_per_pdf: int
):
# Get the same background scans every other experiment
# Only one of each background should be added
if int(experiment.sample.id) % 2 == 0:
Expand Down Expand Up @@ -337,7 +372,10 @@ def test_same_experiment_in_different_instrument_sessions_will_add_background_in
"experiment_definition": {
"name": "background_scan",
"id": "",
"data": {"background": {"bg_type": "fq"}},
"data": {
"background": {"bg_type": "fq", "time_per_pdf": 10},
"time_per_pdf": 10,
},
},
},
"id": "",
Expand All @@ -355,7 +393,10 @@ def test_same_experiment_in_different_instrument_sessions_will_add_background_in
"experiment_definition": {
"name": "background_scan",
"id": "",
"data": {"background": {"bg_type": "fq"}},
"data": {
"background": {"bg_type": "fq", "time_per_pdf": 10},
"time_per_pdf": 10,
},
},
},
"id": "",
Expand All @@ -380,36 +421,40 @@ def test_add_required_background_scans_if_found_in_tiled_then_no_background_adde
(
{"sample": "my_sample"},
["tiled_id"],
[BackgroundInfo(bg_type="bs")],
[BackgroundInfo(bg_type="bs", time_per_pdf=5)],
{
"metadata": {
"tiled_backgrounds": {"tiled_id": BackgroundInfo(bg_type="bs")}
"tiled_backgrounds": {
"tiled_id": BackgroundInfo(bg_type="bs", time_per_pdf=5)
}
},
"sample": "my_sample",
},
),
(
{},
["tiled_id"],
[BackgroundInfo(bg_type="bs")],
[BackgroundInfo(bg_type="bs", time_per_pdf=5)],
{
"metadata": {
"tiled_backgrounds": {"tiled_id": BackgroundInfo(bg_type="bs")}
"tiled_backgrounds": {
"tiled_id": BackgroundInfo(bg_type="bs", time_per_pdf=5)
}
},
},
),
(
{"sample": "my_sample"},
["tiled_id_1", "tiled_id_2"],
[
BackgroundInfo(bg_type="bs"),
BackgroundInfo(bg_type="air"),
BackgroundInfo(bg_type="bs", time_per_pdf=5),
BackgroundInfo(bg_type="air", time_per_pdf=5),
],
{
"metadata": {
"tiled_backgrounds": {
"tiled_id_1": BackgroundInfo(bg_type="bs"),
"tiled_id_2": BackgroundInfo(bg_type="air"),
"tiled_id_1": BackgroundInfo(bg_type="bs", time_per_pdf=5),
"tiled_id_2": BackgroundInfo(bg_type="air", time_per_pdf=5),
}
},
"sample": "my_sample",
Expand All @@ -427,3 +472,9 @@ def test_add_tiled_background_to_md_adds_expected_metadata(
I151Converter()._add_tiled_background_to_md(params, tiled_id, background)

assert params == expected_params


async def test_queue_with_i15_1_converter_can_sync(queue_with_i15_1_plugin: TaskQueue):
first_task = await queue_with_i15_1_plugin.get_task_by_position(0)
assert first_task
await queue_with_i15_1_plugin.move_task(first_task.id, 2)
2 changes: 1 addition & 1 deletion tests/unit_tests/test_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ def fail_conversion(
assert response.status_code == 422
assert response.json() == {
"error": "converter_error",
"message": "Conversion failed because xyz",
"message": "SomeError: Conversion failed because xyz",
}
assert task_queue_with_history._queue == ["2", "3", "4"]

Expand Down
Loading