feat(api, robot-server): set sent runtime values and report parameters in analysis #14735

jbleon95 · 2024-03-26T17:10:27Z

Overview

Completes the analysis portion of AUTH-222.

This PR hooks up the runtime parameter API code to setting sent override values as well as reporting the parameters in the analysis summary, using the Pydantic models created in #14638 and #14718.

To report the parameters in analysis, a new method was added to the definitions to convert them into the Pydantic models. This method can then be used by the ParameterContext to get a list of all defined definitions. In order to have access to these after run execution, the ProtocolRunner was refactored to own this class so we can have access to it after the parameters are parsed and validated.

A method to set the parameters was also added and hooked up to excute and run_python, which will validate and set all sent parameters to be used in the protocol, for both analysis and execution.

Test Plan

Used this protocol and checked the analysis result to make sure all parameters were being properly reported.

metadata = {
    'protocolName': 'Parameter Analysis Reporting Test',
}

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


def add_parameters(parameters):
    parameters.add_int(
        display_name="Sample count",
        variable_name="sample_count",
        default=6,
        minimum=1,
        maximum=12,
        description="How many samples to process."
    )
    parameters.add_float(
        display_name="Pipette volume",
        variable_name="volume",
        default=20.1,
        choices=[
            {"display_name": "Low Volume", "value": 10.23},
            {"display_name": "Medium Volume", "value": 20.1},
            {"display_name": "High Volume", "value": 50.5},
        ],
        description="How many microliters to pipette of each sample.",
        unit="µL"
    )
    parameters.add_bool(
        display_name="Dry Run",
        variable_name="dry_run",
        default=False,
        description="Skip aspirate and dispense steps."
    )
    parameters.add_str(
        display_name="Pipette Name",
        variable_name="pipette",
        choices=[
            {"display_name": "Single channel 50µL", "value": "flex_1channel_50"},
            {"display_name": "Eight Channel 50µL", "value": "flex_8channel_50"},
        ],
        default="flex_1channel_50",
        description="What pipette to use during the protocol.",
    )



def run(context):
    trash_bin = context.load_trash_bin('A3')
    tip_rack = context.load_labware('opentrons_flex_96_tiprack_50ul', 'D2')
    source = context.load_labware('opentrons_96_wellplate_200ul_pcr_full_skirt', 'C1')
    destination = context.load_labware('opentrons_96_wellplate_200ul_pcr_full_skirt', 'C3')

    pipette = context.load_instrument(context.params.pipette, mount="left", tip_racks=[tip_rack])

    for i in range(context.params.sample_count):
        source_well = source.wells()[i]
        destination_well = destination.wells()[i]

        pipette.pick_up_tip()
        pipette.move_to(source_well.top())
        if not context.params.dry_run:
            pipette.aspirate(context.params.volume, source_well)
            pipette.dispense(context.params.volume, destination_well)

        pipette.drop_tip()

and in the analysis

  "runTimeParameters": [
    {
      "displayName": "Sample count",
      "variableName": "sample_count",
      "description": "How many samples to process.",
      "type": "int",
      "min": 1,
      "max": 12,
      "value": 6,
      "default": 6
    },
    {
      "displayName": "Pipette volume",
      "variableName": "volume",
      "description": "How many microliters to pipette of each sample.",
      "type": "float",
      "choices": [
        {
          "displayName": "Low Volume",
          "value": 10.23
        },
        {
          "displayName": "Medium Volume",
          "value": 20.1
        },
        {
          "displayName": "High Volume",
          "value": 50.5
        }
      ],
      "value": 20.1,
      "default": 20.1
    },
    {
      "displayName": "Dry Run",
      "variableName": "dry_run",
      "description": "Skip aspirate and dispense steps.",
      "type": "bool",
      "value": false,
      "default": false
    },
    {
      "displayName": "Pipette Name",
      "variableName": "pipette",
      "description": "What pipette to use during the protocol.",
      "type": "str",
      "choices": [
        {
          "displayName": "Single channel 50µL",
          "value": "flex_1channel_50"
        },
        {
          "displayName": "Eight Channel 50µL",
          "value": "flex_8channel_50"
        }
      ],
      "value": "flex_1channel_50",
      "default": "flex_1channel_50"
    }
  ],

Changelog

Added methods to get a list of all defined parameter as Pydantic model objects to send to client
Hooked up parameters to be reported in analysis
Use the values sent over via the protocols endpoint (and the runs in the future`) to set defined parameters

Review requests

Risk assessment

Low, no significant architecture changes and mostly adding real data to stubs.

codecov · 2024-03-26T17:14:35Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.19%. Comparing base (b82b030) to head (6b5eb3c).
Report is 3 commits behind head on edge.

Additional details and impacted files

@@            Coverage Diff             @@
##             edge   #14735      +/-   ##
==========================================
- Coverage   67.19%   67.19%   -0.01%     
==========================================
  Files        2495     2495              
  Lines       71520    71460      -60     
  Branches     9020     9039      +19     
==========================================
- Hits        48056    48015      -41     
+ Misses      21342    21312      -30     
- Partials     2122     2133      +11

Flag	Coverage Δ
g-code-testing	`92.43% <ø> (ø)`

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

Files	Coverage Δ
...i/src/opentrons/protocol_runner/legacy_wrappers.py	`100.00% <ø> (ø)`
...i/src/opentrons/protocol_runner/protocol_runner.py	`100.00% <ø> (ø)`
api/src/opentrons/protocols/execution/execute.py	`90.00% <ø> (-0.91%)`	⬇️
...rc/opentrons/protocols/execution/execute_python.py	`89.18% <ø> (-0.56%)`	⬇️
...server/robot_server/protocols/protocol_analyzer.py	`100.00% <ø> (ø)`
robot-server/robot_server/runs/engine_store.py	`95.91% <ø> (ø)`

... and 40 files with indirect coverage changes

…port_rtp_in_analysis

sanni-t

🙌 🎉 So exciting!! This looks really good!
Just have a few nitpicks

api/src/opentrons/protocol_runner/protocol_runner.py

api/src/opentrons/protocols/execution/execute.py

sanni-t · 2024-03-28T17:15:14Z

api/src/opentrons/protocols/parameters/validation.py

+    if isinstance(value, float) and parameter_type is int and value.is_integer():
+        validated_value = int(value)
+    else:
+        validated_value = value


if value = 123.4 and the parameter type is int then this will assign 123.4 as the validated value instead of raising an error.

Oh, is this value then going to go through validate_type and will then raise an error?
In that case, can we add this explanation in the docstring?

api/src/opentrons/protocols/parameters/validation.py

sanni-t · 2024-03-28T17:34:08Z

api/src/opentrons/protocols/parameters/validation.py

+    validated_value: Union[float, bool, str]
+    if isinstance(value, bool):
+        raise ParameterValueError("Cannot send a boolean value as an enum type")
+    elif isinstance(value, int):
+        validated_value = float(value)
+    else:
+        validated_value = value
+    return validated_value


Why is this necessary? I think pydantic will just accept the int without any issues. As for the boolean value, wouldn't that be covered by the conversion you do in as_protocol_engine_type?

I could have sworn I was getting mypy errors for this, but removing it has no issue. Guess it was the same memory I had of putting in the set_parameters method

api/tests/opentrons/protocols/parameters/test_validation.py

sanni-t

Thanks for making the changes!

…s in analysis (#14735) Runtime parameter values sent via protocols endpoint will now be set, and all parameters and values used are now reported in analysis.

jbleon95 added 7 commits March 25, 2024 10:22

method for setting parameter values

bd066ef

convert parameters to PE types and fix tests

edcecb0

refactor protocol runner to own parameter context

4e9471d

include parameters in RunResult

852ed1e

report parameters in analysis response

7c277cc

add missing docstring

069ee15

unit tests for new validation functions

5e8569f

jbleon95 added 2 commits March 27, 2024 10:51

unit tests for parameter definition model functions

4c2ceff

unit tests for parameter context

7314619

jbleon95 marked this pull request as ready for review March 27, 2024 18:28

jbleon95 requested a review from a team as a code owner March 27, 2024 18:28

jbleon95 requested review from a team and sanni-t March 27, 2024 18:42

jbleon95 added 3 commits March 27, 2024 16:00

add test for raising when parameter does not exist

7634e42

Merge branch 'edge' of https://github.com/Opentrons/opentrons into re…

76bf19f

…port_rtp_in_analysis

actually set the parameters like the PR says it does

09511b6

sanni-t approved these changes Mar 28, 2024

View reviewed changes

jbleon95 added 2 commits March 28, 2024 14:35

changes per code review

2bd7c0d

docstring update I forgot

6b5eb3c

sanni-t approved these changes Mar 28, 2024

View reviewed changes

jbleon95 merged commit 691e372 into edge Mar 28, 2024
22 checks passed

jbleon95 deleted the report_rtp_in_analysis branch March 28, 2024 19:16

sanni-t mentioned this pull request Apr 1, 2024

fix(robot-server): maintain correct order of protocol analyses #14762

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(api, robot-server): set sent runtime values and report parameters in analysis #14735

feat(api, robot-server): set sent runtime values and report parameters in analysis #14735

jbleon95 commented Mar 26, 2024 •

edited

Loading

codecov bot commented Mar 26, 2024 •

edited

Loading

sanni-t left a comment

sanni-t Mar 28, 2024

sanni-t Mar 28, 2024

sanni-t Mar 28, 2024

jbleon95 Mar 28, 2024

sanni-t left a comment

feat(api, robot-server): set sent runtime values and report parameters in analysis #14735

feat(api, robot-server): set sent runtime values and report parameters in analysis #14735

Conversation

jbleon95 commented Mar 26, 2024 • edited Loading

Overview

Test Plan

Changelog

Review requests

Risk assessment

codecov bot commented Mar 26, 2024 • edited Loading

Codecov Report

sanni-t left a comment

Choose a reason for hiding this comment

sanni-t Mar 28, 2024

Choose a reason for hiding this comment

sanni-t Mar 28, 2024

Choose a reason for hiding this comment

sanni-t Mar 28, 2024

Choose a reason for hiding this comment

jbleon95 Mar 28, 2024

Choose a reason for hiding this comment

sanni-t left a comment

Choose a reason for hiding this comment

jbleon95 commented Mar 26, 2024 •

edited

Loading

codecov bot commented Mar 26, 2024 •

edited

Loading