ResourceStack, Plate, Lid inheritance logic fix
fix: resource stack handling for Lid in LiquidHandler and PlateHolder.
Specifically, Lid will now be assigned to parent Plate if top item of ResourceStack is Plate. Change to carrier.py ensures that ResourceStack is inferred correctly (up to two levels of parent above, since Lid may be child of plate, or directly a child of ResourceStack).
Future stability improvements: Geometric Plate comptability check before assigining as child. If the plate will not slot along skirting, it should not register as a child.
Thank you for the awesome work!
I'm not sure what you mean by this:
Future stability improvements: Geometric Plate comptability check before assigining as child. If the plate will not slot along skirting, it should not register as a child.
@ericguan04, could you please elaborate? :)
Thank you for the awesome work!
I'm not sure what you mean by this:
Future stability improvements: Geometric Plate comptability check before assigining as child. If the plate will not slot along skirting, it should not register as a child.
@ericguan04, could you please elaborate? :)
Sure thing! My supervisor and I was just concerned that there is no check for incompatible plates+lids after implementing the logic above (automatically attaching a lid child to a plate if the plate is on the top level of the stack). On that note, we also thought about adding an argument that let's the user choose whether to automatically add the lid as a child to the plate on the top of the stack, since perhaps there may be a time where the user won't want the lid to be capped on. I hope that clarifies things!
incompatible plates+lids
great point! please PR in the future if you have time!
incompatible plates+lids
great point! please PR in the future if you have time!
Depending on what you want PLR to do here, this might actually be a very difficult task:
If this is really needed, let's start with a definition set of what "incompatibility between plates and lids" means precisely in this context?
incompatible plates+lids
great point! please PR in the future if you have time!
Depending on what you want PLR to do here, this might actually be a very difficult task:
If this is really needed, let's start with a definition set of what "incompatibility between plates and lids" means precisely in this context?
You're right, this isn't a small task. What we mean by incompatibility is an edge case where the lid should nest but doesn't.
Our current code, specifically the get_lid_location function, assumes lids will nest inside the plate:
if isinstance(destination, Plate):
plate_location = destination.get_absolute_location()
child_wrt_parent = destination.get_lid_location(
lid.rotated(z=resource_rotation_wrt_destination_wrt_local)
).rotated(destination.get_absolute_rotation())
to_location = plate_location + child_wrt_parent
And this calls:
def get_lid_location(self, lid: Lid) -> Coordinate:
"""Get location of the lid when assigned to the plate. Takes into account sinking and rotation."""
return get_child_location(lid) + Coordinate(0, 0, self.get_size_z() - lid.nesting_z_height)
The issue arises when things like the lid's edge width or dimensions are slightly off. The result? The lid doesn't nest, and we end up with one of three outcomes:
'on_top': The lid just sits fully on top, completely above the plate. 'encompass': Much less likely, but the lid sits flush with the plate top, without adjusting for the nesting height. Catastrophic tilting: The worst case, where the lid sits partially on top and is unstable.
Paths to Resolve This
I see a few ways we could tackle this:
- Documentation: The simplest path. We just document that users are responsible for making sure their lids and plates are compatible and will stack correctly. We may also want to reference this issue so and more thoroughly address it only once it is actually brought up by more users.
- Override Argument: We could add an argument, something like move_lid(independent: bool), which would treat the lid as its own object in the stack, so its Z-height registers properly. We could also add independent_behavior: Literal['on_top', 'encompass'] for more specific control.
- Class-Based Check: A check like
lid.__class_.__name__ in plate.__class__.__name__to ensure the lid belongs to the plate class. While this is fairly simple, I think we would want to support actions like temporarily moving a lid onto a potentially different class of plate, so this may not be the most desirable. - Geometric Inference: This would be pretty complex to implement and thoroughly test, especially for an edge case, but it would be the most robust and general solution in the long run.
My Recommendation
Given the potentially undesirable behavior with option 3 and the complexity of implementing option 4, I recommend Option 1 (Documentation) for now, and maybe Option 2 (Override Argument) as a good next step.
What are your thoughts?
anything i can do to help move this forward?
anything i can do to help move this forward?
@rickwierenga just added some additional documentation to the ResourceStack notebook. it is in a new pull request because I tried switching branches (accidently made this PR from my local main) but now the branches and PRs are out of sync - apologies for that. See PR #583
Documentation: The simplest path. We just document that users are responsible for making sure their lids and plates are compatible and will stack correctly. We may also want to reference this issue so and more thoroughly address it only once it is actually brought up by more users.
From @maraxen suggestions, I tried implementing solution 1 - please check the documentation I added. what i wrote is definitely a draft so do let me know what needs to be changed for consistency, clarity, etc.