-
Notifications
You must be signed in to change notification settings - Fork 59
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
Add thermocycler module #108
base: main
Are you sure you want to change the base?
Conversation
async def get_current_block_temperature(self): | ||
raise NotImplementedError(f"Block temperature data not available for Opentrons") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this not the get_current_temperature
from above? If not, what is get_current_temperature
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Opentrons API has both module["data"]["currentTemperature"]
and module["data"]["lidTemperature"]
but not module["data"]["blockTemperature"]
.
It could be that module["data"]["currentTemperature"] == module["data"]["blockTemperature"]
but that's not obvious from the docs. Possibly it's some average temp?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't know 🤷♂️
Perhaps include this info in a docstring to make future debugging easier?
7a6501f
to
f2d4752
Compare
@rickwierenga How do we want to handle slot assignments for the PCR module? In my test script I'm assigning it to slot 7, which works fine, but then
In reality, the module covers all of slot 7, all of slot 9, and some of slot 8 & 11 such that you couldn't put a tip rack on there (water beaker would be fine). Do we want to add some custom logic so that the thermocycler automatically takes up slot 7&10 or just leave it as is? |
how does OT do it in their official software? It seems that slide 10 needs to be blocked too, and probably 8 and 11 as well. I'm not sure how to best model this in PLR. The collision checker is easy, but how Opentrons handles this in the API will unfortunately have to impact our decision. We also have to make the summary look nice. |
Yea - I'm going to implement an MVP version of this first without deck modifications, but I agree that we need to include those changes. |
@rickwierenga It's working giving the Opentrons docs on the thermocycler a read. They have a few nice methods that we should discuss implementing, including:
It would also be good to discuss how to implement good cycling logic - the Opentons solution seems quite elegant to me. |
should be doable using PLR volume trackers
this is perhaps something that can be implemented in the frontend |
I like the explicit Python version in the docs over the list of dictionaries. A core assumption with PLR is that protocols are best expressed as Python code (because it's Turing complete and integrates well). We should definitely provide basic utilities for the basic PCR cycling steps, as a frontend Python method. I'm not a fan of expressing this in a list as much. This is very much a secondary and worse way to use it, and unnecessarily increases complexity of PLR. (PLR users can implement this easily themselves in their scripts, if they prefer) The "this code would generate 60 lines in the protocol’s run log" (from OT docs) seems an advantage, not a disadvantage, as logs are supposed to be explicit imo. +you get type checking, which you don't get with lists/dicts |
I added an implementation for running profiles, but have not tested it yet. @rickwierenga Could you review for a sanity check? Something else to consider is that the OT API also lets you define |
8d39ea7
to
d31f66e
Compare
57e7663
to
d9baaf1
Compare
Hi everyone, I am just reading through this PR and have to familiarize myself more with the PLR-integration of the OT-2. But, more generally speaking, I believe the agnostic Not all thermocyclers allow modifications of their ramp rate but many do, and it can be very beneficial for different assays to control this parameter, particularly for in vitro+ex vivo applications like DNA origami. This guide, TF: Thermal Cycler Features—Six Key Considerations |
Good flag @BioCam. I plan to resume work on this PR in a couple weeks and can look to integrate this. |
Checking in work here for the thermocycling module. Still in progress