-
Notifications
You must be signed in to change notification settings - Fork 187
fix(shared-data): Sync dimensions and spacing to latest hardware values #18284
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
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Notes: - Run import_frusta_from_hw.py. - Manually fix up the output to reduce the diff. e.g. undo field reorderings, and undo changes like 0->0.0. - The following labware erroneously have a `PATTERN QTY` in the spreadsheet, which will try to create `xCount` and `yCount` here. Undo that. - corning_12_wellplate_6.9ml_flat - corning_384_wellplate_112ul_flat - nest_12_reservoir_15ml - nest_96_wellplate_2ml_deep - opentrons_96_wellplate_200ul_pcr_full_skirt - nest_1_reservoir_195ml has extra junk in the spreadsheet ("--> 0" where I think it should just be "0"), which the script chokes on. Just verify manually that it matches the spreadsheet. - The following tube racks have heterogeneous wells and are split up in the spreadsheet, which we can't handle automatically. Handle them by doing hacky one-off temporary modifications to the script. - opentrons_10_tuberack_nest_4x50ml_6x15ml_conical - opentrons_10_tuberack_falcon_4x50ml_6x15ml_conical - The script needed some help with opentrons_96_aluminumblock_generic_pcr_strip_200ul because it didn't have `innerLabwareGeometry` to begin with. - The script chokes on opentrons_24_tuberack_generic_0.75ml_snapcap_acrylic, but it has empty data in the sheet and it's a retired labware, so just leave it unmodified.
Delete any drafts that we didn't modify.
…umblock_nest_0.5ml_screwcap. Only the former is mentioned in the hardware spreadsheet, but the latter uses the same physical tubes and therefore ought to share the same geometry. Fortunately, we have tests to catch this.
- Change zDimension to 44.2, which comes directly from the hardware spreadsheet. - Change well bottom z to 2.45, which is chosen so that well.z + well.depth == labware.dimensions.zDimension. We trust well.depth and labware.dimensions.zDimension because they both come straight from the hardware spreadsheet.
- Change well depth to 39.23, which comes directly from the hardware spreadsheet. - Change well bottom z to 4.81 so that well.z + well.depth == labware.dimensions.zDimension. zDimension already has a value that comes directly from the hardware spreadsheet.
- Change well depth to 17.399, which comes directly from the hardware spreadsheet. - Change well bottom z to 2.621 so that well.depth + well.z == labware.dimensions.zDimension. zDimension already has a value that comes directly from the hardware spreadsheet.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## edge #18284 +/- ##
==========================================
+ Coverage 23.62% 28.22% +4.60%
==========================================
Files 3055 3102 +47
Lines 255734 235942 -19792
Branches 30114 23820 -6294
==========================================
+ Hits 60419 66602 +6183
+ Misses 195301 169318 -25983
- Partials 14 22 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…dsheet. Notes: - Run import_dimensions_from_hw.py. - Manually fix up the output to reduce the diff. e.g. undo changes like 0->0.0. - Opentrons tube rack labware like opentrons_24_tuberack_generic_2ml_screwcap are incomplete in the spreadsheet and couldn't be parsed by the script. They need separate manual attention.
The hardware spreadsheet doesn't have full information for these definitions--it only has the diameters and depths--so this needed some hacky temporary modifications to the script.
New depth and diameter numbers straight from the hardware spreadsheet. The script couldn't do this automatically because of the heterogeneous tubes. opentrons_10_tuberack_nest_4x50ml_6x15ml_conical would have the same scripting problem, but its numbers are already good.
These labware's latest versions are still unreleased, so we can overwrite those latest versions.
…ions for. These labware's latest versions are released and stable, so we have to leave those alone, and put our fixes in new versions.
Change the numbers, based on the drawing SINGLE WELL RESERVOIR SUPPORT DWG rev A1.0. xDimension 108->108.7: The old value matched the bottom of the well, but a well's xDimension needs to represent the top of the well. 108.7 matches the drawing and the number in innerLabwareGeometry. yDimension 72->70.7: I'm not sure where 72 came from, but 70.7 matches the drawing and the number in innerLabwareGeometry. This definition is modified in-place because this labware has not been sold yet.
x/y were swapped in innerLabwareGeometry. This definition file is modified in-place because it was added in this PR.
This labware is not mentioned in the hardware spreadsheet, but it shares its tubes with opentrons_24_tuberack_nest_0.5ml_screwcap, which is, so we can use its geometry as the source of truth. We can edit this definition in-place because it was added recently and hasn't been released yet.
No contents changed other than the version number.
The script didn't hit these labware because they don't appear in the hardware spreadsheet. But they do share their tubes with other labware that *do* appear in the spreadsheet, so we can use those labware's geometry as the source of truth.
5ed9f57
to
28e2baa
Compare
Closing for now to clean up the PR list, but we should still do this. |
Superseded by #19127. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Follow-up to #18266. Goes towards EXEC-1480.
Changelog
Update these properties of a bunch of standard labware:
wells
)innerLabwareGeometry
)Numbers come from the "XY Spacing" tab of the hardware spreadsheet, linked in the ticket. This PR covers all labware explicitly listed in that tab, plus any labware that share well geometry with them, like the same tubes in different tube racks.
This is mostly done by script, but with a lot of manual fixup. See the commit messages for details.
For easier reviewing, here are the commits where the numbers actually change:
Review requests
One way to review this is to go through all the commits linked above and compare their new numbers against the hardware spreadsheet.
Another way is to start with the hardware spreadsheet, look at the "ERROR" section, and make sure all of those labware have their errors compensated for in this PR.
You'll note that the script makes some corrections that are pretty pedantic, like 17.4 changing to 17.399. I'm keeping these because I believe our JSON files ought to exactly follow the artifacts from the hardware team with a minimum of
editorializing on our part. And if we intend to keep running scripts like this in the future, which I think we should, then the scripts would keep wanting to make these pedantic corrections, so we might as well accept them now and get them out of the way.
There's a bit of a problem here with the tube racks. Their well
depth
s are changing, and any time that happens, we have a choice for whether to keepwell.top()
at the same elevation orwell.bottom()
at the same elevation (or neither). For most labware, we don't have to guess, because the spreadsheet tells us what the height of the labware should be, so we can do Math. For the tube racks, the hardware spreadsheet doesn't have that data.Test Plan and Hands on Testing
Risk assessment
Medium.
The script's math critically relies on an assumption that the top of the labware ought to equal the top of the basic well geometry in
wells
, and that ought to equal the top of the detailed geometry ininnerLabwareGeometry
. This is generally true, but you can imagine nuances like non-volume-carrying upper well sections being represented differently between the three. The script would trample over those nuances.