Skip to content

fix(api): optimize liquid class transfers by only changing volume mode when necessary #20009

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

Merged
merged 9 commits into from
Nov 10, 2025
Merged

fix(api): optimize liquid class transfers by only changing volume mode when necessary #20009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b1313f2
store liquid properties in pipette dict and volume mods and their min…
jbleon95 Oct 29, 2025
15b637e
added volume mode min vol to static pipette config and new methods
jbleon95 Oct 29, 2025
28d223e
added check to lc aspirate step
jbleon95 Oct 29, 2025
cd7314c
prepare to aspirate if we need to
jbleon95 Nov 3, 2025
7dd081a
lint fix
jbleon95 Nov 3, 2025
19da9f8
Merge branch 'edge' of https://github.com/Opentrons/opentrons into lc…
jbleon95 Nov 3, 2025
abd426a
added version bump, version check and unit tests
jbleon95 Nov 6, 2025
08c3aea
Merge branch 'edge' of https://github.com/Opentrons/opentrons into lc…
jbleon95 Nov 6, 2025
2b64c81
update gravimetric script version
jbleon95 Nov 6, 2025
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
4 changes: 4 additions & 0 deletions api/src/opentrons/hardware_control/dev_types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
SupportedTipsDefinition,
PipetteBoundingBoxOffsetDefinition,
AvailableSensorDefinition,
PipetteLiquidPropertiesDefinition,
)
from opentrons_shared_data.gripper import (
GripperModel,
Expand Down Expand Up @@ -106,6 +107,9 @@ class PipetteDict(InstrumentDict):
shaft_ul_per_mm: float
available_sensors: AvailableSensorDefinition
volume_mode: LiquidClasses # LiquidClasses refer to volume mode in this context
available_volume_modes: Dict[
LiquidClasses, PipetteLiquidPropertiesDefinition
] # Ditto


class PipetteStateDict(TypedDict):
Expand Down
1 change: 1 addition & 0 deletions api/src/opentrons/hardware_control/instruments/ot2/pipette_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ def get_attached_instrument(self, mount: MountType) -> PipetteDict:
}
result["shaft_ul_per_mm"] = instr.config.shaft_ul_per_mm
result["volume_mode"] = instr.liquid_class_name
result["available_volume_modes"] = instr.config.liquid_properties
return cast(PipetteDict, result)

@property
Expand Down
1 change: 1 addition & 0 deletions api/src/opentrons/hardware_control/instruments/ot3/pipette_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ def get_attached_instrument(self, mount: OT3Mount) -> PipetteDict:
result["shaft_ul_per_mm"] = instr.config.shaft_ul_per_mm
result["available_sensors"] = instr.config.available_sensors
result["volume_mode"] = instr.liquid_class_name
result["available_volume_modes"] = instr.config.liquid_properties
return cast(PipetteDict, result)

@property
Expand Down
13 changes: 11 additions & 2 deletions api/src/opentrons/protocol_api/core/engine/instrument.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2153,7 +2153,12 @@ def aspirate_liquid_class(
air_gap=0,
)
)
if volume_for_pipette_mode_configuration is not None:
if volume_for_pipette_mode_configuration is not None and (
self._protocol_core.api_version < APIVersion(2, 28)
or self._engine_client.state.pipettes.get_will_volume_mode_change(
self._pipette_id, volume_for_pipette_mode_configuration
)
):
prep_location = Location(
point=source_well.get_top(LIQUID_PROBE_START_OFFSET_FROM_WELL_TOP.z),
labware=source_loc.labware,
Expand All @@ -2171,10 +2176,14 @@ def aspirate_liquid_class(
location=prep_location,
)
last_liquid_and_airgap_in_tip.air_gap = 0
# TODO: do volume configuration + prepare for aspirate only if the mode needs to be changed
self.configure_for_volume(volume_for_pipette_mode_configuration)
self.prepare_to_aspirate()

elif not self._engine_client.state.pipettes.get_ready_to_aspirate(
Copy link
Collaborator

@ddcc4 ddcc4 Nov 6, 2025

Choose a reason for hiding this comment

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

Hm, my philosophical objection to adding more queries to the engine is that it makes life harder for future-us when we want to be able to say what steps a transfer will do without actually performing the transfer.

Does @ncdiehl still want PD step generation to match Python command-for-command? This would make that a bit harder since PD doesn't have an engine to query.

Copy link
Contributor Author

@jbleon95 jbleon95 Nov 6, 2025

Choose a reason for hiding this comment

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

I don't think this necessarily precludes us from doing that in the future, because honestly all we need to know is the pipette's initial state, since we'll prepare to aspirate within the transfer component executor after this. But perhaps I should clean this up a bit to make that more clear or make it more functional. But I think on its own this is a correct code, especially if an overpressure error happens during the course of a protocol which we can't predict.

As for PD, I don't think we have the equivalent to this, but this is checking a variable that is updated in response to pick_up_tip, any form of blow_out, a full dispense and that I imagine we could track

self._pipette_id
):
self.prepare_to_aspirate()
Copy link
Collaborator

@ddcc4 ddcc4 Nov 6, 2025

Choose a reason for hiding this comment

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

Sorry, still trying to wrap my head around what this is doing.

BEFORE,
if volume_for_pipette_mode_configuration is None, we would NOT have prepare_to_aspirate() here regardless of whether the pipette is ready to aspirate, right?

Can that (volume_for_pipette_mode_configuration=None and not being ready to aspirate here) ever happen?

Copy link
Contributor Author

@jbleon95 jbleon95 Nov 6, 2025

Choose a reason for hiding this comment

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

You're right in that this could cause a difference in behavior in existing protocols and I can make sure this doesn't trigger in those instances.

The fact that we were always unconditionally going down this configure_for_volume path (which also includes the movement and prepare for aspirate) was hiding the fact that there were scenarios where we don't need to configure the volume but do need to prepare to aspirate, which would be if the pipette is starting the transfer not prepared to aspirate, i.e. it's last action was a blow-out or full dispense with push out

Copy link
Collaborator

@ddcc4 ddcc4 Nov 6, 2025

Choose a reason for hiding this comment

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

Oh, my question was more: was there ever a case when volume_for_pipette_mode_configuration was unset and we were not ready to aspirate?

Copy link
Contributor Author

@jbleon95 jbleon95 Nov 6, 2025

Choose a reason for hiding this comment

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

I don't think so, because the only time this would be None is in non-first aspirates in consolidate, where we would have already called prepare_to_aspirate beforehand

Copy link
Collaborator

@ddcc4 ddcc4 Nov 7, 2025

Choose a reason for hiding this comment

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

Got it, thanks!


aspirate_point = (
tx_comps_executor.absolute_point_from_position_reference_and_offset(
well=source_well,
Expand Down
9 changes: 9 additions & 0 deletions api/src/opentrons/protocol_engine/resources/pipette_data_provider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ class LoadedStaticPipetteData:
shaft_ul_per_mm: float
available_sensors: pipette_definition.AvailableSensorDefinition
volume_mode: pip_types.LiquidClasses # pip_types Liquid Classes refers to volume modes
available_volume_modes_min_vol: Dict[pip_types.LiquidClasses, float] # Ditto


class VirtualPipetteDataProvider:
Expand Down Expand Up @@ -300,6 +301,10 @@ def _get_virtual_pipette_static_config_by_model( # noqa: C901
available_sensors=config.available_sensors
or pipette_definition.AvailableSensorDefinition(sensors=[]),
volume_mode=liquid_class,
available_volume_modes_min_vol={
volume_mode: props.min_volume
Copy link
Collaborator

@ddcc4 ddcc4 Nov 6, 2025

Choose a reason for hiding this comment

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

Hey, is PipetteLiquidPropertiesDefinition.min_volume a lie? It's defined as:

min_volume: float = Field(
    ...,
    description="The minimum supported volume of the pipette.",
    alias="minVolume",
)

Do you mean the minimum supported volume in this particular mode? If it's the minimum volume for the pipette in general, what's the point of introducing your available_volume_modes with different entries for default and lowVolumeDefault?

Copy link
Contributor Author

@jbleon95 jbleon95 Nov 6, 2025

Choose a reason for hiding this comment

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

This is in practice the minimum supported volume of each mode, and we determine if we switch to low volume mode if both we have a low volume mode AND we are providing a volume below the default minimum.

Copy link
Contributor

@ryanthecoder ryanthecoder Nov 6, 2025

Choose a reason for hiding this comment

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

the min volume should reference the min volume in each of default or lowVolumeDefault

Copy link
Collaborator

@ddcc4 ddcc4 Nov 7, 2025

Choose a reason for hiding this comment

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

@ryanthecoder or @jbleon95: For the lowVolumeDefault mode, is PipetteLiquidPropertiesDefinition.min_volume expected to just be 0?

I.e., for a low-volume-capable pipette, we would have something like:

  • lowVolumeDefault: min_volume=0, max_volume=x
  • default: min_volume=x, max_volume=1000

?

Copy link
Member

@sfoster1 sfoster1 Nov 7, 2025

Choose a reason for hiding this comment

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

No, a min_volume is non-zero. in the low volume modes it's usually something like 0.5

for volume_mode, props in config.liquid_properties.items()
},
)

def get_virtual_pipette_static_config(
Expand Down Expand Up @@ -356,6 +361,10 @@ def get_pipette_static_config(
shaft_ul_per_mm=pipette_dict["shaft_ul_per_mm"],
available_sensors=available_sensors,
volume_mode=pipette_dict["volume_mode"],
available_volume_modes_min_vol={
volume_mode: props.min_volume
for volume_mode, props in pipette_dict["available_volume_modes"].items()
},
)


Expand Down
26 changes: 26 additions & 0 deletions api/src/opentrons/protocol_engine/state/pipettes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ class StaticPipetteConfig:
shaft_ul_per_mm: float
available_sensors: pipette_definition.AvailableSensorDefinition
volume_mode: VolumeModes
available_volume_modes_min_vol: Dict[VolumeModes, float]


@dataclasses.dataclass
Expand Down Expand Up @@ -318,6 +319,7 @@ def _update_pipette_config(self, state_update: update_types.StateUpdate) -> None
shaft_ul_per_mm=config.shaft_ul_per_mm,
available_sensors=config.available_sensors,
volume_mode=config.volume_mode,
available_volume_modes_min_vol=config.available_volume_modes_min_vol,
)
self._state.flow_rates_by_id[
state_update.pipette_config.pipette_id
Expand Down Expand Up @@ -876,6 +878,30 @@ def get_is_low_volume_mode(self, pipette_id: str) -> bool:
"""Determine if the pipette is currently in low volume mode."""
return self.get_config(pipette_id).volume_mode == VolumeModes.lowVolumeDefault

def get_volume_mode_from_volume(
self, pipette_id: str, volume: float
) -> VolumeModes:
"""Get the volume mode for the given pipette and volume quantity."""
available_volume_modes_min_vol = self.get_config(
pipette_id
).available_volume_modes_min_vol
has_low_volume_mode = (
VolumeModes.lowVolumeDefault in available_volume_modes_min_vol
)

if not has_low_volume_mode:
return VolumeModes.default
if volume >= available_volume_modes_min_vol[VolumeModes.default]:
return VolumeModes.default
Copy link
Collaborator

@ddcc4 ddcc4 Nov 6, 2025

Choose a reason for hiding this comment

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

Hm, this PR seems to do a lot of plumbing (adding PipetteDict.available_volume_modes, and then the available_volume_modes_min_vol dict) just to see if volume >= available_volume_modes_min_vol[VolumeModes.default] here.

I thought you were going to use available_volume_modes_min_vol[VolumeModes.lowVolumeDefault] somehow.

If you just need the "high-volume mode transition point," would it be easier to just add that one number to the PipetteDict?

Copy link
Contributor Author

@jbleon95 jbleon95 Nov 6, 2025

Choose a reason for hiding this comment

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

Yeah I originally was passing the whole dictionary to the engine before I realized it added a lot of cruft to unit tests and decided it was easier to just pass what we need. I can go back and refactor PipetteDict to be more straightforward if we can't think of another reason to keep the full object for any future work

return VolumeModes.lowVolumeDefault

def get_will_volume_mode_change(self, pipette_id: str, volume: float) -> bool:
"""Determine if the pipette will change volume mode based on current volume mode and new volume."""
return (
self.get_volume_mode_from_volume(pipette_id, volume)
!= self.get_config(pipette_id).volume_mode
)

def lookup_volume_to_mm_conversion(
self, pipette_id: str, volume: float, action: str
) -> float:
Expand Down
2 changes: 1 addition & 1 deletion api/src/opentrons/protocols/api_support/definitions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from .types import APIVersion

MAX_SUPPORTED_VERSION = APIVersion(2, 27)
MAX_SUPPORTED_VERSION = APIVersion(2, 28)
"""The maximum supported protocol API version in this release."""

MIN_SUPPORTED_VERSION = APIVersion(2, 0)
Expand Down
Loading
Loading