Skip to content

Commit

Permalink
fix(api): Use the latest information when estimating liquid height (#…
Browse files Browse the repository at this point in the history
…17374)

<!--
Thanks for taking the time to open a Pull Request (PR)! Please make sure
you've read the "Opening Pull Requests" section of our Contributing
Guide:


https://github.com/Opentrons/opentrons/blob/edge/CONTRIBUTING.md#opening-pull-requests

GitHub provides robust markdown to format your PR. Links, diagrams,
pictures, and videos along with text formatting make it possible to
create a rich and informative PR. For more information on GitHub
markdown, see:


https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax

To ensure your code is reviewed quickly and thoroughly, please fill out
the sections below to the best of your ability!
-->

# Overview
There was a bug in the estimation code under the following scenario
Well volume is set with Load Liquid
Well is then probed
Well is operated on and the probed volume and loaded volume are updated
based on the operation
Future well operations are based on the load liquid and not the probe
because it happened to be first in the "is_valid" check


Now the well volume after an operation is based on which happened LAST
load_liquid or liquid probe.
<!--
Describe your PR at a high level. State acceptance criteria and how this
PR fits into other work. Link issues, PRs, and other relevant resources.
-->

## Test Plan and Hands on Testing

<!--
Describe your testing of the PR. Emphasize testing not reflected in the
code. Attach protocols, logs, screenshots and any other assets that
support your testing.
-->

## Changelog

<!--
List changes introduced by this PR considering future developers and the
end user. Give careful thought and clear documentation to breaking
changes.
-->

## Review requests

<!--
- What do you need from reviewers to feel confident this PR is ready to
merge?
- Ask questions.
-->

## Risk assessment

<!--
- Indicate the level of attention this PR needs.
- Provide context to guide reviewers.
- Discuss trade-offs, coupling, and side effects.
- Look for the possibility, even if you think it's small, that your
change may affect some other part of the system.
- For instance, changing return tip behavior may also change the
behavior of labware calibration.
- How do your unit tests and on hands on testing mitigate this PR's
risks and the risk of future regressions?
- Especially in high risk PRs, explain how you know your testing is
enough.
-->
  • Loading branch information
ryanthecoder authored Jan 29, 2025
1 parent fd2533d commit 2661488
Show file tree
Hide file tree
Showing 3 changed files with 187 additions and 8 deletions.
12 changes: 11 additions & 1 deletion api/src/opentrons/protocol_engine/state/geometry.py
Original file line number Diff line number Diff line change
Expand Up @@ -1448,17 +1448,25 @@ def get_meniscus_height(
well_name: str,
) -> float:
"""Returns stored meniscus height in specified well."""
last_updated = self._wells.get_last_liquid_update(labware_id, well_name)
if last_updated is None:
raise errors.LiquidHeightUnknownError(
"Must LiquidProbe or LoadLiquid before specifying WellOrigin.MENISCUS."
)

well_liquid = self._wells.get_well_liquid_info(
labware_id=labware_id, well_name=well_name
)
if (
well_liquid.probed_height is not None
and well_liquid.probed_height.height is not None
and well_liquid.probed_height.last_probed == last_updated
):
return well_liquid.probed_height.height
elif (
well_liquid.loaded_volume is not None
and well_liquid.loaded_volume.volume is not None
and well_liquid.loaded_volume.last_loaded == last_updated
):
return self.get_well_height_at_volume(
labware_id=labware_id,
Expand All @@ -1468,15 +1476,17 @@ def get_meniscus_height(
elif (
well_liquid.probed_volume is not None
and well_liquid.probed_volume.volume is not None
and well_liquid.probed_volume.last_probed == last_updated
):
return self.get_well_height_at_volume(
labware_id=labware_id,
well_name=well_name,
volume=well_liquid.probed_volume.volume,
)
else:
# This should not happen if there was an update but who knows
raise errors.LiquidHeightUnknownError(
"Must LiquidProbe or LoadLiquid before specifying WellOrigin.MENISCUS."
f"Unable to find liquid height despite an update at {last_updated}."
)

def get_well_handling_height(
Expand Down
17 changes: 17 additions & 0 deletions api/src/opentrons/protocol_engine/state/wells.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from dataclasses import dataclass
from typing import Dict, List, Union, Iterator, Optional, Tuple, overload, TypeVar
from datetime import datetime

from opentrons.protocol_engine.types import (
ProbedHeightInfo,
Expand Down Expand Up @@ -177,6 +178,22 @@ def get_well_liquid_info(self, labware_id: str, well_name: str) -> WellLiquidInf
probed_volume=probed_volume_info,
)

def get_last_liquid_update(
self, labware_id: str, well_name: str
) -> Optional[datetime]:
"""Return the timestamp of the last load or probe done on the well."""
info = self.get_well_liquid_info(labware_id, well_name)
update_times: List[datetime] = []
if info.loaded_volume is not None and info.loaded_volume.volume is not None:
update_times.append(info.loaded_volume.last_loaded)
if info.probed_height is not None and info.probed_height.height is not None:
update_times.append(info.probed_height.last_probed)
if info.probed_volume is not None and info.probed_volume.volume is not None:
update_times.append(info.probed_volume.last_probed)
if len(update_times) > 0:
return max(update_times)
return None

def get_all(self) -> List[WellInfoSummary]:
"""Get all well liquid info summaries."""

Expand Down
166 changes: 159 additions & 7 deletions api/tests/opentrons/protocol_engine/state/test_geometry_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
TipGeometry,
ModuleDefinition,
ProbedHeightInfo,
ProbedVolumeInfo,
LoadedVolumeInfo,
WellLiquidInfo,
)
Expand Down Expand Up @@ -1577,10 +1578,14 @@ def test_get_well_position_with_meniscus_offset(
decoy.when(mock_labware_view.get_well_definition("labware-id", "B2")).then_return(
well_def
)
probe_time = datetime.now()
decoy.when(mock_well_view.get_last_liquid_update("labware-id", "B2")).then_return(
probe_time
)
decoy.when(mock_well_view.get_well_liquid_info("labware-id", "B2")).then_return(
WellLiquidInfo(
probed_volume=None,
probed_height=ProbedHeightInfo(height=70.5, last_probed=datetime.now()),
probed_height=ProbedHeightInfo(height=70.5, last_probed=probe_time),
loaded_volume=None,
)
)
Expand Down Expand Up @@ -1639,10 +1644,14 @@ def test_get_well_position_with_volume_offset_raises_error(
decoy.when(mock_labware_view.get_well_definition("labware-id", "B2")).then_return(
well_def
)
probe_time = datetime.now()
decoy.when(mock_well_view.get_last_liquid_update("labware-id", "B2")).then_return(
probe_time
)
decoy.when(mock_well_view.get_well_liquid_info("labware-id", "B2")).then_return(
WellLiquidInfo(
loaded_volume=None,
probed_height=ProbedHeightInfo(height=45.0, last_probed=datetime.now()),
probed_height=ProbedHeightInfo(height=45.0, last_probed=probe_time),
probed_volume=None,
)
)
Expand Down Expand Up @@ -1698,13 +1707,17 @@ def test_get_well_position_with_meniscus_and_literal_volume_offset(
decoy.when(
mock_addressable_area_view.get_addressable_area_position(DeckSlotName.SLOT_4.id)
).then_return(slot_pos)
probe_time = datetime.now()
decoy.when(mock_well_view.get_last_liquid_update("labware-id", "B2")).then_return(
probe_time
)
decoy.when(mock_labware_view.get_well_definition("labware-id", "B2")).then_return(
well_def
)
decoy.when(mock_well_view.get_well_liquid_info("labware-id", "B2")).then_return(
WellLiquidInfo(
loaded_volume=None,
probed_height=ProbedHeightInfo(height=45.0, last_probed=datetime.now()),
probed_height=ProbedHeightInfo(height=45.0, last_probed=probe_time),
probed_volume=None,
)
)
Expand Down Expand Up @@ -1771,10 +1784,14 @@ def test_get_well_position_with_meniscus_and_float_volume_offset(
decoy.when(mock_labware_view.get_well_definition("labware-id", "B2")).then_return(
well_def
)
probe_time = datetime.now()
decoy.when(mock_well_view.get_last_liquid_update("labware-id", "B2")).then_return(
probe_time
)
decoy.when(mock_well_view.get_well_liquid_info("labware-id", "B2")).then_return(
WellLiquidInfo(
loaded_volume=None,
probed_height=ProbedHeightInfo(height=45.0, last_probed=datetime.now()),
probed_height=ProbedHeightInfo(height=45.0, last_probed=probe_time),
probed_volume=None,
)
)
Expand Down Expand Up @@ -1840,10 +1857,14 @@ def test_get_well_position_raises_validation_error(
decoy.when(mock_labware_view.get_well_definition("labware-id", "B2")).then_return(
well_def
)
probe_time = datetime.now()
decoy.when(mock_well_view.get_last_liquid_update("labware-id", "B2")).then_return(
probe_time
)
decoy.when(mock_well_view.get_well_liquid_info("labware-id", "B2")).then_return(
WellLiquidInfo(
loaded_volume=None,
probed_height=ProbedHeightInfo(height=40.0, last_probed=datetime.now()),
probed_height=ProbedHeightInfo(height=40.0, last_probed=probe_time),
probed_volume=None,
)
)
Expand Down Expand Up @@ -1905,10 +1926,14 @@ def test_get_meniscus_height(
decoy.when(mock_labware_view.get_well_definition("labware-id", "B2")).then_return(
well_def
)
probe_time = datetime.now()
decoy.when(mock_well_view.get_last_liquid_update("labware-id", "B2")).then_return(
probe_time
)
decoy.when(mock_well_view.get_well_liquid_info("labware-id", "B2")).then_return(
WellLiquidInfo(
loaded_volume=LoadedVolumeInfo(
volume=2000.0, last_loaded=datetime.now(), operations_since_load=0
volume=2000.0, last_loaded=probe_time, operations_since_load=0
),
probed_height=None,
probed_volume=None,
Expand Down Expand Up @@ -3349,10 +3374,14 @@ def test_validate_dispense_volume_into_well_meniscus(
decoy.when(mock_labware_view.get_well_geometry("labware-id", "A1")).then_return(
inner_well_def
)
probe_time = datetime.now()
decoy.when(mock_well_view.get_last_liquid_update("labware-id", "A1")).then_return(
probe_time
)
decoy.when(mock_well_view.get_well_liquid_info("labware-id", "A1")).then_return(
WellLiquidInfo(
loaded_volume=None,
probed_height=ProbedHeightInfo(height=40.0, last_probed=datetime.now()),
probed_height=ProbedHeightInfo(height=40.0, last_probed=probe_time),
probed_volume=None,
)
)
Expand All @@ -3369,6 +3398,129 @@ def test_validate_dispense_volume_into_well_meniscus(
)


def test_get_latest_volume_information(
decoy: Decoy,
mock_labware_view: LabwareView,
mock_well_view: WellView,
subject: GeometryView,
) -> None:
"""It should raise an InvalidDispenseVolumeError if too much volume is specified."""
# Setup
labware_def = _load_labware_definition_data()
assert labware_def.wells is not None
well_def = labware_def.wells["A1"]
assert labware_def.innerLabwareGeometry is not None
inner_well_def = labware_def.innerLabwareGeometry["welldefinition1111"]

load_time = datetime.min
probe_time = datetime.now()

decoy.when(mock_labware_view.get_well_definition("labware-id", "A1")).then_return(
well_def
)
decoy.when(mock_labware_view.get_well_geometry("labware-id", "A1")).then_return(
inner_well_def
)
ten_ul_height = subject.get_well_height_at_volume(
labware_id="labware-id", well_name="A1", volume=10.0
)
twenty_ul_height = subject.get_well_height_at_volume(
labware_id="labware-id", well_name="A1", volume=20.0
)

# Make sure Get height with no information raises an error
decoy.when(mock_well_view.get_last_liquid_update("labware-id", "A1")).then_return(
None
)
decoy.when(mock_well_view.get_well_liquid_info("labware-id", "A1")).then_return(
WellLiquidInfo(
loaded_volume=None,
probed_height=None,
probed_volume=None,
)
)
decoy.when(mock_well_view.get_last_liquid_update("labware-id", "A1")).then_return(
None
)

with pytest.raises(errors.LiquidHeightUnknownError):
subject.get_meniscus_height(labware_id="labware-id", well_name="A1")
# Make sure get height with a valid load returns the correct height
decoy.when(mock_well_view.get_well_liquid_info("labware-id", "A1")).then_return(
WellLiquidInfo(
loaded_volume=LoadedVolumeInfo(
volume=10.0, last_loaded=load_time, operations_since_load=0
),
probed_height=None,
probed_volume=None,
)
)

decoy.when(mock_well_view.get_last_liquid_update("labware-id", "A1")).then_return(
load_time
)
assert (
subject.get_meniscus_height(labware_id="labware-id", well_name="A1")
== ten_ul_height
)

# Make sure that if there is a probe after a load that we get the correct height
decoy.when(mock_well_view.get_well_liquid_info("labware-id", "A1")).then_return(
WellLiquidInfo(
loaded_volume=LoadedVolumeInfo(
volume=10.0, last_loaded=load_time, operations_since_load=0
),
probed_height=ProbedHeightInfo(height=40.0, last_probed=probe_time),
probed_volume=None,
)
)
decoy.when(mock_well_view.get_last_liquid_update("labware-id", "A1")).then_return(
probe_time
)

assert subject.get_meniscus_height(labware_id="labware-id", well_name="A1") == 40.0

# Simulate a pipetting action and make sure we get the height based on the most current one
decoy.when(mock_well_view.get_well_liquid_info("labware-id", "A1")).then_return(
WellLiquidInfo(
loaded_volume=LoadedVolumeInfo(
volume=10.0, last_loaded=load_time, operations_since_load=1
),
probed_height=None,
probed_volume=ProbedVolumeInfo(
volume=20.0, last_probed=probe_time, operations_since_probe=1
),
)
)
decoy.when(mock_well_view.get_last_liquid_update("labware-id", "A1")).then_return(
probe_time
)
assert (
subject.get_meniscus_height(labware_id="labware-id", well_name="A1")
== twenty_ul_height
)

# Simulate a calling load_liquid after a probe and make sure we get the height based on the load_liquid
decoy.when(mock_well_view.get_well_liquid_info("labware-id", "A1")).then_return(
WellLiquidInfo(
loaded_volume=LoadedVolumeInfo(
volume=10.0, last_loaded=datetime.max, operations_since_load=0
),
probed_height=ProbedHeightInfo(height=40.0, last_probed=probe_time),
probed_volume=ProbedVolumeInfo(
volume=20.0, last_probed=probe_time, operations_since_probe=0
),
)
)
decoy.when(mock_well_view.get_last_liquid_update("labware-id", "A1")).then_return(
datetime.max
)
assert (
subject.get_meniscus_height(labware_id="labware-id", well_name="A1")
== ten_ul_height
)


@pytest.mark.parametrize(
[
"labware_id",
Expand Down

0 comments on commit 2661488

Please sign in to comment.