search

lneuhaus / pyrpl

pyrpl turns your RedPitaya into a powerful DSP device, especially suitable as a lockbox in quantum optics experiments.
http://lneuhaus.github.io/pyrpl/
MIT License
139 stars 108 forks source link

Make Module.reads-function asynchronous #354

Open lneuhaus opened 5 years ago

lneuhaus commented 5 years ago

Or find another solution to not waste so much kernel time waiting for the data to arrive from the red pitaya.

lneuhaus commented 5 years ago

Issue #241 is the same as this one

SamuelDeleglise commented 5 years ago

I have been looking (drowning) into that today. And it looks like it is going to be very tough to do, at least with an eventloop approach. The reason is that all the communication to the redpitaya is hidden behind attributes such that the asynchronous code should be well isolated in the getter of the attributes (There is no way to have the coroutine architecture propagate beyond that level). And hence, we should have a way to wait for the setting of the attribute to be done (at least on one given redpitaya) until returning from the attribute getter. This is typically done with LOOP.run_until_complete(future). This is the way I do it in async_utils.wait().

Hence, the natural solution would be to call my function async_utils.wait() in the getter. I have been testing it and it works until you call the getter from within a coroutine (for instance the one that is used in scope._trace_async()). It looks like python is allergic to the nested event loop structure (https://bugs.python.org/issue22239).

This basically forces us to have only one big island of coroutines stacked with await calls all the way from the bottom to the top of the stack--> No async redpitaya communication hidden in attributes

Maybe another option would be to do the communication in separated threads (one per redpitaya) that would be waited by the ...

lneuhaus commented 5 years ago

I agree. However, what we could think of is a way to group communication with the redpitaya into "atomic" units to save communication time in certain cases. So something like this

with iq.atomic() as iq:
    iq.amplitude=0.4
    iq.frequency=30

would be sent as a single TCP packet. Do you think there is a need for this?

SamuelDeleglise commented 5 years ago

I see, this could probably improve the performance. For instance the scope has an update period of 40 ms, eventhough the read time for a curve is 9 ms. I would not do it before having a clear breakup of the time spent in every method though... have you already used profiling libraries?

lneuhaus commented 5 years ago

Nope not yet. But there is a unit test that just plots a sine with pyqtgraph that reaches not much higher frame rate, so I would not think that there is too much room for improvement either (unless plotting and dataprocessing can run on different CPUs in parallel, or if we rewrite the plotting functions for better performance e.g. by only plotting updates instead of replotting everything). The proposal would however help e.g. the NA by allowing updates to the amplitude and frequency to be written almost simultaneously. But again, I propose we leave this here as a low-priority, long-term feature request.