feat(api): support flow rate (uL/sec) in JSON protocols #2123

IanLondon · 2018-08-23T21:02:00Z

This PR is also serving as its own ticket.

overview

JSON protocol schema + JSON protocol executor (in Python API) + PD all updated to support setting flow rate for aspirate and dispense commands.

This overview is gonna get a little long, but please think it through, because we need a pattern of managing implicit/default values in protocols that permits "reproducibility by default, and flexibility by choice" for protocol creation, editing, and execution.

Background context: In an upcoming ticket (#1323), PD will allow user to specify "Use default flow rate" vs "Use flow rate of __ uL/sec", for each step.

For "use specified flow rate", it's a straightforward problem. This PR allows JSON protocols to include a flow-rate parameter in aspirate and dispense commands that sets the flow rate for that command only.

But what does "use default flow rate" mean in PD? Specifically, where do these values come from (and at what point in the revision history)?

A protocol that is working fine shouldn't suddenly & silently change its operation when we update the defaults. So this PR introduces the key "default-values" which has inner keys "aspirate-flow-rate" and "dispense-flow-rate" for each pipette model. When a user is saving a protocol, PD will include the flow rate defaults from shared-data/robot-data/pipette-config.json (whatever the values were when that version of PD was built). Other 3rd party protocol creators will be expected to do the same thing.

When a user goes to run a protocol, the JSON protocol executor will use these saved values for that protocol. With this PR, the JSON protocol executor will never use the API default values for flow-rate.

(Note: Default flow rate values used by the API also come from pipette-config.json. But they can mis-match -- the API can be built with a newer or older version of that file, than the version of the file built into the version of PD which the protocol was saved with. Whoof that is hard to put in a sentence)

In the future, by saving flow rate default values in the JSON protocol, we'll be able to do things like:

When opening a PD file in PD, we can easily detect if defaults are different in the current version and allow the user to save or maintain those defaults
When opening a JSON protocol (whether from PD or another protocol creator) in Run App, we can read the saved defaults, compare them to the Run App or the API's defaults, and give the user options for what to do

I think the "default-values" key will expand to encompass more of those implicit constant values that go into executing a protocol. But there is a limit to it -- otherwise we'll wind up having to detail every constant value in the Python API/server and save it in every protocol! We have to be choosy about what really matters to the protocol, and what can be left implicit and a pure responsibility of the API.

changelog

update JSON protocol schema in shared-data
add "default-values" field to PD saved files
support flow rate in JSON protocol executor

review requests

Make a new protocol with the version of PD off this branch
There's no way to add flow-rate in PD yet, so edit the saved JSON:

                {
                    "command": "dispense",
                    "params": {
                        "pipette": "pipette:p300_multi_v1.3:bd4a9b10-a639-11e8-90b4-372a00fca620",
                        "volume": 50,
                        "labware": "cad54000-a639-11e8-90b4-372a00fca620:trough-12row",
                        "well": "A1",
                        "flow-rate": 1200   <-------- ADD THIS
                    }
                }

You can also change the flow rates for specific pipettes under the "default-values" key. These values are used when the "flow-rate" param is unspecified.

How do you make sure that the flow rate is actually changed? Unfortunately I think the best way is to use a significantly fast (4x) or slow (1/4x) flow rate value and test with water on a robot... it's hard to even use print statements to see that it's really working in Pipette. But set_speed is a decent place to put a debugging print statement, keep in mind it will get called 2x once with aspirate and once with dispense, for each Pipette.set_flow_rate 😢

- update JSON protocol schema in shared-data - add "default-values" field to PD saved files - support flow rate in JSON protocol executor

IanLondon · 2018-08-23T21:10:20Z

shared-data/protocol-json-schema/protocol-schema.json

+      "description": "Special keyword specifying the model of the pipette. TODO finalize these model names they are still TBD",
+      "type": "string",
+      "enum": [
+        "p10_single_v1",


I would like these _v* version suffixes to go away in JSON protocols (I have an WIP PR that's been up for a while to do this), but if flow rate changes between pipette model versions, then we do indeed need the version suffix in the "default-values" -- but it's fine if it's not in the pipette itself. So the version suffix would only be relevant when applying default values, and that could be determined at runtime... wish there was a nicer way though

@IanLondon and @b-cooper does my comment I added in #1825 sum up what we discussed a while back to the best of y'all's knowledge?

Also (perhaps @andySigler could jump in here): what are the advantages / disadvantages to locking a default flow rate to a given pipette configuration rather than a specific model? It might be nice to say all p300_singles have the same default flow rate if we could get away with it

IanLondon · 2018-08-23T21:10:58Z

shared-data/protocol-json-schema/protocol-schema.json

@@ -163,18 +219,7 @@
              "enum": ["left", "right"]
            },
            "model": {
-              "description": "Special keyword specifying the model of the pipette. TODO finalize these model names they are still TBD",


Just moved to a "definition" above (and added v1.3 suffix set)

IanLondon · 2018-08-23T21:11:56Z

api/opentrons/protocols/__init__.py

+        raise ValueError(
+            'Command tried to use labware "{}", but that ID does not exist ' +
+            'in protocol\'s "labware" section'.format(labwareId))
+    return labware.wells(well)


Laura pointed out that Placeable.get is a confusing alias for Placeable.wells so I'm going with the more explicit here -- shouldn't change anything

codecov · 2018-08-23T21:14:12Z

Codecov Report

Merging #2123 into edge will increase coverage by 1.42%.
The diff coverage is 33.33%.

@@            Coverage Diff            @@
##             edge   #2123      +/-   ##
=========================================
+ Coverage   32.18%   33.6%   +1.42%     
=========================================
  Files         455     456       +1     
  Lines        7336    7698     +362     
=========================================
+ Hits         2361    2587     +226     
- Misses       4975    5111     +136

Impacted Files	Coverage Δ
protocol-designer/src/file-types.js	`0% <ø> (ø)`	⬆️
shared-data/js/pipettes.js	`0% <0%> (ø)`	⬆️
...ol-designer/src/file-data/selectors/fileCreator.js	`26.66% <100%> (+5.23%)`	⬆️
app-shell/src/config.js	`21.62% <0%> (-3.38%)`	⬇️
app/src/robot/api-client/client.js	`87.67% <0%> (-0.53%)`	⬇️
app/src/components/RunPanel/index.js	`0% <0%> (ø)`	⬆️
app/src/components/AppSettings/AppInfoCard.js	`0% <0%> (ø)`	⬆️
...c/components/RobotSettings/AdvancedSettingsCard.js	`0% <0%> (ø)`	⬆️
app/src/pages/Upload.js	`0% <0%> (ø)`	⬆️
...mponents/InstrumentSettings/AttachedModulesCard.js	`0% <0%> (ø)`	⬆️
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c242de9...2983cd1. Read the comment docs.

b-cooper

🥇

Kadee80

🍀

IanLondon added 2 commits August 22, 2018 18:05

feat(api): support flow rate (uL/sec) in JSON protocols

2d63347

- update JSON protocol schema in shared-data - add "default-values" field to PD saved files - support flow rate in JSON protocol executor

fix flow rate

2983cd1

IanLondon added api Affects the `api` project protocol designer Affects the `protocol-designer` project json protocol schema labels Aug 23, 2018

IanLondon commented Aug 23, 2018

View reviewed changes

IanLondon requested review from mcous, btmorr, b-cooper, Kadee80 and Laura-Danielle August 23, 2018 21:12

IanLondon added ready for review medium labels Aug 23, 2018

IanLondon self-assigned this Aug 23, 2018

b-cooper approved these changes Aug 29, 2018

View reviewed changes

Kadee80 approved these changes Aug 30, 2018

View reviewed changes

IanLondon merged commit b0f944e into edge Sep 3, 2018

IanLondon deleted the pd_api-support-flow-rate branch September 3, 2018 14:25

IanLondon mentioned this pull request Sep 11, 2018

PD Advanced Step Settings: Flow Rate #1323

Closed

IanLondon mentioned this pull request Dec 3, 2018

PD ignores flow rate settings #2773

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(api): support flow rate (uL/sec) in JSON protocols #2123

feat(api): support flow rate (uL/sec) in JSON protocols #2123

IanLondon commented Aug 23, 2018 •

edited

Loading

IanLondon Aug 23, 2018

mcous Sep 4, 2018

IanLondon Aug 23, 2018

IanLondon Aug 23, 2018

codecov bot commented Aug 23, 2018 •

edited

Loading

b-cooper left a comment

Kadee80 left a comment

feat(api): support flow rate (uL/sec) in JSON protocols #2123

feat(api): support flow rate (uL/sec) in JSON protocols #2123

Conversation

IanLondon commented Aug 23, 2018 • edited Loading

overview

changelog

review requests

IanLondon Aug 23, 2018

Choose a reason for hiding this comment

mcous Sep 4, 2018

Choose a reason for hiding this comment

IanLondon Aug 23, 2018

Choose a reason for hiding this comment

IanLondon Aug 23, 2018

Choose a reason for hiding this comment

codecov bot commented Aug 23, 2018 • edited Loading

Codecov Report

b-cooper left a comment

Choose a reason for hiding this comment

Kadee80 left a comment

Choose a reason for hiding this comment

IanLondon commented Aug 23, 2018 •

edited

Loading

codecov bot commented Aug 23, 2018 •

edited

Loading