Skip to content
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

Draft
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

lennijusten
Copy link
Contributor

Checking in work here for the thermocycling module. Still in progress

Comment on lines 78 to 79
async def get_current_block_temperature(self):
raise NotImplementedError(f"Block temperature data not available for Opentrons")
Copy link
Member

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?

Copy link
Contributor Author

@lennijusten lennijusten Apr 23, 2024

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?

Copy link
Member

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?

@lennijusten
Copy link
Contributor Author

@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 lh.summary() yields:

Deck: 624.3mm x 565.2mm

+-----------------+-----------------+-----------------+
|                 |                 |                 |
| 10: Empty       | 11: Empty       | 12: trash_co... |
|                 |                 |                 |
+-----------------+-----------------+-----------------+
|                 |                 |                 |
|  7: pcr         |  8: Empty       |  9: Empty       |
|                 |                 |                 |
+-----------------+-----------------+-----------------+
|                 |                 |                 |
|  4: Empty       |  5: Empty       |  6: Empty       |
|                 |                 |                 |
+-----------------+-----------------+-----------------+
|                 |                 |                 |
|  1: Empty       |  2: Empty       |  3: Empty       |
|                 |                 |                 |
+-----------------+-----------------+-----------------+

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?

@rickwierenga
Copy link
Member

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.

@lennijusten
Copy link
Contributor Author

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.

@lennijusten
Copy link
Contributor Author

lennijusten commented Apr 30, 2024

@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:

  • Block Max Voume (high priority)
    • "The Thermocycler’s block temperature controller varies its behavior based on the amount of liquid in the wells of its labware. Accurately specifying the liquid volume allows the Thermocycler to more precisely control the temperature of the samples. You should set the block_max_volume parameter to the amount of liquid in the fullest well, measured in µL. If not specified, the Thermocycler will assume samples of 25 µL."
  • Hold Time
    • "You can optionally instruct the Thermocycler to hold its block temperature for a specific amount of time. You can specify hold_time_minutes, hold_time_seconds, or both (in which case they will be added together)."
    • This would be nice to add to the temperature controller as well.

It would also be good to discuss how to implement good cycling logic - the Opentons solution seems quite elegant to me.

@rickwierenga
Copy link
Member

Block Max Voume (high priority)

should be doable using PLR volume trackers

Hold Time

this is perhaps something that can be implemented in the frontend

@rickwierenga
Copy link
Member

rickwierenga commented Apr 30, 2024

It would also be good to discuss how to implement good cycling logic - the Opentons solution seems quite elegant to me.

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

@lennijusten
Copy link
Contributor Author

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 repetitions like tc_mod.execute_profile(steps=profile, repetitions=20, block_max_volume=32) (docs) where you can repeat a specific profile. Thoughts on adding this vs just having the user assemble complete profiles beforehand?

@rickwierenga rickwierenga marked this pull request as draft May 22, 2024 01:49
@BioCam
Copy link
Contributor

BioCam commented Jun 6, 2024

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 ThermocyclerBackend class should have a .set_ramp_rate method and a self.ramp_rate attribute.

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
is very nice to check broadly what the class should cover.

@lennijusten
Copy link
Contributor Author

Good flag @BioCam. I plan to resume work on this PR in a couple weeks and can look to integrate this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants