search

HPInc / HP-Digital-Microfluidics

HP Digital Microfluidics Software Platform and Libraries
MIT License
3 stars 1 forks source link

HeaterState' object has no attribute 'ready #79

Open EvanKirshenbaum opened 7 months ago

EvanKirshenbaum commented 7 months ago

I was running joey.py and ran into this. It might be a bug:

> python tools/joey.py thermocycle -t 50C -d 10s
Setting tick to 100.0 ms
Clock Thread exited
Exception in thread Clock Thread:
Traceback (most recent call last):
  File "/usr/lib64/python3.10/threading.py", line 1009, in _bootstrap_inner
    self.run()
  File "/home/vernica/proj/thylacine/mpam/src/mpam/engine.py", line 538, in run
    new_queue: list[CT] = self._process_queue(queue, tag = "pre-tick")
  File "/home/vernica/proj/thylacine/mpam/src/mpam/engine.py", line 495, in _process_queue
    new_delay: Optional[Ticks] = fn()
  File "/home/vernica/proj/thylacine/mpam/src/mpam/processes.py", line 158, in <lambda>
    board.before_tick(lambda: next(iterator))
  File "/home/vernica/proj/thylacine/mpam/src/mpam/processes.py", line 128, in iterator
    finish = next(i)
  File "/home/vernica/proj/thylacine/mpam/src/mpam/thermocycle.py", line 363, in iterator
    while not these_heaters.ready or not those_heaters.ready:
AttributeError: 'HeaterState' object has no attribute 'ready'
Migrated from internal repository. Originally created by Rares Vernica on Mar 31, 2022 at 11:16 AM PDT.
EvanKirshenbaum commented 7 months ago

Interesting. I had never tested the code with a single temperature because, well, "thermocycle" implies cycling between temperatures. Since there's no drop in temperature those_heaters are never used and, importantly, those_heaters.change_to() (which sets ready) is never called. It does make sense, though, as it allows the same code to be used for cooking drops at a single temperature.

The easiest (and probably cleanest) way to fix this is to simply initialize ready to True by default. Then if change_to() isn't called, the loop will terminate as soon as these_heaters get back to ambient temperature. So, just say

class HeaterState:
    ...
    ready: bool = True

An alternative would be to change

        if len(downs) > 0:
            those_heaters.change_to(downs.popleft())

to

        if len(downs) > 0:
            those_heaters.change_to(downs.popleft())
        else
            those_heaters.change_to(None)

before the 

for phase in phases:
    ...

loop. I don't think I'd do that, though, because on a board like Joey, with the middle heater shared, it would be perfectly reasonable to do a normal thermocycle on one side of the board and a simple cook on the other, so the cooking "thermocycle" shouldn't mess with the middle (shared) heater.

Migrated from internal repository. Originally created by @EvanKirshenbaum on Mar 31, 2022 at 11:45 AM PDT.
EvanKirshenbaum commented 7 months ago

If it is not intended to be used with a single temperature we can add a check when parsing the parameters and refuse to run.

Migrated from internal repository. Originally created by Rares Vernica on Mar 31, 2022 at 12:06 PM PDT.
EvanKirshenbaum commented 7 months ago

Nah. I meant to add that as an alternative in my last comment, but as I mentioned, simple heating at a single temperature is something people want to do, and the logic for moving into a zone, moving around within the zone, and moving out of the zone is identical, so we might as well do it in one place. (Perhaps the name should be changed at some point.)

Migrated from internal repository. Originally created by @EvanKirshenbaum on Mar 31, 2022 at 12:39 PM PDT.