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

feat(engine): thermocycler run profile #10921

Merged
merged 7 commits into from
Jun 30, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@
thermocycler.DeactivateLid,
thermocycler.OpenLid,
thermocycler.CloseLid,
thermocycler.RunProfile,
]

CommandParams = Union[
Expand Down Expand Up @@ -230,6 +231,8 @@
thermocycler.DeactivateLidParams,
thermocycler.OpenLidParams,
thermocycler.CloseLidParams,
thermocycler.RunProfileParams,
thermocycler.RunProfileStepParams,
]

CommandType = Union[
Expand Down Expand Up @@ -271,6 +274,7 @@
thermocycler.DeactivateLidCommandType,
thermocycler.OpenLidCommandType,
thermocycler.CloseLidCommandType,
thermocycler.RunProfileCommandType,
]

CommandCreate = Union[
Expand Down Expand Up @@ -311,6 +315,7 @@
thermocycler.DeactivateLidCreate,
thermocycler.OpenLidCreate,
thermocycler.CloseLidCreate,
thermocycler.RunProfileCreate,
]

CommandResult = Union[
Expand Down Expand Up @@ -352,4 +357,5 @@
thermocycler.DeactivateLidResult,
thermocycler.OpenLidResult,
thermocycler.CloseLidResult,
thermocycler.RunProfileResult,
]
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,15 @@
CloseLidCreate,
)

from .run_profile import (
RunProfileCommandType,
RunProfileParams,
RunProfileStepParams,
RunProfileResult,
RunProfile,
RunProfileCreate,
)


__all__ = [
# Set target block temperature command models
Expand Down Expand Up @@ -114,4 +123,11 @@
"CloseLidResult",
"CloseLid",
"CloseLidCreate",
# Run profile command models,
"RunProfileCommandType",
"RunProfileParams",
"RunProfileStepParams",
"RunProfileResult",
"RunProfile",
"RunProfileCreate",
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
"""Command models to execute a Thermocycler profile."""
from __future__ import annotations
from typing import List, Optional, TYPE_CHECKING
from typing_extensions import Literal, Type

from pydantic import BaseModel, Field

from ..command import AbstractCommandImpl, BaseCommand, BaseCommandCreate

if TYPE_CHECKING:
from opentrons.protocol_engine.state import StateView
from opentrons.protocol_engine.execution import EquipmentHandler


RunProfileCommandType = Literal["thermocycler/runProfile"]


class RunProfileStepParams(BaseModel):
"""Input parameters for an individual Thermocycler profile step."""

celsius: float = Field(..., description="Target temperature in °C.")
holdSeconds: int = Field(
..., description="Time to hold target temperature at in seconds."
)


class RunProfileParams(BaseModel):
"""Input parameters to run a Thermocycler profile."""

moduleId: str = Field(..., description="Unique ID of the Thermocycler.")
profile: List[RunProfileStepParams] = Field(
...,
description="Array of profile steps with target temperature and temperature hold time.",
)
blockMaxVolumeUl: float = Field(
Copy link
Contributor

Choose a reason for hiding this comment

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

@shlokamin or @sanni-t, do either of y'all know why this is a required parameter of runProfile but optional for setTargetBlockTemperature? My gut reaction is that it should be optional in runProfile, too.

It's optional once you hit the hardware control API, FWIW

Copy link
Member

Choose a reason for hiding this comment

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

AFAIK it should be optional, not sure why it's required in the JSON schema. im fine with updating the schema to make it optional as part of this PR (i can do that as part of the frontend side). that cool with you @sanni-t ?

Copy link
Member

@shlokamin shlokamin Jun 28, 2022

Choose a reason for hiding this comment

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

also note to self, i need to add to the JSON schema migration to make sure <v6 protocols get these parameters updated

Copy link
Member

Choose a reason for hiding this comment

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

Yep, volume should be optional. It defaults to 25uL in firmware

Copy link
Member

Choose a reason for hiding this comment

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

Unless we want to make sure that PD protocols always specify the volume for ideal heating. I'm guessing that since PD does volume tracking, the volume will always be specified anyway?

The volume parameter was added to setTargetBlockTemperature only in v6. Hence that one definitely needs to stay optional to allow older protocols to run as expected.

Copy link
Member

Choose a reason for hiding this comment

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

good point, the thermocycler step form in PD does require volume, but thats a PD concern (not a step generation/schema concern), so ill go ahead and mark it as optional

...,
description="Amount of liquid in uL of the most-full well in labware loaded onto the thermocycler.",
)


class RunProfileResult(BaseModel):
"""Result data from running a Thermocycler profile."""


class RunProfileImpl(AbstractCommandImpl[RunProfileParams, RunProfileResult]):
"""Execution implementation of a Thermocycler's run profile command."""

def __init__(
self,
state_view: StateView,
equipment: EquipmentHandler,
**unused_dependencies: object,
) -> None:
self._state_view = state_view
self._equipment = equipment

async def execute(self, params: RunProfileParams) -> RunProfileResult:
"""Run a Thermocycler profile."""
thermocycler_state = self._state_view.modules.get_thermocycler_module_substate(
params.moduleId
)
thermocycler_hardware = self._equipment.get_module_hardware_api(
thermocycler_state.module_id
)

steps = []
for profile_step in params.profile:
target_temperature = thermocycler_state.validate_target_block_temperature(
profile_step.celsius
)
steps.append(
{
"temperature": target_temperature,
"hold_time_seconds": profile_step.holdSeconds,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I would love for the hardware control API to accept a dataclass or NamedTuple rather than a dict. Might add it to the confluence doc

)
jbleon95 marked this conversation as resolved.
Show resolved Hide resolved

target_volume = thermocycler_state.validate_max_block_volume(
params.blockMaxVolumeUl
)

if thermocycler_hardware is not None:
# TODO(jbl 2022-06-27) hardcoded constant 1 for `repetitions` should be
# moved from HardwareControlAPI to the Python ProtocolContext
await thermocycler_hardware.cycle_temperatures(
steps=steps, repetitions=1, volume=target_volume
)

return RunProfileResult()


class RunProfile(BaseCommand[RunProfileParams, RunProfileResult]):
"""A command to execute a Thermocycler profile run."""

commandType: RunProfileCommandType = "thermocycler/runProfile"
params: RunProfileParams
result: Optional[RunProfileResult]

_ImplementationCls: Type[RunProfileImpl] = RunProfileImpl


class RunProfileCreate(BaseCommandCreate[RunProfileParams]):
"""A request to execute a Thermocycler profile run."""

commandType: RunProfileCommandType = "thermocycler/runProfile"
params: RunProfileParams

_CommandCls: Type[RunProfile] = RunProfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
"""Test Thermocycler run profile command implementation."""
from decoy import Decoy

from opentrons.hardware_control.modules import Thermocycler

from opentrons.protocol_engine.state import StateView
from opentrons.protocol_engine.state.module_substates import (
ThermocyclerModuleSubState,
ThermocyclerModuleId,
)
from opentrons.protocol_engine.execution import EquipmentHandler
from opentrons.protocol_engine.commands import thermocycler as tc_commands
from opentrons.protocol_engine.commands.thermocycler.run_profile import (
RunProfileImpl,
)


async def test_run_profile(
decoy: Decoy,
state_view: StateView,
equipment: EquipmentHandler,
) -> None:
"""It should be able to execute the specified module's profile run."""
subject = RunProfileImpl(state_view=state_view, equipment=equipment)

step_data = [
tc_commands.RunProfileStepParams(celsius=12.3, holdSeconds=45),
tc_commands.RunProfileStepParams(celsius=45.6, holdSeconds=78),
]
data = tc_commands.RunProfileParams(
moduleId="input-thermocycler-id",
profile=step_data,
blockMaxVolumeUl=56.7,
)
expected_result = tc_commands.RunProfileResult()

tc_module_substate = decoy.mock(cls=ThermocyclerModuleSubState)
tc_hardware = decoy.mock(cls=Thermocycler)

decoy.when(
state_view.modules.get_thermocycler_module_substate("input-thermocycler-id")
).then_return(tc_module_substate)

decoy.when(tc_module_substate.module_id).then_return(
ThermocyclerModuleId("thermocycler-id")
)

# Stub temperature validation from hs module view
decoy.when(tc_module_substate.validate_target_block_temperature(12.3)).then_return(
32.1
)
decoy.when(tc_module_substate.validate_target_block_temperature(45.6)).then_return(
65.4
)

# Stub volume validation from hs module view
decoy.when(tc_module_substate.validate_max_block_volume(56.7)).then_return(76.5)

# Get attached hardware modules
decoy.when(
equipment.get_module_hardware_api(ThermocyclerModuleId("thermocycler-id"))
).then_return(tc_hardware)

result = await subject.execute(data)

decoy.verify(
await tc_hardware.cycle_temperatures(
steps=[
{"temperature": 32.1, "hold_time_seconds": 45},
{"temperature": 65.4, "hold_time_seconds": 78},
],
repetitions=1,
volume=76.5,
),
times=1,
)
assert result == expected_result
12 changes: 6 additions & 6 deletions shared-data/protocol/schemas/6.json
Original file line number Diff line number Diff line change
Expand Up @@ -843,7 +843,7 @@
"commandType": { "enum": ["thermocycler/runProfile"] },
"params": {
"type": "object",
"required": ["moduleId", "profile", "volume"],
Copy link
Member

Choose a reason for hiding this comment

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

removing volume (renamed to blockMaxVolumeUl) as a required param

"required": ["moduleId", "profile"],
"properties": {
"moduleId": {
"type": "string",
Expand All @@ -853,20 +853,20 @@
"type": "array",
"items": {
"type": "object",
"required": ["temperature", "holdTime"],
"required": ["celsius", "holdSeconds"],
"properties": {
"temperature": {
"description": "Target temperature of profile step",
"celsius": {
"description": "Target temperature (in celsius) of profile step",
"type": "number"
},
"holdTime": {
"holdSeconds": {
"description": "Time (in seconds) to hold once temperature is reached",
"type": "number"
}
}
}
},
"volume": {
"blockMaxVolumeUl": {
"type": "number"
}
}
Expand Down
6 changes: 3 additions & 3 deletions shared-data/protocol/types/schemaV6/command/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,14 +279,14 @@ export interface ShakeSpeedParams {
}

export interface AtomicProfileStep {
holdTime: number
temperature: number
holdSeconds: number
celsius: number
}

export interface TCProfileParams {
moduleId: string
profile: AtomicProfileStep[]
volume: number
blockMaxVolumeUl?: number
}

export interface ModuleOnlyParams {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,14 +125,25 @@ describe('thermocycler atomic commands', () => {
const robotInitialState = getRobotInitialState()
const result = commandCreator(params, invariantContext, robotInitialState)
const res = getSuccessResult(result)
// delete this once params are changed to conform to v6 params
const v6Params = {
...params,
moduleId: params.module,
celsius: params.temperature,
}
delete v6Params.module
delete v6Params.temperature
if (v6Params.profile != null) {
v6Params.profile = v6Params.profile.map(
(profileItem: { temperature: number; holdTime: number }) => ({
celsius: profileItem.temperature,
holdSeconds: profileItem.holdTime,
})
)
}
if (v6Params.volume != null) {
v6Params.blockMaxVolumeUl = v6Params.volume
delete v6Params.volume
}
expect(res.commands).toEqual([
{
commandType: expectedType,
Expand Down
14 changes: 7 additions & 7 deletions step-generation/src/__tests__/thermocyclerProfileStep.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ describe('thermocyclerProfileStep', () => {
params: {
moduleId: 'thermocyclerId',
profile: [],
volume: 42,
blockMaxVolumeUl: 42,
},
},
{
Expand Down Expand Up @@ -118,8 +118,8 @@ describe('thermocyclerProfileStep', () => {
commandType: 'thermocycler/runProfile',
params: {
moduleId: 'thermocyclerId',
profile: [{ temperature: 61, holdTime: 99 }],
volume: 42,
profile: [{ celsius: 61, holdSeconds: 99 }],
blockMaxVolumeUl: 42,
},
},
{
Expand Down Expand Up @@ -185,8 +185,8 @@ describe('thermocyclerProfileStep', () => {
commandType: 'thermocycler/runProfile',
params: {
moduleId: 'thermocyclerId',
profile: [{ temperature: 61, holdTime: 99 }],
volume: 42,
profile: [{ celsius: 61, holdSeconds: 99 }],
blockMaxVolumeUl: 42,
},
},
{
Expand Down Expand Up @@ -246,8 +246,8 @@ describe('thermocyclerProfileStep', () => {
commandType: 'thermocycler/runProfile',
params: {
moduleId: 'thermocyclerId',
profile: [{ temperature: 61, holdTime: 99 }],
volume: 42,
profile: [{ celsius: 61, holdSeconds: 99 }],
blockMaxVolumeUl: 42,
},
},
{
Expand Down
3 changes: 1 addition & 2 deletions step-generation/src/__tests__/thermocyclerUpdates.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import { makeImmutableStateUpdater } from '../__utils__'
import { makeContext, getInitialRobotStateStandard } from '../fixtures'
import type {
ModuleOnlyParams,
TCProfileParams,
TemperatureParams,
ThermocyclerSetTargetBlockTemperatureParams,
} from '@opentrons/shared-data/protocol/types/schemaV6/command/module'
Expand Down Expand Up @@ -206,7 +205,7 @@ describe('thermocycler state updaters', () => {
testName: 'forThermocyclerOpenLid should set lidOpen to true',
},
]
const profileCases: TestCases<TCProfileParams> = [
const profileCases: TestCases<any> = [
{
params: {
moduleId,
Expand Down
Loading