openUC2 / ImSwitch

ImSwitch is a software solution in Python that aims at generalizing microscope control by providing a solution for flexible control of multiple microscope modalities.
https://imswitch.readthedocs.io/
GNU General Public License v3.0
10 stars 16 forks source link

Processing SIM in background - not properly implemented #33

Closed beniroquai closed 11 months ago

beniroquai commented 1 year ago

Hey @kasasxav, I guess I'm getting confused over the use of the Worker-updater strategy in the SIMController again. We are collecting 9 images and want to calibrate the fourier peaks and then process them. I'm doing it wrong ;-) Would you mind helping me with this line: https://github.com/openUC2/ImSwitch/blob/SIM_PCO/imswitch/imcontrol/controller/controllers/SIMController.py#L309

I guess we somehow want to call this in the worker in the background, or?

Thank you !!! :)

kasasxav commented 1 year ago

Hi @beniroquai , could you tell me what is wrong with your approach? Is there some error showing up or is it working slow?

beniroquai commented 1 year ago

Hey @kasasxav , I think I'm still struggling with the idea of controller-worker. We want to collect 9 frames, i.e. update() sends images after frame/SLM synchronisation to the worker

self.imageComputationWorker.prepareForNewSIMStack(im)

and they are getting accumulated:

            if len(self.allFrames)>8:
                self.allFramesNP = np.array(self.allFrames)
                self.allFramesList = self.allFrames
                self.allFrames = []
                if isDEBUG:
                    date = datetime. now(). strftime("%Y_%m_%d-%I-%M-%S_%p")
                    tif.imsave(f"filename_{date}.tif", self.allFramesNP)

                self._numQueuedImagesMutex.lock()
                self._numQueuedImages += 1
                self._numQueuedImagesMutex.unlock()

                # FIXME: This is not how we should do it, but how can we tell the compute SimImage to process the images in background?!
                self.computeSIMImage()

The last line makes me suspicious self.computeSIMImage()

I'd assume that we should rather call this function somewhere else to not block the main thread (?). Right now, the camera stream and GUI freezes. Should I start yet another background thread?

The method calibrate() takes a moment to get computed..

kasasxav commented 1 year ago

Hi @beniroquai , I don't know what would be the best way to do it but I also suspect that update() might not be the best way. Because that method is done to update widgets with the current liveview frames (which are one every 100ms or so), while you want to take all the images from the camera. I am thinking if you could use the record function and save the raw data into a file, and have a thread in your controller that opens that file and does the processing? I am not sure what is the best way of doing this, is there any literature about someone that did something similar?

kasasxav commented 1 year ago

I know @jacopoabramo wants to do some live processing on data, and I think there might not be enough functionality in ImSwitch to do that, maybe you guys can think of something?

jacopoabramo commented 1 year ago

I have several thoughts on this, but right now I'm following a course and will be available after 3:00 pm (Berlin time). I will update with a following comment.

jacopoabramo commented 1 year ago

Hi @beniroquai , so I checked the code (I'm on a small break right now). It looks to me as if you're trying to a producer-consumer strategy. You should refine a couple of things: instead of using a list use a deque from the collections built-in library. It's thread-safe by definition so you don't need any funky mutex logic. If you want to trigger a computation in background whenever the buffer is full at 9 images you'll have to spawn another thread which does the computation and connect this thread to a signal which sends the resulted images in output somewhere to the viewer. In this way what's happening is that the frame that accumulates the frames also does the computation, while instead you have to delegate it to another operator. Am I correct or confusing?

beniroquai commented 1 year ago

@jacopoabramo this sounds totally legit to me! I can have a try. Do you have any working solution with the deque method that I could use for "inspiration" ;-) I can also just try to get my fingers dirty of course.

jacopoabramo commented 1 year ago

@beniroquai unfortunately no, but I can make a small snippet of code later on and post it here (I realized the course lasts to 5:00 pm instead of 3:00 pm. It did sound too good to be true...)

beniroquai commented 1 year ago

That would be great! Thanks! :)

Message ID: @.***>

jacopoabramo commented 1 year ago

Hi @beniroquai , so this is the snippet I prepared:

import numpy as np
import sys
from collections import deque
from qtpy.QtCore import QThread, QObject, Signal, QCoreApplication

class Producer(QThread):
    stackReady = Signal(np.ndarray)
    def __init__(self, maxlen: int) -> None:
        super().__init__()
        self.queue = deque(maxlen=maxlen)

    def run(self) -> None:
        while self.isRunning():
            data = np.random.randint(0, 100, (256, 256), dtype=np.uint16)
            self.queue.append(data)
            if len(self.queue) == self.queue.maxlen:
                self.stackReady.emit(np.stack(self.queue))
                self.queue.clear()

class Consumer(QObject):
    resultReady = Signal(np.float64)

    def __init__(self) -> None:
        super().__init__()

    def consume(self, input: np.ndarray) -> None:
        self.resultReady.emit(np.average(input))

class Printer(QObject):
    def __init__(self) -> None:
        super().__init__()

    def showData(self, input: np.float64):
        print(input)

app = QCoreApplication(sys.argv)

# infinite running thread;
# this is done by re-implementing the "run()" method
# the running condition can be also controlled with flags
producerThread = Producer(maxlen=10)

# classic QThread implementation
consumerWorker, consumerThread = Consumer(), QThread()
consumerWorker.moveToThread(consumerThread)

# normal QObject, does not need a separate thread
# since we associate this with the viewer
printerWorker = Printer()

# connect signals
producerThread.stackReady.connect(consumerWorker.consume)
consumerWorker.resultReady.connect(printerWorker.showData)

# start threads in the order Consumer -> Producer
consumerThread.start()
producerThread.start()

# quit and join threads in the opposite order: Producer -> Consumer
# this section here is commented because to make the example work one needs
# to use the QCoreApplication to generate the event loop. ImSwitch does this already
# in background. Whenever you quit the application you should call these methods too
# producerThread.quit()
# consumerThread.quit()

# press CTRL + Z to quit application, as it will run indefinetely
sys.exit(app.exec_())

In all honesty I believe that this only solves the issue in not using the mutexes, but actually since I encapsulated the queue within a single thread it doesn't even make sense to use a dequeue but a list will do just fine.

At any rate, this solution "should" work because you're just spawning another thread which collects the number of frames you need and spits a signal whenever the buffer is filled. In this way this is guaranteed to be in background. Basically in your producer thread instead of getting random data you just collect the images via getLatestFrame from your detector.

The reason I don't like it is: we're wasting an extra thread because the accumulation of the frames should be done directly by the detector manager (which is the true producer) and then you have the consumer thread which takes the buffer, makes the computation etc etc.

You could think of using the getChunk method from the detector and just implement the consumer thread... it's a bit of a mess, because at any rate you break pattern with the rest of the implementation. I have too many thoughts about this and I kinda get confused by overlapping them, I'm sorry if I'm not sounding clear enough.

beniroquai commented 1 year ago

Thanks @jacopoabramo, @ranranking and I had a labsession today and made it work. For now quick and dirty. I guess I have to get more familiar with the mutex structure. Do you have a good resource for reading into it? It's not a QT-only thing or?

jacopoabramo commented 1 year ago

@beniroquai sorry for the late reply.

No it's not, mutex is a software concept which is unrelated to a specific language and you can implement it any way you want. I guess you should dwelve more into how multithreading and multiprocessing works.

I usually end up in real-python for some explanations after googling the problem I have, i.e.: https://realpython.com/intro-to-python-threading/

beniroquai commented 11 months ago

Fixed the issue, right @ranranking