HPInc / HP-Digital-Microfluidics

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

Redo GUI not in matplotlib #52

Open EvanKirshenbaum opened 7 months ago

EvanKirshenbaum commented 7 months ago

Just to get it on paper, I've been looking at seeing if there's a better layout/interaction library than matplotlib (which really wasn't designed for this). Some candidates:

Originally posted by @EvanKirshenbaum in https://github.com/HPInc/HP-Digital-Microfluidics/issues/50 [comment by @EvanKirshenbaum on Aug 13, 2021 at 3:48 PM PDT]

Migrated from internal repository. Originally created by @EvanKirshenbaum on Aug 16, 2021 at 10:24 AM PDT.
EvanKirshenbaum commented 7 months ago

As I've been trying to add controls to the monitor GUI (#50), it has become increasingly clear that matplotlib isn't actually designed for this sort of thing. (In its defense, it was designed for plotting data and allowing minimal interaction with data.) To do decent layout and obtain decent results, I should really switch to a package that's designed for building general-purpose UIs.

I've mentioned two of them above. There are probably others. I have no idea what the learning curves are for them or how well they'll handle the tasks of drawing and updating the actual board (which has turned out surprisingly well).

I'm going to try to force myself not to worry about this for a while. What I have does work, even if it's ugly, and the functionality is the important thing (except when taking videos).

Migrated from internal repository. Originally created by @EvanKirshenbaum on Aug 16, 2021 at 11:59 AM PDT.
EvanKirshenbaum commented 7 months ago

On a cursory investigation, it looks as though tkinter is the standard, and is included in the distribution, so that may be the way to go. Some resources

EvanKirshenbaum commented 7 months ago

I'm trying to identify the cause of high CPU usage. Creating a simple plot in matplotlib results in minimal CPU usage. I assume matplotlib is used in a specific/special mode to allow for user interaction. What are the key parts in the mpam that turn matplotlib from a UI that displays a static plot to the interactive/animated UI from mpam?

Migrated from internal repository. Originally created by Rares Vernica on Mar 03, 2022 at 12:35 PM PST.
EvanKirshenbaum commented 7 months ago

matplotlib backends are discussed here

  1. Does mpam require or is designed for a specific backend?
  2. What backend is used in Windows? Use matplotlib.get_backend() to display the backend in use.
    Migrated from internal repository. Originally created by Rares Vernica on Mar 03, 2022 at 12:49 PM PST.
EvanKirshenbaum commented 7 months ago

Any clues on what makes the matplotlib window to always steal focus?

Migrated from internal repository. Originally created by Rares Vernica on Mar 03, 2022 at 12:55 PM PST.
EvanKirshenbaum commented 7 months ago

I tired four different backends in Linux tkagg, tkcairo, qtagg, and qtcairo. In all of them I paused the UI. While the UI was paused the python process used close to 100% of CPU.

In parallel, I tried a animation example from the matplotlib manual. This is a continuous animation. With all three backends the CPU usage is in the 20-30% range while the animation was continuously running.

Migrated from internal repository. Originally created by Rares Vernica on Mar 03, 2022 at 1:55 PM PST.
EvanKirshenbaum commented 7 months ago

Okay, let's see. (Nearly) everything that deals with matplotlib is in src/monitor.py. (The exceptions are a couple of places that just use it for type checking.)

The back end used on my machine is TkAgg. I don't set it explicitly, so it must be the default here.

I have no idea what's causing it to steal focus. I don't think I do anything special. The basic board setup is all in BoardMonitor.__init__, with the __init__s in PadMonitor, WellPadMonitor, and WellMonitor adding some shapes.

As to what makes it animated and interactive, basically there are two things:

Migrated from internal repository. Originally created by @EvanKirshenbaum on Mar 03, 2022 at 2:01 PM PST.
EvanKirshenbaum commented 7 months ago

BoardMonitor.keep_alive is called from System.run_monitored in device.py. It is defined as

    def run_monitored(self, fn: Callable[[System],T],
                      *, 
                      min_time: Time = 0*sec, 
                      max_time: Optional[Time] = None, 
                      update_interval: Time = 20*ms,
                      control_setup: Optional[Callable[[BoardMonitor, SubplotSpec], Any]] = None,
                      control_fraction: Optional[float] = None,
                      macro_file_name: Optional[str] = None,
                      thread_name: Optional[str] = None,
                      ) -> T:
        from mpam.monitor import BoardMonitor  # @Reimport
        val: T

        done = Event()
        def run_it() -> None:
            nonlocal val
            with self:
                val = fn(self)  # @UnusedVariable
            done.set()
            self.shutdown()

        if thread_name is None:
            thread_name = f"Monitored task @ {time_now()}"
        thread = Thread(target=run_it, name=thread_name)
        monitor = BoardMonitor(self.board,
                               control_setup=control_setup,
                               control_fraction=control_fraction,
                               macro_file_name=macro_file_name)
        self.monitor = monitor
        thread.start()
        monitor.keep_alive(sentinel = lambda : done.is_set(),
                           min_time = min_time,
                           max_time = max_time,
                           update_interval = update_interval)

        return val

Note that fn (the actual user-provided protocol to run) is run (in the local run_it()) in its own thread and the idle loop for the display is kept in the main thread. (IIRC, Tk was unhappy running in any other thread.)

You might want to try taking out code from keep_alive and seeing if you can find anything that makes it stop slowing down.

Migrated from internal repository. Originally created by @EvanKirshenbaum on Mar 03, 2022 at 2:09 PM PST.
EvanKirshenbaum commented 7 months ago

Notes on why the UI windows keeps coming into focus. In Windows, minimizing the window seems to keep it out of focus. In Linux, minimizing the window does not help as it keeps restoring itself :-/

  1. https://stackoverflow.com/a/44352761/418730
  2. https://stackoverflow.com/a/45734500/418730

Overall, it seems the culprit is pyplot.pause which in turn calls pyplot.show which brings the window into focus. The recommendation is to avoid using pause or to implement our own pause.

Migrated from internal repository. Originally created by Rares Vernica on Mar 10, 2022 at 10:11 AM PST.
EvanKirshenbaum commented 7 months ago

Wondering if there is a simpler way of "redoing" the UI. Since matplotlib has a tkinter backend, maybe we could just display the matplotlib part in the tkinter canvas and write the rest in tkinter instead of rewriting everything.

Migrated from internal repository. Originally created by Mark Huber on Jun 16, 2022 at 2:48 PM PDT.
EvanKirshenbaum commented 7 months ago

You mean let matplotlib draw the (updating) board image in the tkinter canvas and use lower-level widgets for the buttons and text boxes? Might be worth a shot. We will still likely want to reimplement BoardMonitor.keep_alive(), which I suspect is where a lot of our performance problems are coming from. (In particular, the call to pyplot.pause().)

Even if we do this, there are still a few things that we might want to drop down for. In particular, when we draw the reagent names under the well circles, it would be really nice if we could add some line wrapping, which matplotlib doesn't seem to do easily. And I'd like to be able to get hover text (ideally in a box down below rather than obscuring the board) when you pause the mouse over a drop (or pad) to get a description of the state and content.

Migrated from internal repository. Originally created by @EvanKirshenbaum on Jun 16, 2022 at 2:56 PM PDT.
EvanKirshenbaum commented 7 months ago

You mean let matplotlib draw the (updating) board image in the tkinter canvas and use lower-level widgets for the buttons and text boxes?

That's what I meant. Found an example here where a matplotlib graph is plotted on a tkinter canvas

In particular, when we draw the reagent names under the well circles

That's where I got the idea, I was trying to draw the well texts and then realized there's no annotation method in tkinter as in matplotlib. I have to manually figure out where to draw it which is hard since we're shifting everything via .scale() and that method doesn't take the same units as the canvas. So I don't really know how to place that text correctly other than doing some calculations between our centimeter units and the scaling points.

Migrated from internal repository. Originally created by Mark Huber on Jun 16, 2022 at 3:03 PM PDT.
EvanKirshenbaum commented 7 months ago

Woooooowwww, it actually works. I can import the matplotlib stuff into the tkinter canvas and have extra buttons and input entries. The input entries are a bit laggy though. I guess that's due to threading. I checked around a bit and found this post talking about it. The solution seems to keep the GUI separated from the data acquisition thread. I believe this is being done in the keep_alive() method:

while not done():
     self.process_display_updates()
     self.figure.canvas.draw_idle()
     pyplot.pause(pause)

self.process_display_updates() does the changes, and self.figure.canvas.draw_idle() then draws it

Migrated from internal repository. Originally created by Mark Huber on Jun 17, 2022 at 12:38 PM PDT.
EvanKirshenbaum commented 7 months ago

I'm not sure about this, it seems like we will be still using a plotting library to move interactive circles move on the screen. The original goal was to move away from this. Remember that we want to reproduce the functionality, not necessarily make something that looks identical. We should be open to make changes on how it looks based on what is available in the new library. We shouldn't be looking for the simplest way to reproduce the UI, as we already have the old UI for that.

Migrated from internal repository. Originally created by Rares Vernica on Jun 17, 2022 at 1:41 PM PDT.
EvanKirshenbaum commented 7 months ago

true, but we could use the things that we like about the existing UI, like the board and the circles and create the things we don't like about it with tkinter (like the legend, the input entry, tooltips)?

Migrated from internal repository. Originally created by Mark Huber on Jun 17, 2022 at 1:54 PM PDT.
EvanKirshenbaum commented 7 months ago

I'm not sure about this, it seems like we will be still using a plotting library to move interactive circles move on the screen. The original goal was to move away from this. Remember that we want to reproduce the functionality, not necessarily make something that looks identical. We should be open to make changes on how it looks based on what is available in the new library. We shouldn't be looking for the simplest way to reproduce the UI, as we already have the old UI for that.

Long-term, I agree with you. Short-term, I'd like to start by addressing the main pain points that people are experiencing, which to my mind are

  1. The input text box is really hard to use due to (1) performance and (2) lack of word wrap/multi-line input
  2. The difficulty adding buttons and other controls to the display.
  3. The inability to add tooltips to get feedback on what is in drops and the status of pads on hover
  4. The inability to fold reagent names under well descriptions

If we can get these solved relatively quickly by leaving the main board image alone and working on the things that go around that, it will be a help, and then we can actually measure to see if the fact that the main monitor is written in matplot is the thing that's slowing us down and how we could go about replacing it.

Migrated from internal repository. Originally created by @EvanKirshenbaum on Jun 17, 2022 at 1:59 PM PDT.
EvanKirshenbaum commented 7 months ago

If we're going to change what the display looks like, I'd like to discuss an actual proposal before going too far down that path.

Migrated from internal repository. Originally created by @EvanKirshenbaum on Jun 17, 2022 at 2:00 PM PDT.
EvanKirshenbaum commented 7 months ago

Responding here to e-mail from Mark Huber to get it captured.

I made an interesting observation, the responsiveness of the input does get better when the interval speed is increased. It it’s set to 2 seconds it becomes pretty smooth unless something major is happening on the board such as drawing the circles for the wells. Or when a lot of drops are on the board it occasionally lags. This occasionally lagging doesn’t seem to go away when we increase the interval speed (I increased it to 10seconds and still saw it happening). It’s very minor though.

Sigh. This is why I have to remember to actually read the documentation.

What you just said was completely counterintuitive until I actually looked at the docstring for pause()​, which says "Run the GUI event loop for interval seconds".

It's not "pause the display updates so you can get work done", it's "pause the work to run the display".

I'm actually a bit surprised that other work gets done if you set it to a large value. I certainly wouldn't expect drops to move on the display, since the keep-alive loop

        while not done():
            self.process_display_updates()
            self.figure.canvas.draw_idle()
            pyplot.pause(pause)

only updates the figure during process_display_updates()​. (As an aside, we can probably get rid of the draw_idle()​ call, since it's called from within pause()​ if necessary

I'm a bit confused, as I seem to remember from back when I used Tk a few times (in Tcl and Perl), there was a way to say "Do this whenever you're idle", and that's where we want to hang process_display_updates. Ah, it looks as though something like this should work better

        interval = update_interval.as_number(ms)
        timer = self.figure.canvas.new_timer(interval=interval)
        timer.add_callback(self.process_display_updates)
        timer.start()
        run_for = (10*sec).as_number(sec)
        while not done():
            pyplot.pause(run_for)

This calls Tk's after() method under the covers to add (and keep re-adding) the idle task. (Yes, as near as I can tell, pause() wants a number in seconds and new_timer() wants a number in milliseconds.) In order to get the display to reflect what happens in process_display_updates(), you have to add a call to draw_idle() in process_display_updates():

    def process_display_updates(self) -> None:
        cbs = self.update_callbacks
        if cbs is not None:
            with self.lock:
                self.update_callbacks = None
            for cb in cbs:
                cb()
            if self.figure.stale:
                self.figure.canvas.draw_idle()

I don't think this quite solves the problem, but it might be a step in the right direction.

Migrated from internal repository. Originally created by @EvanKirshenbaum on Jun 22, 2022 at 12:28 PM PDT.