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(api): add new InstrumentContext.transfer_liquid() method #16819

Open
wants to merge 10 commits into
base: edge
Choose a base branch
from
136 changes: 135 additions & 1 deletion api/src/opentrons/protocol_api/_liquid_properties.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
from dataclasses import dataclass
from numpy import interp
from typing import Optional, Dict, Sequence, Tuple
from typing import Optional, Dict, Sequence, Tuple, List

from opentrons_shared_data.liquid_classes.liquid_class_definition import (
AspirateProperties as SharedDataAspirateProperties,
SingleDispenseProperties as SharedDataSingleDispenseProperties,
MultiDispenseProperties as SharedDataMultiDispenseProperties,
DelayProperties as SharedDataDelayProperties,
DelayParams as SharedDataDelayParams,
TouchTipProperties as SharedDataTouchTipProperties,
LiquidClassTouchTipParams as SharedDataTouchTipParams,
MixProperties as SharedDataMixProperties,
MixParams as SharedDataMixParams,
BlowoutProperties as SharedDataBlowoutProperties,
BlowoutParams as SharedDataBlowoutParams,
ByTipTypeSetting as SharedByTipTypeSetting,
Submerge as SharedDataSubmerge,
RetractAspirate as SharedDataRetractAspirate,
Expand Down Expand Up @@ -37,6 +41,10 @@ def as_dict(self) -> Dict[float, float]:
"""Get a dictionary representation of all set volumes and values along with the default."""
return self._properties_by_volume

def as_list_of_tuples(self) -> List[Tuple[float, float]]:
"""Get as list of tuples."""
return [(k, v) for k, v in self._properties_by_volume.items()]
Copy link
Contributor

Choose a reason for hiding this comment

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

this could be simplified to list(self._properties_by_volume.items())

Copy link
Contributor

Choose a reason for hiding this comment

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

I think list(self._properties_by_volume.items()) will do what you want?


def get_for_volume(self, volume: float) -> float:
"""Get a value by volume for this property. Volumes not defined will be interpolated between set volumes."""
validated_volume = validation.ensure_positive_float(volume)
Expand Down Expand Up @@ -101,6 +109,14 @@ def duration(self, new_duration: float) -> None:
validated_duration = validation.ensure_positive_float(new_duration)
self._duration = validated_duration

def as_schema_v1_model(self) -> SharedDataDelayProperties:
Copy link
Contributor

Choose a reason for hiding this comment

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

Question on the versioning: Do you expect the v1 to stick around forever? Like, if we later create a v2 of the schema, are you going to have an as_schema_v2_model() function too? And how would that even work -- like, wouldn't you need

def as_schema_v1_model(self) -> SharedDataDelayPropertiesV1 ...
def as_schema_v2_model(self) -> SharedDataDelayPropertiesV2 ...

What I'm getting at is: if this code is NOT expected to handle v2, v3, etc., the same way, then having v1 in all the function names feels like it's just adding clutter that won't really help make the code extensible for future versions. But is that just a convention we follow?

return SharedDataDelayProperties(
enable=self._enabled,
params=SharedDataDelayParams(duration=self.duration)
if self.duration is not None
else None,
)


@dataclass
class TouchTipProperties:
Expand Down Expand Up @@ -152,6 +168,27 @@ def speed(self, new_speed: float) -> None:
validated_speed = validation.ensure_positive_float(new_speed)
self._speed = validated_speed

def _get_schema_v1_params(self) -> Optional[SharedDataTouchTipParams]:
"""Get the touch tip params in schema v1 shape."""
if (
self._z_offset is not None
and self._mm_to_edge is not None
and self._speed is not None
):
return SharedDataTouchTipParams(
zOffset=self._z_offset,
mmToEdge=self._mm_to_edge,
speed=self._speed,
)
else:
return None

def as_schema_v1_model(self) -> SharedDataTouchTipProperties:
return SharedDataTouchTipProperties(
enable=self._enabled,
params=self._get_schema_v1_params(),
)


@dataclass
class MixProperties:
Expand Down Expand Up @@ -189,6 +226,22 @@ def volume(self, new_volume: float) -> None:
validated_volume = validation.ensure_positive_float(new_volume)
self._volume = validated_volume

def _get_schema_v1_params(self) -> Optional[SharedDataMixParams]:
"""Get the mix params in schema v1 shape."""
if self._repetitions is not None and self._volume is not None:
return SharedDataMixParams(
repetitions=self._repetitions,
volume=self._volume,
)
else:
return None

def as_schema_v1_model(self) -> SharedDataMixProperties:
return SharedDataMixProperties(
enable=self._enabled,
params=self._get_schema_v1_params(),
)


@dataclass
class BlowoutProperties:
Expand Down Expand Up @@ -227,6 +280,22 @@ def flow_rate(self, new_flow_rate: float) -> None:
validated_flow_rate = validation.ensure_positive_float(new_flow_rate)
self._flow_rate = validated_flow_rate

def _get_schema_v1_params(self) -> Optional[SharedDataBlowoutParams]:
"""Get the mix params in schema v1 shape."""
if self._location is not None and self._flow_rate is not None:
return SharedDataBlowoutParams(
location=self._location,
flowRate=self._flow_rate,
)
else:
return None

def as_schema_v1_model(self) -> SharedDataBlowoutProperties:
return SharedDataBlowoutProperties(
enable=self._enabled,
params=self._get_schema_v1_params(),
)


@dataclass
class SubmergeRetractCommon:
Expand Down Expand Up @@ -271,6 +340,14 @@ def delay(self) -> DelayProperties:
class Submerge(SubmergeRetractCommon):
...

def as_schema_v1_model(self) -> SharedDataSubmerge:
return SharedDataSubmerge(
positionReference=self._position_reference,
offset=self._offset,
speed=self._speed,
delay=self._delay.as_schema_v1_model(),
)


@dataclass
class RetractAspirate(SubmergeRetractCommon):
Expand All @@ -286,6 +363,16 @@ def air_gap_by_volume(self) -> LiquidHandlingPropertyByVolume:
def touch_tip(self) -> TouchTipProperties:
return self._touch_tip

def as_schema_v1_model(self) -> SharedDataRetractAspirate:
return SharedDataRetractAspirate(
positionReference=self._position_reference,
offset=self._offset,
speed=self._speed,
airGapByVolume=self._air_gap_by_volume.as_list_of_tuples(),
touchTip=self._touch_tip.as_schema_v1_model(),
delay=self._delay.as_schema_v1_model(),
)


@dataclass
class RetractDispense(SubmergeRetractCommon):
Expand All @@ -306,6 +393,17 @@ def touch_tip(self) -> TouchTipProperties:
def blowout(self) -> BlowoutProperties:
return self._blowout

def as_schema_v1_model(self) -> SharedDataRetractDispense:
return SharedDataRetractDispense(
positionReference=self._position_reference,
offset=self._offset,
speed=self._speed,
airGapByVolume=self._air_gap_by_volume.as_list_of_tuples(),
blowout=self._blowout.as_schema_v1_model(),
touchTip=self._touch_tip.as_schema_v1_model(),
delay=self._delay.as_schema_v1_model(),
)


@dataclass
class BaseLiquidHandlingProperties:
Expand Down Expand Up @@ -370,6 +468,18 @@ def retract(self) -> RetractAspirate:
def mix(self) -> MixProperties:
return self._mix

def as_schema_v1_model(self) -> SharedDataAspirateProperties:
return SharedDataAspirateProperties(
submerge=self._submerge.as_schema_v1_model(),
retract=self._retract.as_schema_v1_model(),
positionReference=self._position_reference,
offset=self._offset,
flowRateByVolume=self._flow_rate_by_volume.as_list_of_tuples(),
preWet=self._pre_wet,
mix=self._mix.as_schema_v1_model(),
delay=self._delay.as_schema_v1_model(),
)


@dataclass
class SingleDispenseProperties(BaseLiquidHandlingProperties):
Expand All @@ -390,6 +500,18 @@ def retract(self) -> RetractDispense:
def mix(self) -> MixProperties:
return self._mix

def as_schema_v1_model(self) -> SharedDataSingleDispenseProperties:
return SharedDataSingleDispenseProperties(
submerge=self._submerge.as_schema_v1_model(),
retract=self._retract.as_schema_v1_model(),
positionReference=self._position_reference,
offset=self._offset,
flowRateByVolume=self._flow_rate_by_volume.as_list_of_tuples(),
mix=self._mix.as_schema_v1_model(),
pushOutByVolume=self._push_out_by_volume.as_list_of_tuples(),
delay=self._delay.as_schema_v1_model(),
)


@dataclass
class MultiDispenseProperties(BaseLiquidHandlingProperties):
Expand All @@ -410,6 +532,18 @@ def conditioning_by_volume(self) -> LiquidHandlingPropertyByVolume:
def disposal_by_volume(self) -> LiquidHandlingPropertyByVolume:
return self._disposal_by_volume

def as_schema_v1_model(self) -> SharedDataMultiDispenseProperties:
return SharedDataMultiDispenseProperties(
submerge=self._submerge.as_schema_v1_model(),
retract=self._retract.as_schema_v1_model(),
positionReference=self._position_reference,
offset=self._offset,
flowRateByVolume=self._flow_rate_by_volume.as_list_of_tuples(),
conditioningByVolume=self._conditioning_by_volume.as_list_of_tuples(),
disposalByVolume=self._disposal_by_volume.as_list_of_tuples(),
delay=self._delay.as_schema_v1_model(),
)


@dataclass
class TransferProperties:
Expand Down
51 changes: 45 additions & 6 deletions api/src/opentrons/protocol_api/core/engine/instrument.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@

from __future__ import annotations

from typing import Optional, TYPE_CHECKING, cast, Union
from opentrons.protocols.api_support.types import APIVersion

from typing import Optional, TYPE_CHECKING, cast, Union, List
from opentrons.types import Location, Mount, NozzleConfigurationType, NozzleMapInterface
from opentrons.hardware_control import SyncHardwareAPI
from opentrons.hardware_control.dev_types import PipetteDict
from opentrons.protocols.api_support.util import FlowRates, find_value_for_api_version
from opentrons.protocols.api_support.types import APIVersion
from opentrons.protocols.advanced_control.transfers.common import TransferTipPolicyV2
from opentrons.protocol_engine import commands as cmd
from opentrons.protocol_engine import (
DeckPoint,
Expand All @@ -27,6 +27,7 @@
PRIMARY_NOZZLE_LITERAL,
NozzleLayoutConfigurationType,
AddressableOffsetVector,
LiquidClassRecord,
)
from opentrons.protocol_engine.errors.exceptions import TipNotAttachedError
from opentrons.protocol_engine.clients import SyncClient as EngineClient
Expand All @@ -38,14 +39,13 @@
from opentrons.protocol_api._nozzle_layout import NozzleLayout
from . import overlap_versions, pipette_movement_conflict

from ..instrument import AbstractInstrument
from .well import WellCore

from ..instrument import AbstractInstrument
from ...disposal_locations import TrashBin, WasteChute

if TYPE_CHECKING:
from .protocol import ProtocolCore

from opentrons.protocol_api._liquid import LiquidClass

_DISPENSE_VOLUME_VALIDATION_ADDED_IN = APIVersion(2, 17)

Expand Down Expand Up @@ -864,6 +864,45 @@ def configure_nozzle_layout(
)
)

def load_liquid_class(
self,
liquid_class: LiquidClass,
pipette_load_name: str,
tiprack_uri: str,
) -> str:
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey, could you explain what Instrument is supposed to represent conceptually? Like, I think it makes sense that an instrument can aspirate or transferLiquid, but is loading liquid classes also something that belongs to the instrument?

"""Load a liquid class into the engine and return its ID."""
transfer_props = liquid_class.get_for(
pipette=pipette_load_name, tiprack=tiprack_uri
)

liquid_class_record = LiquidClassRecord(
liquidClassName=liquid_class.name,
pipetteModel=self.get_model(), # TODO: verify this is the correct 'model' to use
Copy link
Contributor

Choose a reason for hiding this comment

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

The implementation just treats pipetteModel as an opaque string, so it can be whatever you want :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ya, but these need to be consistent. We have two different ways to refer to pipettes- the API load name and the name used by rest of the system. The definition contains properties keyed by API load name and so when looking up values from the definition we have to use the API load name. But most of the get_name or get_model functions use the other name.

tiprack=tiprack_uri,
aspirate=transfer_props.aspirate.as_schema_v1_model(),
singleDispense=transfer_props.dispense.as_schema_v1_model(),
multiDispense=transfer_props.multi_dispense.as_schema_v1_model()
if transfer_props.multi_dispense
else None,
)
result = self._engine_client.execute_command_without_recovery(
cmd.LoadLiquidClassParams(
liquidClassRecord=liquid_class_record,
)
)
return result.liquidClassId

def transfer_liquid(
self,
liquid_class_id: str,
volume: float,
source: List[WellCore],
dest: List[WellCore],
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you say yesterday that source and dest are expected to be exactly the same length?

If so, maybe it's less error-prone if we made the user give us a single list of (source, dest) well pairs, rather than 2 lists?

Copy link
Member Author

Choose a reason for hiding this comment

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

It will just be more work for protocol authors to create a paired list. Most of the time users use things like source=labware1.columns()[0] and dest=labware2.columns()[0].

new_tip: TransferTipPolicyV2,
trash_location: Union[WellCore, Location, TrashBin, WasteChute],
) -> None:
"""Execute transfer using liquid class properties."""

def retract(self) -> None:
"""Retract this instrument to the top of the gantry."""
z_axis = self._engine_client.state.pipettes.get_z_axis(self._pipette_id)
Expand Down
31 changes: 29 additions & 2 deletions api/src/opentrons/protocol_api/core/instrument.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
from __future__ import annotations

from abc import abstractmethod, ABC
from typing import Any, Generic, Optional, TypeVar, Union
from typing import Any, Generic, Optional, TypeVar, Union, List

from opentrons import types
from opentrons.hardware_control.dev_types import PipetteDict
from opentrons.protocols.api_support.util import FlowRates
from opentrons.protocols.advanced_control.transfers.common import TransferTipPolicyV2
from opentrons.protocol_api._nozzle_layout import NozzleLayout

from opentrons.protocol_api._liquid import LiquidClass
from ..disposal_locations import TrashBin, WasteChute
from .well import WellCoreType

Expand Down Expand Up @@ -309,6 +310,32 @@ def configure_nozzle_layout(
"""
...

@abstractmethod
def load_liquid_class(
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, and what's the difference between protocol_api/core/engine/instrument.py and protocol_api/core/instrument.py?

Copy link
Member Author

Choose a reason for hiding this comment

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

protocol_api/core/instrument.py has the outside-facing interfaces that the public context refers to. So just a bunch of abstractmethos declarations. The actual implementation of these methods is carried out by the three 'cores'- engine core/ legacy core/ legacy simulator core. The public context gets the relevant 'core' to use during initialization. All our new features go on the engine core.

self,
liquid_class: LiquidClass,
pipette_load_name: str,
tiprack_uri: str,
) -> str:
"""Load the liquid class properties of given pipette and tiprack into the engine.
Returns: ID of the liquid class record
"""
...

@abstractmethod
def transfer_liquid(
self,
liquid_class_id: str,
volume: float,
source: List[WellCoreType],
dest: List[WellCoreType],
new_tip: TransferTipPolicyV2,
trash_location: Union[WellCoreType, types.Location, TrashBin, WasteChute],
) -> None:
"""Transfer a liquid from source to dest according to liquid class properties."""
...

@abstractmethod
def is_tip_tracking_available(self) -> bool:
"""Return whether auto tip tracking is available for the pipette's current nozzle configuration."""
Expand Down
Loading
Loading