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): simulate liquid probe results #17582

Merged
merged 19 commits into from
Mar 5, 2025

Conversation

caila-marashaj
Copy link
Contributor

@caila-marashaj caila-marashaj commented Feb 25, 2025

Overview

In implementing liquid probing, we implemented a placeholder for simulating liquid_probe_in_place, where we just return the top position of a well unconditionally. This is fine for testing aspirate, but causes a problem if you want to dispense, or really simulate a realistic chain of liquid handling actions.

This code changes that simulated liquid probe function so that it creates and returns a SimulatedProbeResult object, and passes that into the protocol engine. This object will only be created by pipetting::liquid_probe_in_place, and will keep track of the state of aspirates/dispenses in a given well until another liquid_probe is called on the same well. I think we can eventually use this to do some cool error prevention/handling as it pertains to real liquid probe results from on-robot runs, but for now the running state will just be updated in the same place where well updates like _handle_liquid_probed and _handle_liquid_operated are made.

Changelog

  • have VirtualPipettingHandler::liquid_probe_in_place create & return a SimulatedProbeResult instead of well depth
  • create a LiquidTrackingType that is a Union of float and SimulatedProbeResult
  • modify geometry.py functions and places where liquid height/volume values are analyzed to handle SimulatedProbeResults
  • create a SimulatedType in update_types

Test Plan

  • check that protocols involving both liquid tracking and regular aspirate/dispense pass protocol analysis
  • make sure liquid tracking protocols still behave as they do on edge
  • make sure non-liquid tracking aspirate/dispense protocols behave as they do on edge
  • unit tests

@caila-marashaj caila-marashaj changed the title Simulate liquid probe result feat(api): simulate liquid probe results Feb 25, 2025
@caila-marashaj caila-marashaj marked this pull request as ready for review February 25, 2025 20:04
@caila-marashaj caila-marashaj requested a review from a team as a code owner February 25, 2025 20:04
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

Inline changes, but let's also do the following:

The idea for this is that eventually the simulated-value standin will be an instance of a class that can support math operations and return itself; and that new instances will be returned from every unique probe. To support that now, we should make this a class and an instance of that class instead of a string literal. We should define it in opentrons.types, the way other data structures shared by the engine and protocol api, like SlotName, are defined; then we can import it into the engine.

@caila-marashaj caila-marashaj force-pushed the simulate-liquid-probe-result branch from f7af5d7 to c57ecd8 Compare February 28, 2025 17:15
Copy link

codecov bot commented Feb 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 25.68%. Comparing base (e583471) to head (7be981c).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             edge   #17582   +/-   ##
=======================================
  Coverage   25.68%   25.68%           
=======================================
  Files        2851     2851           
  Lines      219457   219457           
  Branches    17971    17971           
=======================================
  Hits        56363    56363           
  Misses     163079   163079           
  Partials       15       15           
Flag Coverage Δ
protocol-designer 18.94% <ø> (ø)
step-generation 4.37% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

Looks great! Let's maybe use the modern type syntax more though. I'm really excited for this one

@caila-marashaj caila-marashaj force-pushed the simulate-liquid-probe-result branch from 5ef5536 to 7be981c Compare March 4, 2025 17:00
@caila-marashaj caila-marashaj requested a review from a team as a code owner March 4, 2025 22:19
@y3rsh y3rsh added the gen-analyses-snapshot-pr Generate a healing PR if the analyses snapshot test fails label Mar 4, 2025
Copy link
Contributor

github-actions bot commented Mar 4, 2025

A PR has been opened to address analyses snapshot changes. Please review the changes here: #17653

@caila-marashaj caila-marashaj force-pushed the simulate-liquid-probe-result branch from 27b98e5 to da6d913 Compare March 5, 2025 16:28
Copy link
Contributor

github-actions bot commented Mar 5, 2025

A PR has been opened to address analyses snapshot changes. Please review the changes here: #17659

Copy link
Contributor

github-actions bot commented Mar 5, 2025

A PR has been opened to address analyses snapshot changes. Please review the changes here: #17659

@caila-marashaj caila-marashaj merged commit eae4cb3 into edge Mar 5, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gen-analyses-snapshot-pr Generate a healing PR if the analyses snapshot test fails
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants