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): add pipette movement restrictions for 96ch col A1 and 8ch single A1 #14230

Merged
merged 14 commits into from
Jan 10, 2024

Conversation

sanni-t
Copy link
Member

@sanni-t sanni-t commented Dec 18, 2023

Closes RSS-423

Overview

Implements deck conflict checking for when:

  • moving a 96 channel pipette configured to use the leftmost nozzle column
  • moving an 8 channel pipette configured to use the backmost nozzle

Even though 7.1.0 release doesn't officially support any partial configurations other than 96-channel rightmost column, one can write protocols that use other configurations. The conflict checking added in this PR makes these not-yet-supported configurations at least safe for use.

Test Plan

  • Below is a protocol built from the integration test for deck conflict. Make sure this analyses correctly on the robot (protocol has errors on purpose) and is actually able to run without any collisions once errors are removed and analysis passes.
  • From my calculations and the integration tests, it seems that the 96-channel first column can access the full deck slot area, including the right edge of deck slot column 3. Please verify that this is actually true.

Test protocol:

from opentrons.protocol_api import COLUMN, ALL


requirements = {
    "robotType": "Flex",
    "apiLevel": "2.16"
}

def run(protocol):

    instrument = protocol.load_instrument("flex_96channel_1000", mount="left")
    trash_labware = protocol.load_labware("opentrons_1_trash_3200ml_fixed", "A3")
    instrument.trash_container = trash_labware

    badly_placed_tiprack = protocol.load_labware("opentrons_flex_96_tiprack_50ul", "C2")
    well_placed_tiprack = protocol.load_labware("opentrons_flex_96_tiprack_50ul", "A1")
    tiprack_on_adapter = protocol.load_labware(
        "opentrons_flex_96_tiprack_50ul",
        "C3",
        adapter="opentrons_flex_96_tiprack_adapter",
    )

    # ############  SHORT LABWARE  ################
    # These labware should be to the east of tall labware to avoid any partial tip deck conflicts
    badly_placed_plate = protocol.load_labware("nest_96_wellplate_200ul_flat", "B1")
    well_placed_plate = protocol.load_labware("nest_96_wellplate_200ul_flat", "B3")

    # ############ TALL LABWARE ###############
    my_tuberack = protocol.load_labware(
        "opentrons_10_tuberack_falcon_4x50ml_6x15ml_conical", "B2"
    )

    # ########### Use Partial Nozzles #############
    instrument.configure_nozzle_layout(style=COLUMN, start="A1")

    # Raises error because of conflict with tiprack on adapter in C3
    instrument.pick_up_tip(badly_placed_tiprack.wells_by_name()["H12"])

    # No error cuz within pipette extent bounds and no taller labware to right of tiprack
    instrument.pick_up_tip(well_placed_tiprack.wells_by_name()["A12"])

    # No error cuz no labware on right of plate
    instrument.aspirate(25, well_placed_plate.wells_by_name()["A4"])
    # No error cuz dispensing from high above plate, so it clears tuberack on the right
    instrument.dispense(25, badly_placed_plate.wells_by_name()["A1"].top(150))

    
    # Raises error because of conflict with tuberack
    instrument.aspirate(25, badly_placed_plate.wells_by_name()["A1"])

    # No error cuz no taller labware on the right
    instrument.aspirate(10, my_tuberack.wells_by_name()["A1"])

    instrument.drop_tip()

    # ######## CHANGE CONFIG TO ALL #########
    instrument.configure_nozzle_layout(style=ALL, tip_racks=[tiprack_on_adapter])

    # No error because of full config
    instrument.pick_up_tip()

    # No error NOW because of full config
    instrument.aspirate(50, badly_placed_plate.wells_by_name()["A1"])

    # No error NOW because of full config
    instrument.dispense(50, badly_placed_plate.wells_by_name()["A1"].bottom())

Changelog

  • updated the pipette movement deck conflict checks to include conflicts with east and south slots for the appropriate pipette & nozzle configuration.
  • added integration tests for the 96-channel configurations. Did not add unit test for it since the existing ones cover the basic functionality testing and an integration test is more appropriate for such instances where the final result is dependent on a lot of internal state parameters.

Review requests

  • Test it on a robot to make sure that all the new as well as old conflicts with a 96-channel work as expected.
  • Any code suggestions

Risk assessment

Medium. Adds new restrictions for yet unsupported pipette configuration but also touches the logic for supported configurations.

@sanni-t sanni-t requested a review from a team as a code owner December 18, 2023 11:00
Copy link

codecov bot commented Dec 18, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d377e93) 68.53% compared to head (f2e1492) 68.53%.
Report is 1 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             edge   #14230   +/-   ##
=======================================
  Coverage   68.53%   68.53%           
=======================================
  Files        2421     2421           
  Lines       67108    67108           
  Branches     8823     8823           
=======================================
  Hits        45993    45993           
  Misses      19040    19040           
  Partials     2075     2075           
Flag Coverage Δ
g-code-testing 96.48% <ø> (ø)
notify-server 89.13% <ø> (ø)

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 good to me pending comments

).to_equivalent_for_robot_type(engine_state.config.robot_type)
destination_slot_num = _deck_slot_to_int(DeckSlotLocation(slotName=labware_slot))
adjacent_slot_num = None
if primary_nozzle == "A12":
Copy link
Member

Choose a reason for hiding this comment

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

we probably want to change this eventually to "column 1"/"column 12" via the column mappings in the pipette geometry def.

).to_equivalent_for_robot_type(engine_state.config.robot_type)
destination_slot = _deck_slot_to_int(DeckSlotLocation(slotName=labware_slot))
adjacent_slot_num = None
if primary_nozzle == "H1":
Copy link
Member

Choose a reason for hiding this comment

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

probably want to change this to column mapping via the geometry def at some point

)
# TODO (spp, 2023-11-07): check for 8-channel nozzle H1 extents on Flex & OT2
if pipette_channels == 96 and nozzle_config == NozzleConfigurationType.COLUMN:
if primary_nozzle == "A12":
Copy link
Member

Choose a reason for hiding this comment

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

probably want to change this to column mapping in the geometry def at some point

@CaseyBatten
Copy link
Contributor

When attempting to drop tips in a trash bin in A3 with the leftmost column (A1) filled with tips, the 96 channel collides with the right side of the robot. Also revealed through this, when the 96 channel has a single column of tips in A1 and is at its rightmost position, the further it can reach is A10. The single column will never reach A11 or A12 within the tiprack/wellplate/labware it is interacting with.

@sanni-t
Copy link
Member Author

sanni-t commented Dec 19, 2023

When attempting to drop tips in a trash bin in A3 with the leftmost column (A1) filled with tips, the 96 channel collides with the right side of the robot.

Hmm, the robot extents/ pipette bounds checking currently doesn't happen when moving to addressable areas so this isn't unexpected.

Also revealed through this, when the 96 channel has a single column of tips in A1 and is at its rightmost position, the further it can reach is A10. The single column will never reach A11 or A12 within the tiprack/wellplate/labware it is interacting with.

I'm assuming you're talking about column A10 of a labware in the third deck column. This is an important thing to fix in this PR. The behavior indicates that the numbers I've used for out-of-bounds check are incorrect. I'll find out what the correct numbers are and update this PR

@SyntaxColoring
Copy link
Contributor

When attempting to drop tips in a trash bin in A3 with the leftmost column (A1) filled with tips, the 96 channel collides with the right side of the robot.

Hmm, the robot extents/ pipette bounds checking currently doesn't happen when moving to addressable areas so this isn't unexpected.

I think I told @CaseyBatten yesterday that this would be a problem for v7.1.0, but I was under the mistaken impression at the time that we were trying to support actually using column 1. After discussing with @sanni-t and Linda, I agree that it's okay for that gap in motion planning and validation to not be fixed in this PR.

Copy link
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

I'm still catching up on the 96-channel code so I haven't worked all the way through this, but this looks good to me so far. Just some minor suggestions.

Copy link
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

Also LGTM pending comments.

@sanni-t sanni-t force-pushed the api-add_left_column_config_restrictions branch from bedf52b to ced6577 Compare January 8, 2024 21:01
@sanni-t sanni-t requested review from a team as code owners January 8, 2024 21:01
@sanni-t sanni-t requested a review from a team January 8, 2024 21:01
@sanni-t sanni-t requested a review from a team as a code owner January 8, 2024 21:01
@sanni-t sanni-t requested review from mjhuff and removed request for a team January 8, 2024 21:01
@sanni-t sanni-t changed the base branch from chore_release-7.1.0 to edge January 8, 2024 21:01
@sanni-t
Copy link
Member Author

sanni-t commented Jan 10, 2024

Tested the new robot bounds on a Flex and all accessible areas were reachable by the pipette without collision while the locations that raise an error are actually not accessible by the pipette.
Exception is when the location we're moving to is an addressable area and doing checks for addressable areas is covered in a separate ticket.

@sanni-t
Copy link
Member Author

sanni-t commented Jan 10, 2024

@ecormany @jwwojak tagging you here to let you know about this robot bounds check change. Turns out we can access the first column of thermocycler labware using A12 nozzle column of the 96 channel (although it's very close to the left bound, so big offsets to this location might still result in out of bounds error). Once this change is released the API will no longer raise an error for this condition and we will have to update the docs that mention this restriction.

from opentrons.protocol_api.core.engine.deck_conflict import (
PartialTipMovementNotAllowedError,
)
from opentrons.types import Point
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
from opentrons.types import Point

There's a lint error from this unused import, but this otherwise looks good to me to merge.

@sanni-t sanni-t merged commit d7cbd75 into edge Jan 10, 2024
23 checks passed
@sanni-t sanni-t deleted the api-add_left_column_config_restrictions branch January 10, 2024 21:15
mjhuff pushed a commit that referenced this pull request Jan 23, 2024
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.

None yet

4 participants