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): support get_max_travel_z for Flex and enable use_virtual_pipettes #12515

Merged
merged 5 commits into from
Apr 19, 2023

Conversation

jbleon95
Copy link
Contributor

Overview

Closes RCORE-655.

This PR adds support for calculating max_travel_z while analyzing an OT-3 Standard robot type protocol, and enables the use_virtual_pipettes field for those protocols. The value used for the max height is the same one the OT3 Hardware API is using, but avoids making a call to the hardware API.

With use_virtual_pipettes on, significant speed-ups both on the app and robot side analysis have been seen, with speedups of 66% seen for real science protocols on the robot, and up to 90% for pipette heavy protocols.

Test Plan

Analysis was tested on a number of protocols, including science ones such as OT-3 Omega HDQ DNA Extraction: Bacteria- Tissue Protocol Park Tips and the 96-channel protocol Omega HDQ DNA Extraction: Bacteria 96 channel testing on both the app and the Flex to ensure they still successfully analyze

Additionally, for speed calculations, the following protocol was tested both with and without the use_virtual_pipettes flag on.

from opentrons.types import Location

metadata = {
    'protocolName': 'use_virtual_pipettes speed test',
}

requirements = {
    "robotType": "OT-3",
    "apiLevel": "2.14"
}

def run(context):
    p1000rack = context.load_labware('opentrons_96_tiprack_1000ul', 1)
    p1000 = context.load_instrument('p1000_single_gen3', 'left', tip_racks=[p1000rack])
    labware = context.load_labware("corning_96_wellplate_360ul_flat", 3)
    well1 = labware.wells()[0]
    well2 = labware.wells()[1]
    well3 = labware.wells()[2]

    p1000.pick_up_tip()

    for _ in range(20000):
        p1000.aspirate(location=well1, volume=100)

        p1000.dispense(location=well2)

       p1000.blow_out(location=well1)

        p1000.touch_tip(location=well3)

        p1000.move_to(well1.bottom())

        # Test move to coordinates
        p1000.move_to(Location(point=well1.top().point, labware=None))


    p1000.drop_tip()

Without the flag on, this takes over 10 minutes to simulate on the app running on an M1 MacBook Pro. With the flag on, this protocol only takes 1 minute to complete analysis.

Changelog

  • Add simulated value for OT-3/Flex get_instrument_max_height for use in analysis/simulation
  • Remove check to use use_virtual_pipettes for OT-2 only, enabling it for OT-3/Flex
  • Fixed incorrect value in shared_data for 96 channel pipette that was causing simulated runs to incorrectly fail analysis

Review requests

  • The simulated max instrument height was chosen to match what the hardware controller was returning with the OT3Simulator. Does using this value make sense, or should it be adjusted somewhat?

Risk assessment

Lowish, this only affects analysis and no changes have been made that will affect actual runs. I've done my due diligence in testing around 10 OT-3 protocols I have loaded, to ensure they still pass. While most of the code has been running successfully on the OT-2 since end of February (and is in 6.3), there is always the small chance of a false failed run. However this feature can be turned off and returned to pre-existing behavior by enabling the disableFastProtocolUpload FF

@jbleon95 jbleon95 requested a review from a team as a code owner April 18, 2023 18:21
@jbleon95 jbleon95 requested review from sfoster1 and a team April 18, 2023 18:22
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.

Yeah this is fine. I'd love to think of another way to do this.

@codecov
Copy link

codecov bot commented Apr 18, 2023

Codecov Report

Merging #12515 (8c5b696) into edge (dfcfca5) will increase coverage by 0.19%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #12515      +/-   ##
==========================================
+ Coverage   73.44%   73.64%   +0.19%     
==========================================
  Files        1498     2255     +757     
  Lines       49103    61991   +12888     
  Branches     2991     6547    +3556     
==========================================
+ Hits        36066    45654    +9588     
- Misses      12581    14781    +2200     
- Partials      456     1556    +1100     
Flag Coverage Δ
app 72.24% <ø> (+25.39%) ⬆️
notify-server 89.13% <ø> (ø)
protocol-designer 46.34% <ø> (ø)
shared-data 73.30% <ø> (-1.96%) ⬇️
step-generation 88.20% <ø> (ø)

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

Impacted Files Coverage Δ
api/src/opentrons/protocol_runner/__init__.py 100.00% <ø> (ø)
...ntrons/protocol_runner/create_simulating_runner.py 84.61% <ø> (-2.89%) ⬇️
...i/src/opentrons/protocol_runner/legacy_wrappers.py 100.00% <ø> (ø)
...i/src/opentrons/protocol_runner/protocol_runner.py 100.00% <ø> (ø)
g-code-testing/g_code_parsing/g_code_engine.py 82.02% <ø> (+0.70%) ⬆️
...obot-server/robot_server/protocols/dependencies.py 100.00% <ø> (ø)
...server/robot_server/protocols/protocol_analyzer.py 100.00% <ø> (ø)
robot-server/robot_server/runs/engine_store.py 96.00% <ø> (-0.08%) ⬇️

... and 759 files with indirect coverage changes

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.

Nice!

A few comments and questions, but I think this is basically good to go.

shared-data/pipette/definitions/1/pipetteNameSpecs.json Outdated Show resolved Hide resolved
@SyntaxColoring SyntaxColoring self-requested a review April 19, 2023 18:14
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.

🏎️

@jbleon95 jbleon95 merged commit ca7bbba into edge Apr 19, 2023
@SyntaxColoring SyntaxColoring deleted the RCORE-655_max_instrument_height_flex branch February 29, 2024 22:11
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.

4 participants