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(protocol-designer): emit Python for aspirate command #17594

Merged
merged 1 commit into from
Feb 26, 2025

Conversation

ddcc4
Copy link
Contributor

@ddcc4 ddcc4 commented Feb 26, 2025

Overview

This implements Python code generation for the PD atomic aspirate command. AUTH-1094

The generated code looks like this:

pipette_left.aspirate(
    volume=75,
    location=well_plate_2["B2"].bottom(z=1),
    rate=716 / pipette_left.flow_rate.aspirate,
)

Test Plan and Hands on Testing

Added unit tests.

I also took the generated code and added it to a Python protocol and confirmed that it is understandable by simulate.

Review requests

PD has its own code to load the default flow rates for a particular pipette/tip/volume:

export function getMatchingTipLiquidSpecs(
pipetteEntity: PipetteEntity,
volume: number,
tiprack: string
): SupportedTip {

I was trying really hard to use those default flow rates to compute the ratio in JavaScript to pass to aspirate(rate=...). E.g., if we knew that the default flow rate is 700 uL/s, and the PD user wants a flow rate of 350 uL/s, I wanted to generate code like aspirate(rate=0.5).

But that turned out to be unworkable. PD has a very specific algorithm for picking the default flow rates, which is not the same as what the PAPI implements. Furthermore, we discovered that the current implementation for default flow rates in the PAPI is just wrong.

So in this PR, I had to emit an explicit division expression to generate the aspirate(rate=...) argument.

Risk assessment

Low. Python export in PD is not available to external users yet.

@ddcc4 ddcc4 requested review from sanni-t and ncdiehl11 February 26, 2025 18:42
@ddcc4 ddcc4 requested a review from a team as a code owner February 26, 2025 18:42
Copy link

codecov bot commented Feb 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 25.64%. Comparing base (7fdb309) to head (ed9425d).
Report is 2 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #17594      +/-   ##
==========================================
- Coverage   25.64%   25.64%   -0.01%     
==========================================
  Files        2841     2841              
  Lines      218631   218730      +99     
  Branches    17944    17944              
==========================================
+ Hits        56077    56092      +15     
- Misses     162539   162623      +84     
  Partials       15       15              
Flag Coverage Δ
protocol-designer 18.88% <100.00%> (+0.01%) ⬆️
step-generation 4.38% <100.00%> (+<0.01%) ⬆️

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

Files with missing lines Coverage Δ
...-generation/src/commandCreators/atomic/aspirate.ts 89.65% <100.00%> (+0.71%) ⬆️

... and 4 files with indirect coverage changes

)}]${formatPyWellLocation(wellLocation)}`,
// rate= is a ratio in the PAPI, and we have no good way to figure out what
// flowrate the PAPI has set the pipette to, so we just have to do a division:
`rate=${flowRate} / ${pipettePythonName}.flow_rate.aspirate`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

this will effectively stick the division expression into the Python rate arg value, right? is that what we want?

Copy link
Collaborator

Choose a reason for hiding this comment

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

sorry, just re-read the PR description. makes sense to me.

@ddcc4 ddcc4 merged commit fa6f2f6 into edge Feb 26, 2025
18 checks passed
@ddcc4 ddcc4 requested a review from jbleon95 February 26, 2025 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants