Pipette model offsets #838

andySigler · 2018-02-14T23:05:27Z

overview

Fixes #817

This PR adds the property model_offset to pipette config, which is used to describe the dimensional differences between pipette. For example, a dimensional difference is that the P10_Single pipette is longer than the P300_Single

changelog

This PR refactors config.instrument_offset to store just the delta between what was expected vs what was found from tip-probe. Previously, config.instrument_offset described the entire offset of that instrument, including any offset from the pipette model's dimensions, so it could not be re-used between different models.

This PR also adds a version property to robot_config, to allow migrating from old configurations version to new ones.

Lastly, something that should be changed later on, is that the tip-length per pipette is saved in the robot config file. For example, if was using a P10_Single pipette, the generated config file would contain a tip_length key that looks like:

{
    "tip_length": {
        "p10_single": 40
    }
}

Later on, this tip_length should be a property of a tip-rack, not the pipette. This way, multiple pipettes can share the same tips without requiring a new tip-probe. Also, this way a single pipette can use multiple tips during a protocol run.

…del-offset or mount

… calibration data

codecov · 2018-02-14T23:35:21Z

Codecov Report

Merging #838 into edge will increase coverage by 0.15%.
The diff coverage is 98.27%.

@@            Coverage Diff             @@
##             edge     #838      +/-   ##
==========================================
+ Coverage   69.26%   69.42%   +0.15%     
==========================================
  Files         233      233              
  Lines        7181     7221      +40     
  Branches      312      312              
==========================================
+ Hits         4974     5013      +39     
- Misses       2118     2119       +1     
  Partials       89       89

Impacted Files	Coverage Δ
api/opentrons/robot/robot_configs.py	`100% <100%> (ø)`	⬆️
api/opentrons/util/calibration_functions.py	`98.73% <100%> (ø)`	⬆️
api/opentrons/robot/robot.py	`87.07% <100%> (+0.18%)`	⬆️
api/opentrons/instruments/pipette_config.py	`100% <100%> (ø)`	⬆️
api/opentrons/instruments/pipette.py	`96.53% <100%> (+0.01%)`	⬆️
api/opentrons/deck_calibration/dc_main.py	`34% <50%> (ø)`	⬆️
api/opentrons/config/config.py	`94.73% <0%> (-5.27%)`	⬇️

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 148d316...8a22960. Read the comment docs.

…ffset

btmorr · 2018-02-27T21:21:47Z

api/opentrons/instruments/pipette.py

@@ -181,6 +186,11 @@ def __init__(
 self.set_flow_rate(
 aspirate=aspirate_flow_rate, dispense=dispense_flow_rate)

+ # TODO (andy): remove from pipette, move to tip-rack
+ self.robot.config.tip_length[self.name] = \
+ self.robot.config.tip_length.get(self.name, tip_length)


Using the initialization parameter as the fallback here makes it such that if the tip length has previously been set in the config, whatever is specified in initialization would not be honored. I think the logic here should be reversed: if not tip_length, check config

overwriting the length specified in initialization is intentional here. Pipettes are initialized before every protocol run, each with a default tip-length depending on the model. If the tip has already been probed, then we do not want to use that default length. Solving this confusion should probably be done in another PR by saving all tip-length measurements to the associated tip-rack labware

If the purpose of this parameter to serve as a fallback length perhaps renaming the parameter to fallback_tip_length or something similar would lessen the confusion until we are able to move tip length to labware definitions.

This strikes me as related to the problems we had when there was no split between a labware definition and labware calibration

btmorr · 2018-02-27T21:22:32Z

api/opentrons/instruments/pipette.py

+ # TODO (andy): remove from pipette, move to tip-rack
+ self.robot.config.tip_length[self.name] = \
+ self.robot.config.tip_length.get(self.name, tip_length)
+ robot_configs.save(self.robot.config)


I don't think we should save here. In general, I'm not actually super confident in the logic of save, and I think using it more often adds risk around bad override values being stored

mcous

Requesting changes for the datetime thing because lack of consistency in this area has caused problems before

mcous · 2018-03-05T19:54:08Z

api/opentrons/robot/robot_configs.py

+
+ if version in migration_functions:
+ # backup the loaded configuration json file
+ tag = datetime.now().isoformat().split('.')[0].replace(':', '-')


I'm not really sure how this logic is used, or if it's even part of this PR vs a copy-paste thing, but any datetimes should be formatted in ms since epoch (see #365 for context and discussion). Additionally, datetime.now().isoformat() is in local timezone, not UTC (not a problem with ms since epoch)

mcous · 2018-03-05T20:04:27Z

api/opentrons/robot/robot_configs.py

+ if version in migration_functions:
+ # backup the loaded configuration json file
+ tag = datetime.now().isoformat().split('.')[0].replace(':', '-')
+ tag += '-v{}'.format(version)


Since I'm already commenting on this stuff: this tag construction may be easier to understand as a single format call:

tag = '{}-v{}`.format( int(time.time() * 1000), version)

mcous · 2018-03-05T20:09:39Z

api/opentrons/robot/robot_configs.py

+ migrate_func = migration_functions[version]
+ config_json = migrate_func(config_json)
+ # recursively update the config
+ # until there are no more migration methods for it's version


it's vs its typo 😛

mcous · 2018-03-05T20:14:11Z

api/opentrons/instruments/pipette.py

@@ -181,6 +186,11 @@ def __init__(
 self.set_flow_rate(
 aspirate=aspirate_flow_rate, dispense=dispense_flow_rate)

+ # TODO (andy): remove from pipette, move to tip-rack
+ self.robot.config.tip_length[self.name] = \
+ self.robot.config.tip_length.get(self.name, tip_length)


If the purpose of this parameter to serve as a fallback length perhaps renaming the parameter to fallback_tip_length or something similar would lessen the confusion until we are able to move tip length to labware definitions.

This strikes me as related to the problems we had when there was no split between a labware definition and labware calibration

…del-offset or mount

… calibration data

…ffset

Fixed by Andy

Laura-Danielle

Works with the following protocol:

from opentrons import containers, instruments, robot

rack1 = containers.load('tiprack-200ul', '3')
rack3 = containers.load('tiprack-200ul', '6')

plate = containers.load('96-flat', '1')


p300_single = instruments.P300_Single(
   mount='right',
   tip_racks=[rack1]
   )

p300_single_2 = instruments.P300_Single(
   mount='left',
   tip_racks=[rack1]
   )


p300_single.pick_up_tip()
p300_single.aspirate(30, plate.wells('A1'))
p300_single.dispense(30, plate.wells('B1'))
p300_single.move_to(plate.wells('B5'))
p300_single.drop_tip()

p300_single_2.pick_up_tip(rack1.wells('B1'))
p300_single_2.aspirate(5, plate.wells('A1'))
p300_single_2.dispense(5, plate.wells('A2'))
p300_single_2.move_to(plate.wells('A6')) 
p300_single_2.drop_tip()

Without calibrating the second model to the labware.

andySigler added 6 commits February 10, 2018 16:35

applies mount X offsets to mount Movers instead of each Pipette

03eb158

instrument offsets are deltas independant of pipette model

9da063f

Merge branch 'v3a' into api-pipette-model-offsets

4e5a6d5

Merge branch 'v3a' into api-pipette-model-offsets

9f2efd7

tip-probe update pipette location using found delta, not involving mo…

35c3d4f

…del-offset or mount

brings back single/multi offsets so still works with previously saved…

caf95c6

… calibration data

andySigler requested a review from btmorr February 14, 2018 23:05

andySigler added api Affects the `api` project WIP labels Feb 14, 2018

andySigler added 4 commits February 15, 2018 11:50

fixes merge conflicts

108f723

Merge branch 'v3a' into api-pipette-model-offsets

92e1288

adds version number to robot config, in order to migrate instrument_o…

719e8df

…ffset

Merge branch 'v3a' into api-pipette-model-offsets

4ed7f4a

andySigler force-pushed the api-pipette-model-offsets branch from 5535264 to 31418c9 Compare February 19, 2018 16:05

andySigler added ready for review and removed WIP labels Feb 19, 2018

backs up previous robot config when adding version number

48d8c6e

andySigler force-pushed the api-pipette-model-offsets branch from 31418c9 to 48d8c6e Compare February 19, 2018 18:20

migrate config json using dict of methods using config version as key

9ee9d82

andySigler force-pushed the api-pipette-model-offsets branch from 520d8f5 to 9ee9d82 Compare February 19, 2018 19:34

andySigler added 3 commits February 19, 2018 16:08

removes colons from config backup filename

c6f1685

Merge branch 'v3a' into api-pipette-model-offsets

447c93c

saves tip_length in config with key as pipette.name

1bef385

andySigler force-pushed the api-pipette-model-offsets branch from 13df478 to 1bef385 Compare February 21, 2018 00:01

Merge branch 'v3a' into api-pipette-model-offsets

6bb6a46

andySigler force-pushed the api-pipette-model-offsets branch 4 times, most recently from 1958f72 to dfeec9b Compare February 21, 2018 22:32

corrects miscalculated multi-channel offset

0a1dcb9

andySigler force-pushed the api-pipette-model-offsets branch from dfeec9b to 0a1dcb9 Compare February 21, 2018 22:40

Laura-Danielle self-requested a review February 23, 2018 22:08

andySigler mentioned this pull request Feb 27, 2018

P10 Tip Probe Height #927

Closed

btmorr previously requested changes Feb 27, 2018

View reviewed changes

btmorr changed the base branch from v3a to edge February 27, 2018 21:32

fixes merge conflicts; removes config.save() from pipette constructor

3c9dab6

mcous previously requested changes Mar 5, 2018

View reviewed changes

andySigler added 9 commits March 5, 2018 16:14

applies mount X offsets to mount Movers instead of each Pipette

c811593

instrument offsets are deltas independant of pipette model

0f06d11

tip-probe update pipette location using found delta, not involving mo…

c4ab8cf

…del-offset or mount

brings back single/multi offsets so still works with previously saved…

8e4baa3

… calibration data

adds version number to robot config, in order to migrate instrument_o…

cbfa4fc

…ffset

backs up previous robot config when adding version number

382d968

migrate config json using dict of methods using config version as key

ad71899

removes colons from config backup filename

10f61b1

saves tip_length in config with key as pipette.name

b3e5022

Laura-Danielle force-pushed the api-pipette-model-offsets branch from 3c9dab6 to b3e5022 Compare March 5, 2018 21:29

andySigler added 4 commits March 5, 2018 17:08

fixes merge conflicts

3b689eb

renames tip_length to fallback_tip_length

6bd378f

changes timestamps to millis since epoch

0d20f65

Merge branch 'edge' into api-pipette-model-offsets

8262cac

Laura-Danielle approved these changes Mar 7, 2018

View reviewed changes

andySigler force-pushed the api-pipette-model-offsets branch from c2d0220 to ae113f1 Compare March 7, 2018 15:26

_well_to_json() uses coordinates relative to parent container

8a22960

andySigler force-pushed the api-pipette-model-offsets branch from ae113f1 to 8a22960 Compare March 7, 2018 15:28

andySigler merged commit f9b9c2a into edge Mar 7, 2018

andySigler deleted the api-pipette-model-offsets branch March 7, 2018 16:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pipette model offsets #838

Pipette model offsets #838

andySigler commented Feb 14, 2018 •

edited

Loading

codecov bot commented Feb 14, 2018 •

edited

Loading

btmorr Feb 27, 2018

andySigler Mar 5, 2018

mcous Mar 5, 2018

btmorr Feb 27, 2018

mcous left a comment

mcous Mar 5, 2018

mcous Mar 5, 2018

mcous Mar 5, 2018

mcous Mar 5, 2018

Laura-Danielle left a comment

Pipette model offsets #838

Pipette model offsets #838

Conversation

andySigler commented Feb 14, 2018 • edited Loading

overview

changelog

codecov bot commented Feb 14, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcous left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Laura-Danielle left a comment

Choose a reason for hiding this comment

andySigler commented Feb 14, 2018 •

edited

Loading

codecov bot commented Feb 14, 2018 •

edited

Loading