Skip to content

Commit

Permalink
fix(app,api): Display thermocycler profile cycles (#16414)
Browse files Browse the repository at this point in the history
We identified a simple bug with the app: it displays wonky numbers for
thermocycler profile cycles:
![image
(1)](https://github.com/user-attachments/assets/8005c0b6-c55e-4b48-bf50-7d90f91fa63b)

This wouldn't repeat anything at all, what's going on?

Well, it turns out that when Protocol Designer added thermocycler
support, they wanted something a bit more in depth than the Python
protocol API's "list of steps, number of times to repeat the cycle"
format. They wanted users to be able to add single steps and cycles, in
arbitrary order.

To make the two styles work with the engine, we made the engine's
`thermocycler/runProfile` command take a flat list of steps - any of the
structure of repeated commands was removed. In the app's `CommandText`
display, we then had to fix up the way we displayed profiles to remove
references to a `repetitions` value that now didn't exist... and we just
didn't, instead setting the `repetitions` element to just be the number
of steps in the profile.

Fixing this ended up being a bit involved.

## summary
- ad17260 Make a new interface for the
hardware controller `execute_profile` function of the thermocycler,
since it's important that thermocycler cycles execute inside an asyncio
worker so that they won't be interrupted by e.g. pausing. This new
interface takes the PD style of protocol. It relies under the hood on
the same function that actually sends stuff to the thermocycler, so
there shouldn't be any functional execution changes.
- 335e25a Make a new
`thermocycler/runExtendedProfile` command in the protocol engine, that
takes as its parameters a structured list of either steps or cycles,
adhering to PD's expectations since the PAPI's expectations are a strict
subset of PD. This implements its command by using the new hardware
controller.
- e842a2a Use the new structured data
to implement a command text for the new command that can render the
equivalent of what PD makes (and, again, what the PAPI does, which is a
strict subset)
- e842a2a Also make the old command
text not use a "repetitions" keyword that doesn't exist
- 0043ab7 Switch the PAPI to emitting
the new command in api 2.21

## new UI
These UI changes will be on
https://s3-us-west-2.amazonaws.com/opentrons-components/rqa-2771-thermocycler-extended-profiles/index.html?path=/docs/app-molecules-command-commandtext--docs
for the desktop and general text and
https://s3-us-west-2.amazonaws.com/opentrons-components/rqa-2771-thermocycler-extended-profiles/index.html?path=/docs/app-molecules-command-command--docs
for the ODD colored-background elements whenever they upload. Here's a
screenshot of what they were like when the PR was opened:

`CommandText` of `thermocycler/runProfile` :

![runProfileCommandText](https://github.com/user-attachments/assets/dcad4ce8-2f0e-418d-8618-066a084dc832)

Note the change to the first line. This needs wordsmithing.

`CommandText` of `thermocycler/runExtendedProfile`:

![runProfileExtended](https://github.com/user-attachments/assets/6b1a1977-9200-4d78-84dc-2961d76cfff9)

Note the two-level rendering. The common case will probably be that
there's only cycles

## todo
- [x] wordsmith
- ~[ ] add to PD?~ no, but structured in a way that it will be easy
eventually
- [x] Test. Oh boy

Closes RQA-2771

---------

Co-authored-by: Ed Cormany <[email protected]>
  • Loading branch information
sfoster1 and ecormany authored Oct 8, 2024
1 parent 519482d commit 9d16384
Show file tree
Hide file tree
Showing 30 changed files with 6,132 additions and 89 deletions.
61 changes: 59 additions & 2 deletions api/src/opentrons/hardware_control/modules/thermocycler.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import asyncio
import logging
from typing import Callable, Optional, List, Dict, Mapping
from typing import Callable, Optional, List, Dict, Mapping, Union, cast
from opentrons.drivers.rpi_drivers.types import USBPort
from opentrons.drivers.types import ThermocyclerLidStatus, Temperature, PlateTemperature
from opentrons.hardware_control.modules.lid_temp_status import LidTemperatureStatus
Expand Down Expand Up @@ -363,6 +363,39 @@ async def cycle_temperatures(
self.make_cancellable(task)
await task

async def execute_profile(
self,
profile: List[Union[types.ThermocyclerCycle, types.ThermocyclerStep]],
volume: Optional[float] = None,
) -> None:
"""Begin a set temperature profile, with both repeating and non-repeating steps.
Args:
profile: The temperature profile to follow.
volume: Optional volume
Returns: None
"""
await self.wait_for_is_running()
self._total_cycle_count = 0
self._total_step_count = 0
self._current_cycle_index = 0
self._current_step_index = 0
for step_or_cycle in profile:
if "steps" in step_or_cycle:
# basically https://github.com/python/mypy/issues/14766
this_cycle = cast(types.ThermocyclerCycle, step_or_cycle)
self._total_cycle_count += this_cycle["repetitions"]
self._total_step_count += (
len(this_cycle["steps"]) * this_cycle["repetitions"]
)
else:
self._total_step_count += 1
self._total_cycle_count += 1
task = self._loop.create_task(self._execute_profile(profile, volume))
self.make_cancellable(task)
await task

async def set_lid_temperature(self, temperature: float) -> None:
"""Set the lid temperature in degrees Celsius"""
await self.wait_for_is_running()
Expand Down Expand Up @@ -574,7 +607,7 @@ async def _execute_cycles(
self,
steps: List[types.ThermocyclerStep],
repetitions: int,
volume: Optional[float] = None,
volume: Optional[float],
) -> None:
"""
Execute cycles.
Expand All @@ -592,6 +625,30 @@ async def _execute_cycles(
self._current_step_index = step_idx + 1 # science starts at 1
await self._execute_cycle_step(step, volume)

async def _execute_profile(
self,
profile: List[Union[types.ThermocyclerCycle, types.ThermocyclerStep]],
volume: Optional[float],
) -> None:
"""
Execute profiles.
Profiles command a thermocycler pattern that can contain multiple cycles and out-of-cycle steps.
"""
self._current_cycle_index = 0
self._current_step_index = 0
for step_or_cycle in profile:
self._current_cycle_index += 1
if "repetitions" in step_or_cycle:
# basically https://github.com/python/mypy/issues/14766
this_cycle = cast(types.ThermocyclerCycle, step_or_cycle)
for rep in range(this_cycle["repetitions"]):
for step in this_cycle["steps"]:
self._current_step_index += 1
await self._execute_cycle_step(step, volume)
else:
await self._execute_cycle_step(step_or_cycle, volume)

# TODO(mc, 2022-10-13): why does this exist?
# Do the driver and poller really need to be disconnected?
# Could we accomplish the same thing by latching the error state
Expand Down
5 changes: 5 additions & 0 deletions api/src/opentrons/hardware_control/modules/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ class ThermocyclerStep(ThermocyclerStepBase, total=False):
hold_time_minutes: float


class ThermocyclerCycle(TypedDict):
steps: List[ThermocyclerStep]
repetitions: int


UploadFunction = Callable[[str, str, Dict[str, Any]], Awaitable[Tuple[bool, str]]]


Expand Down
56 changes: 48 additions & 8 deletions api/src/opentrons/protocol_api/core/engine/module_core.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
"""Protocol API module implementation logic."""
from __future__ import annotations

from typing import Optional, List, Dict
from typing import Optional, List, Dict, Union

from opentrons.hardware_control import SynchronousAdapter, modules as hw_modules
from opentrons.hardware_control.modules.types import (
ModuleModel,
TemperatureStatus,
MagneticStatus,
ThermocyclerStep,
SpeedStatus,
module_model_from_string,
)
Expand All @@ -27,7 +26,7 @@
CannotPerformModuleAction,
)

from opentrons.protocols.api_support.types import APIVersion
from opentrons.protocols.api_support.types import APIVersion, ThermocyclerStep

from ... import validation
from ..module import (
Expand Down Expand Up @@ -327,15 +326,13 @@ def wait_for_lid_temperature(self) -> None:
cmd.thermocycler.WaitForLidTemperatureParams(moduleId=self.module_id)
)

def execute_profile(
def _execute_profile_pre_221(
self,
steps: List[ThermocyclerStep],
repetitions: int,
block_max_volume: Optional[float] = None,
block_max_volume: Optional[float],
) -> None:
"""Execute a Thermocycler Profile."""
self._repetitions = repetitions
self._step_count = len(steps)
"""Execute a thermocycler profile using thermocycler/runProfile and flattened steps."""
engine_steps = [
cmd.thermocycler.RunProfileStepParams(
celsius=step["temperature"],
Expand All @@ -352,6 +349,49 @@ def execute_profile(
)
)

def _execute_profile_post_221(
self,
steps: List[ThermocyclerStep],
repetitions: int,
block_max_volume: Optional[float],
) -> None:
"""Execute a thermocycler profile using thermocycler/runExtendedProfile."""
engine_steps: List[
Union[cmd.thermocycler.ProfileCycle, cmd.thermocycler.ProfileStep]
] = [
cmd.thermocycler.ProfileCycle(
repetitions=repetitions,
steps=[
cmd.thermocycler.ProfileStep(
celsius=step["temperature"],
holdSeconds=step["hold_time_seconds"],
)
for step in steps
],
)
]
self._engine_client.execute_command(
cmd.thermocycler.RunExtendedProfileParams(
moduleId=self.module_id,
profileElements=engine_steps,
blockMaxVolumeUl=block_max_volume,
)
)

def execute_profile(
self,
steps: List[ThermocyclerStep],
repetitions: int,
block_max_volume: Optional[float] = None,
) -> None:
"""Execute a Thermocycler Profile."""
self._repetitions = repetitions
self._step_count = len(steps)
if self.api_version >= APIVersion(2, 21):
return self._execute_profile_post_221(steps, repetitions, block_max_volume)
else:
return self._execute_profile_pre_221(steps, repetitions, block_max_volume)

def deactivate_lid(self) -> None:
"""Turn off the heated lid."""
self._engine_client.execute_command(
Expand Down
10 changes: 8 additions & 2 deletions api/src/opentrons/protocol_api/module_contexts.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@
from opentrons_shared_data.module.types import ModuleModel, ModuleType

from opentrons.legacy_broker import LegacyBroker
from opentrons.hardware_control.modules import ThermocyclerStep
from opentrons.legacy_commands import module_commands as cmds
from opentrons.legacy_commands.publisher import CommandPublisher, publish
from opentrons.protocols.api_support.types import APIVersion
from opentrons.protocols.api_support.types import APIVersion, ThermocyclerStep
from opentrons.protocols.api_support.util import (
APIVersionError,
requires_version,
Expand Down Expand Up @@ -629,6 +628,13 @@ def execute_profile(
``hold_time_minutes`` and ``hold_time_seconds`` must be defined
and for each step.
.. note:
Before API Version 2.21, Thermocycler profiles run with this command
would be listed in the app as having a number of repetitions equal to
their step count. At or above API Version 2.21, the structure of the
Thermocycler cycles is preserved.
"""
repetitions = validation.ensure_thermocycler_repetition_count(repetitions)
validated_steps = validation.ensure_thermocycler_profile_steps(steps)
Expand Down
3 changes: 1 addition & 2 deletions api/src/opentrons/protocol_api/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from opentrons_shared_data.pipette.types import PipetteNameType
from opentrons_shared_data.robot.types import RobotType

from opentrons.protocols.api_support.types import APIVersion
from opentrons.protocols.api_support.types import APIVersion, ThermocyclerStep
from opentrons.protocols.api_support.util import APIVersionError
from opentrons.protocols.models import LabwareDefinition
from opentrons.types import Mount, DeckSlotName, StagingSlotName, Location
Expand All @@ -30,7 +30,6 @@
HeaterShakerModuleModel,
MagneticBlockModel,
AbsorbanceReaderModel,
ThermocyclerStep,
)

from .disposal_locations import TrashBin, WasteChute
Expand Down
5 changes: 5 additions & 0 deletions api/src/opentrons/protocol_engine/commands/command_unions.py
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@
thermocycler.OpenLid,
thermocycler.CloseLid,
thermocycler.RunProfile,
thermocycler.RunExtendedProfile,
absorbance_reader.CloseLid,
absorbance_reader.OpenLid,
absorbance_reader.Initialize,
Expand Down Expand Up @@ -456,6 +457,7 @@
thermocycler.OpenLidParams,
thermocycler.CloseLidParams,
thermocycler.RunProfileParams,
thermocycler.RunExtendedProfileParams,
absorbance_reader.CloseLidParams,
absorbance_reader.OpenLidParams,
absorbance_reader.InitializeParams,
Expand Down Expand Up @@ -530,6 +532,7 @@
thermocycler.OpenLidCommandType,
thermocycler.CloseLidCommandType,
thermocycler.RunProfileCommandType,
thermocycler.RunExtendedProfileCommandType,
absorbance_reader.CloseLidCommandType,
absorbance_reader.OpenLidCommandType,
absorbance_reader.InitializeCommandType,
Expand Down Expand Up @@ -605,6 +608,7 @@
thermocycler.OpenLidCreate,
thermocycler.CloseLidCreate,
thermocycler.RunProfileCreate,
thermocycler.RunExtendedProfileCreate,
absorbance_reader.CloseLidCreate,
absorbance_reader.OpenLidCreate,
absorbance_reader.InitializeCreate,
Expand Down Expand Up @@ -681,6 +685,7 @@
thermocycler.OpenLidResult,
thermocycler.CloseLidResult,
thermocycler.RunProfileResult,
thermocycler.RunExtendedProfileResult,
absorbance_reader.CloseLidResult,
absorbance_reader.OpenLidResult,
absorbance_reader.InitializeResult,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,16 @@
RunProfileCreate,
)

from .run_extended_profile import (
RunExtendedProfileCommandType,
RunExtendedProfileParams,
RunExtendedProfileResult,
RunExtendedProfile,
RunExtendedProfileCreate,
ProfileCycle,
ProfileStep,
)


__all__ = [
# Set target block temperature command models
Expand Down Expand Up @@ -130,4 +140,13 @@
"RunProfileResult",
"RunProfile",
"RunProfileCreate",
# Run extended profile command models.
"RunExtendedProfileCommandType",
"RunExtendedProfileParams",
"RunExtendedProfileStepParams",
"RunExtendedProfileResult",
"RunExtendedProfile",
"RunExtendedProfileCreate",
"ProfileCycle",
"ProfileStep",
]
Loading

0 comments on commit 9d16384

Please sign in to comment.