From 266148833070577775cc8ce8437feb9188a5d125 Mon Sep 17 00:00:00 2001 From: Ryan Howard Date: Wed, 29 Jan 2025 10:01:47 -0500 Subject: [PATCH] fix(api): Use the latest information when estimating liquid height (#17374) # 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. ## Test Plan and Hands on Testing ## Changelog ## Review requests ## Risk assessment --- .../protocol_engine/state/geometry.py | 12 +- .../opentrons/protocol_engine/state/wells.py | 17 ++ .../state/test_geometry_view.py | 166 +++++++++++++++++- 3 files changed, 187 insertions(+), 8 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/geometry.py b/api/src/opentrons/protocol_engine/state/geometry.py index cf6b4521713..9a817564c67 100644 --- a/api/src/opentrons/protocol_engine/state/geometry.py +++ b/api/src/opentrons/protocol_engine/state/geometry.py @@ -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, @@ -1468,6 +1476,7 @@ 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, @@ -1475,8 +1484,9 @@ def get_meniscus_height( 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( diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py index fdcb8322094..727ef20da59 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -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, @@ -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.""" diff --git a/api/tests/opentrons/protocol_engine/state/test_geometry_view.py b/api/tests/opentrons/protocol_engine/state/test_geometry_view.py index bf82c17c6bc..f5f86fc8c4c 100644 --- a/api/tests/opentrons/protocol_engine/state/test_geometry_view.py +++ b/api/tests/opentrons/protocol_engine/state/test_geometry_view.py @@ -62,6 +62,7 @@ TipGeometry, ModuleDefinition, ProbedHeightInfo, + ProbedVolumeInfo, LoadedVolumeInfo, WellLiquidInfo, ) @@ -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, ) ) @@ -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, ) ) @@ -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, ) ) @@ -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, ) ) @@ -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, ) ) @@ -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, @@ -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, ) ) @@ -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",