feat(api): group together multiple wells for multi-channel transfers#17900
Conversation
…_and_normalize_transfer_args
|
This is a very clever solution. That said, I’m a little sad to see us moving away from what feels like a simpler API—one that only ever requires thinking in terms of primary tip. Are there examples where this additional way of targeting shines or simplifies use cases? I’d love to see them to help better understand adding this method of targeting. |
I do think that it is more "readable" (as in expressing the intent of the function and having a lay person read it) having a transfer between two columns be expressed like instead of And to go even further with this example, if you wanted to say transfer an entire plate's worth of columns, I think its much much more readable having this rather then That said, the primary nozzle targeting is still available if with the |
|
Thank you for the examples and I agree, that is more readable. Did not think of it that way. |
I agree with everything you said, yeah. I think this bit in particular is going to be a sticking point no matter what solution we end up with - either it's not clear from the protocol what will occur, or you have to edit the list. I think a good way to solve this might be to have some extra utilities that would be like |
SyntaxColoring
left a comment
There was a problem hiding this comment.
Still looking through this (feel free to not wait for me if you have other approvals), but it all looks sensible so far. One question though.
| Union[types.Location, labware.Well, TrashBin, WasteChute] | ||
| ] = None, | ||
| return_tip: bool = False, | ||
| visit_every_well: bool = False, |
There was a problem hiding this comment.
Nitpick: I find the name visit_every_well a little confusing because even when this is False, the pipette does still visit every well, in the sense that some tip will descend into every well. In other words, it's not as if visit_every_well=False skips wells.
There was a problem hiding this comment.
What about, like, locate_with_primary_nozzle=False? Assuming "primary nozzle" is already a public-facing PAPI concept. I always forget the exact terminology.
| @pytest.fixture | ||
| def mock_96_well_labware(decoy: Decoy) -> Labware: | ||
| """Get a mock 96 well labware.""" | ||
| mock_96_well_labware = decoy.mock(cls=Labware) | ||
| decoy.when(mock_96_well_labware.parameters).then_return({"format": "96Standard"}) # type: ignore[typeddict-item] | ||
| labware_wells_by_column = [] | ||
| for column in WELLS_BY_COLUMN_96: | ||
| wells_by_column = [] | ||
| for well_name in column: | ||
| mock_well = decoy.mock(cls=Well) | ||
| decoy.when(mock_well.well_name).then_return(well_name) | ||
| wells_by_column.append(mock_well) | ||
| labware_wells_by_column.append(wells_by_column) | ||
| decoy.when(mock_96_well_labware.columns()).then_return(labware_wells_by_column) | ||
| return mock_96_well_labware |
There was a problem hiding this comment.
Nit:
I think these tests would get a lot cleaner if transfer_liquid_utils.py took just the data that it actually needed, like well names and quirks, instead of full Labware objects. Then you could do all of this without mocks.
|
This seems good to me. While this isn't the exact behavior of |
SyntaxColoring
left a comment
There was a problem hiding this comment.
I haven't had a chance to really scrutinize the logic because of my own time constraints, but this all looks reasonable and I agree that we can keep polishing it (e.g. bikeshed the name visit_every_well) after feature freeze.
| dest=(types.Location(types.Point(), labware=dest), dest._core), | ||
| dest=( | ||
| types.Location(types.Point(), labware=verified_dest), | ||
| verified_dest._core, | ||
| ), |
There was a problem hiding this comment.
Not introduced in this PR, but is types.Point() correct here? I thought Location.point was always in absolute deck coordinates, so wouldn't this try to pipette at deck coordinates (0, 0, 0)?
There was a problem hiding this comment.
We discard this Point in the implementation, or rather, we re-calculate it based on the positions specified in the liquid class. We need to pass this Location type in order to have access to the labware (or LabwareLike) object when updating the location cache.
sanni-t
left a comment
There was a problem hiding this comment.
Looks good! I like the consolidated well grouping logic!
Just have some code organizational nitpicks but they don't need to be addressed in this PR.
Overview
Closes AUTH-1412.
This PR adds multi-channel pipette support to the liquid class transfer methods (
transfer_liquid,distribute_liquid,consolidate_liquid) by means of adding well grouping logic for multi-tip configurations addressing 96 and 384 well plates.The existing aspirate and dispense commands take a single well target, but the transfer functions are able to take multiple well targets. This introduces an interesting model of behavior. When a single tip pipette/configuration is used, each well given as an argument is addressed by that instrument's tip. But in the existing implementation, and in the existing aspirate/dispense, multi-tip configurations instead are only telling the pipette where to move the pipette. To illustrate this difference, when pipetting with a single tip to a column (say A1 through H1 of a 96 well plate), those eight wells are given and those eight wells are each visited once by the single tip of the pipette. But to make an 8-channel pipette (with a primary nozzle of A1) visit all eight wells only once, the argument would instead by only
A1. It's up to the user to understand the primary nozzle and where the other tips will end up.This PR intends to introduce a new paradigm, which is tentatively called well grouping. The idea is to hold to the logic that each well given as an argument is visited exactly once by the pipette's tip (assuming the well is given once as an argument). This means that giving a column for an 8-channel pipette would have that pipette visit the entire column once in one go, rather than visiting each well with it's primary tip. Similarly, giving a whole row for a row configuration would have that row picked up in one go, and giving a whole labware's worth of wells for a full 96-tip configuration would have the picked up in one go.
While this logic can be used for partial columns and any future partial row or sub-rectangular configurations, this PR only allows this to be used for full columns, full rows and full 96-tip configurations.
The implementation of this logic is strict in that it requires the wells to be given in order, where the first well in any sequence of a column/row/whole well plate is the top-most/left-most/top-left-most well in the plate. Then every well after that that would be covered by the pipette's tip configuration needs to be given, and an error will be raised if a different well is provided in that list or if not all wells are given.
The old
transferand related methods had a similar outcome in behavior, but was implemented in a much simpler and less robust way (It would literally reduce the list of wells to only wells in the first row, or first two rows for a 384 well plate for a pipette with more than one tip, no additional logic or checking if the whole column was provided).If this behavior is not desired by a user, a new flag has been added,
visit_every_well, that if set to beTrue, will revert to the previous behavior of unconditionally visiting every well given with the primary nozzle.Test Plan and Hands on Testing
A lot of the different combinations of configurations, 96 and 384 well plates, and scenarios have been tested in unit tests, and the following protocol has been tested on a robot to confirm it works as expected.
Changelog
transfer_liquid,distribute_liquid,consolidate_liquidfor 8-channel columns, 12-channel rows and 96-channel full configurations, for 96 and 384 well platesvisit_every_wellargument to revert to previous behavior where every well would be visited by the primary nozzleReview requests
Naming of things, arguments and error messages specifically, as always. There's also the question of if this behavior should be the default (I believe it should be but there could be an argument to reverse it).
There's also a couple of interesting quirks to the way I've implemented this logic. The primary one being that although wells are expected to be in order, really only the first well in a sequence matters, since that's the one we will choose to target and then check the next N-1 wells (where N is the number of tips in the configuration) to see if they match (384 well plates are a little more complicated but hold mostly true). This means for an 8-channel column, an ordered list of wells A1 through H1 would work the same as A1, and then an arbitrary order of B1 through H1, but having anything other than A1 be first would cause the function to fail (assuming A1 is in that list, as it would not be covered if you targeted anything below it). There would be ways of either making this behavior smarting or enforcing stricter rules to make this more clear, but it was worth mentioning here these corner cases.
Risk assessment
Medium, this does change the default behavior for the new liquid class transfer methods and a significant way, but it is limited to these new functions that have yet to been released and it does function similarly to the old transfer with an 8-channel pipette.