RFC: prototyping papi transfer + engine core + pe load liquid class

[sanni-t]

RFC for AUTH-865

This is a prototype of what PAPI & PE implementation of liquid class handling will roughly look like. It shows the flow of control between PAPI & PE when executing the transfer function that uses a liquid class. The code is a mix of pseudo-code and true code.

There are quite a few lines that build on top of assumed future changes, for example, TouchTip command is used here assuming that it'll have an mmToEdge param. The most visible such assumption is that each of the liquid handling PE commands is assumed to have a liquidClassId param. This is one of the main things I'm looking for comments on whether it's a good thing or not.

Summary of changes:

1. Adds a protocol engine LoadLiquidClass command that will take in a liquid class's properties and save it in state. It will provide a liquid class ID that can be used in liquid handling functions.
2. Adds an InstrumentContext.transfer() (overloaded) method that takes in a liquid class
3. Assumes that there will be some sort of minimal 'transfer plan builder' that will build each 1-to-1 transfer's locations, including tip pick up location (if any), source well, dest well, and tip drop well.
4. Adds an implementation of the engine core's transfer function that calls engine commands for executing each 1-to-1 transfer from above, while using the liquid class properties.

Request for comments:

1. Opinion on this approach
2. Is point 3 above the correct way to go? It is slightly similar to the transfer planner we currently have without going into absolutely all details of each transfer. It keeps the planning to a minimal so that we can use the tip tracking provision of the protocol api and convert the location types to what's expected by the engine core.
3. This approach will require all the liquid handling (and maybe WaitForDuration) commands to include a new optional liquidClassId param. Any problems with that?

[sfoster1]

As you know, I do love a builder pattern so I support it here. More comments inline.

[sfoster1]

having the pipette id and tiprack id in here doesn't seem right to me. It means the command orchestration layer has to do a bunch of extra work to load the full liquid class and tie it back to a possibly-generated ID for the relevant pipette. It seems better to have the command take in the full liquid class, and then later when the command orchestration layer references the LC in a command like aspirate the engine is the one to verify that the LC has properties for the pipette specified by ID in the aspirate and the tip that pipette is using and pulls the right kind of properties.

Also is tiprackId an engine instance ID? or is it a labware URI? If it's an instance ID, wouldn't you have to load a huge number of liquid classes?

[sanni-t]

You're totally right. Going to change this to accept the liquid class as is, and the engine then saves only the properties related to the pipette ID & tiprack uri in question. I want the engine to save only relevant parts of the liquid class so that the run summary doesn't get bloated because of parts of the liquid class that will be irrelevant not just to the transfer in question but the entire run. This will be definitely useful when handling built-in liquid classes which will have properties corresponding to all pipette+tiprack combos we support.

[sfoster1]

Even trimming out unnecessary stuff strikes me as a bit of a premature optimization. IMO if we're going to put the data in the engine, we should put the entire data in the engine. Sure, the liquid classes are big, but you're gonna be using like 1-10ish of them per protocol, right?

[sanni-t]

So, unlike other definitions like labware and modules, liquid class properties are going to be mutable. If the engine were to save the entire liquid class, then every time there's a change to the properties for a specific pipette+tip combination, the snapshots of the entire liquid class will need to be updated. If we save split properties per pipette+tip combination then only the snapshot of the specific combo will need to be updated.

But I understand your point about pre-mature optimization; maybe the optimization above isn't worth the code bloat. I'll change this to saving complete liquid class and if we run into the above issue more often than we'd like to then I'll add the optimization.

[ddcc4]

Hey @sfoster1, I don't know if saving the entire LiquidClass is a good idea, and trying to save the whole thing is making me struggle with how to represent the data in the PE.

So LiquidClass starts out with the RECOMMENDED settings for every possible pipette and tip type, right? So it looks something like:

LiquidClass {
  - pipette1 + tip2 -> TransferProperties(flowrate=5, ...)
  - pipette2 + tip3 -> TransferProperties(flowrate=6, ...)
  - etc ...
}

But for any given transfer, the user can customize the settings. So the user's code is going to look something like this:

my_liquid_class = ...retrieve LiquidClass for viscous liquids...
transfer_settings = my_liquid_class.get_for(pipette2, tip3) # the particular pipette/tip we're using

# Transfer #1
transfer_settings.blah.flowrate = 7 # customize the flow rate
transfer_liquid(..., liquid_class=my_liquid_class)

# Transfer #2
transfer_settings.blah.flowrate = 8 # customize the flow rate differently
transfer_liquid(..., liquid_class=my_liquid_class)

Inside the implementation of transfer_liquid(), Sanniti is going to upload the settings for that transfer onto the PE. In her original proposal, she would upload something like this to record the settings we actually used:

# Transfer #1:
pipette=pipette2, tip=tip3, flowrate=7
# Transfer #2:
pipette=pipette2, tip=tip3, flowrate=8

With your proposal to store the entire LiquidClass, Sanniti would have to upload the settings for every possible pipette/tip for every transfer, even though only one of the pipettes is used for each transfer. Semantically, I don't know what it means to store the settings for the all pipettes we did NOT use (especially since we would probably have modified the settings if we HAD used those pipettes instead).

And in terms of implementation, if we have to store the entire LiquidClass, but we also want to know what settings were used for each particular transfer, then Sanniti wants me to build a fancy mechanism to compare the LiquidClasses from transfer to transfer and assign a new UUID to each distinct version of the LiquidClass we used (AUTH-851). It might make my life easier if we only had to store the settings we actually used for each transfer like in the original proposal here.

[sanni-t]

I think all questions have been answered and the architecture has been approved in in-person talks.
Some additional tasks needed to enable this arch change are documented in AUTH-1063 and AUTH-866