-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Qt embedding example: Separate drawing and data retrieval timers #28589
8000 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
Conversation
d2dd466
to
e632e9e
Compare
Given that we are fully embedding in Qt in this example we should be trusting it to manage it's event queue and not "helping" (this seems very similar to trying to "help" the garbage collector which ended badly for us). It would be better to sort out if we are blocking the event loop someplace and fix that. The timer here in matplotlib/lib/matplotlib/backends/backend_qt.py Lines 182 to 208 in 0da7d89
matplotlib/lib/matplotlib/backend_bases.py Lines 2311 to 2334 in 0da7d89
matplotlib/lib/matplotlib/backends/backend_qt.py Lines 211 to 214 in 0da7d89
I suspect that a better fix is to change the |
Hmm indeed using matplotlib.drawing.methods.comparison.mp4And the code: #!/usr/bin/env python
import sys
import time
import numpy as np
from matplotlib.backends.backend_qtagg import FigureCanvas
from matplotlib.backends.backend_qtagg import \
NavigationToolbar2QT as NavigationToolbar
from matplotlib.backends.qt_compat import QtWidgets, QtCore
from matplotlib.figure import Figure
class ApplicationWindow(QtWidgets.QMainWindow):
def __init__(self, method):
super().__init__()
self._main = QtWidgets.QWidget()
self.setCentralWidget(self._main)
self.method = method
self.setWindowTitle(method)
layout = QtWidgets.QVBoxLayout(self._main)
static_canvas = FigureCanvas(Figure(figsize=(5, 3)))
# Ideally one would use self.addToolBar here, but it is slightly
# incompatible between PyQt6 and other bindings, so we just add the
# toolbar as a plain widget instead.
layout.addWidget(NavigationToolbar(static_canvas, self))
layout.addWidget(static_canvas)
self._static_ax = static_canvas.figure.subplots()
t = np.linspace(0, 10, 501)
self._static_ax.plot(t, np.tan(t), ".")
self.t = np.linspace(0, 10, 10001)
dynamic_canvas1 = FigureCanvas(Figure(figsize=(5, 3)))
layout.addWidget(dynamic_canvas1)
layout.addWidget(NavigationToolbar(dynamic_canvas1, self))
self._dynamic_ax1 = dynamic_canvas1.figure.subplots()
# Set up a Line2D.
self._line1, = self._dynamic_ax1.plot(t, np.sin(t + time.time()))
dynamic_canvas2 = FigureCanvas(Figure(figsize=(5, 3)))
layout.addWidget(dynamic_canvas2)
layout.addWidget(NavigationToolbar(dynamic_canvas2, self))
self._dynamic_ax2 = dynamic_canvas2.figure.subplots()
# Set up a Line2D.
self._line2, = self._dynamic_ax2.plot(t, np.sin(t + time.time()))
self._timer = QtCore.QTimer()
self._timer.timeout.connect(self._update_canvas)
self._timer.start(1)
def _update_canvas(self):
# Shift the sinusoid as a function of time.
self._line1.set_data(self.t, np.sin(self.t + time.time()))
self._line2.set_data(self.t, np.sin(self.t - time.time()))
if self.method == "draw":
self._line1.figure.canvas.draw()
self._line2.figure.canvas.draw()
elif self.method == "draw_idle":
self._line1.figure.canvas.draw_idle()
self._line2.figure.canvas.draw_idle()
elif self.method == "processEvents":
self._line1.figure.canvas.draw()
self._line2.figure.canvas.draw()
qapp.processEvents()
if __name__ == "__main__":
# Check whether there is already a running QApplication (e.g., if running
# from an IDE).
qapp = QtWidgets.QApplication.instance()
if not qapp:
qapp = QtWidgets.QApplication(sys.argv)
app = ApplicationWindow(sys.argv[1])
app.show()
app.activateWindow()
app.raise_()
qapp.exec() Note that this issue is significant (at least for me) only on Windows. |
One of my favorite jokes is: Both junior engineers and senior engineers take the same short-cuts, but the senior engineers know to feel guilty about it. You are asking the timer to run at at 1kHz (my understanding is My understanding of |
OK I got convinced that using Hence I modified the PR such that it uses two timers with different frequencies - the high frequency timer for updating the data and the slow (50Hz) timer for drawing the canvas. Also the comments now explain the topic much better for beginner Qt users. |
e632e9e
to
8b89c80
Compare
8b89c80
to
0f92e8c
Compare
self.drawingTimer.start() | ||
|
||
def _update_data(self): | ||
self.xdata = np.linspace(0, 10, 101) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To check my understanding, because these are being run by timmers, that means they are run atomically on the event loop so we do not need to worry about locking between this method and _update_canvas
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To check my understanding, because these are being run by timmers, that means they are run atomically on the event loop so we do not need to worry about locking between this method and
_update_canvas
?
I'm not sure what do you mean by locking, are you talking about potential issues of xdata not in sync with ydata or such?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm worried about the sequence of things where:
- you set
self.xdata
self._line.set_data(self.xdata, self.ydata)
runs- you set
self.ydata
In this particular example, if we were losing that race it would be OK because the xdata is always the same (important from a "correct plot" point of view) and the same length (important from a "the code will run without raising" point of view). If there are threads involved the above sequence is possible as with non-free threading Python (aka all released versions) the interpreter can drop the GIL after any bytecode evaluation and then any thread can be the next one to run (and in the future with free-threading builds of Python it is just the YOLO land).
However, if the calls to these methods are serialized by the event loop (it runs the one at a time in the order they were put on the queue) then we are safe because Qt is providing the level of locking we need.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK I see what you mean. Currently the dataTimer
starts (with .start()
) before the drawingTimer
starts, and also the timeout of the 1st timer for sure is reached before the timeout of the 2nd timer (due to the different intervals), so I am pretty sure we can count on that.
Not only that, even if both timers had the same interval, the drawingTimer
's callback that uses self._line.set_data(self.xdata, self.ydata)
always has a self.{x,y}data
attributes available, because I ran manually the self._update_data
method before I create the line with self._dynamic_ax.plot
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider the case where :
import random
self.xdata = np.linspace(0, 10, random.randint(100, 150))
Which callback run first or if the attributes exist is not the issue, the chance of them running concurrently is the problem!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider the case where :
import random self.xdata = np.linspace(0, 10, random.randint(100, 150))
Which callback run first or if the attributes exist is not the issue, the chance of them running concurrently is the problem!
Hmm I'm pretty sure that the timers' callbacks run concurrently, and they block the event loop - that's my main conclusion of the investigation I performed on the "stuck" issue I experienced. In the case you presented, at the same callback that sets self.xdata
, self.ydata
should be set as well, to make sure they have the same size, and then when the blocking callback of the drawing happens - they will still have the same size.
Perhaps it is worth explaining in a comment? Or perhaps I will set self.xdata
in the _update_data
callback to the same value, although it is not really needed, just for the sake of the example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
I think this does add some complexity to example, but I think it is complexity the represents problems users who go down this route are likely to actually face (and the other option in QThreads).
0f92e8c
to
7af1d83
Compare
In the previous version of this example, if the timer interval would have been simply decreased to 1ms, the GUI could have appeared as stuck on some platforms / (slow) machines, because the event loop didn't find the time to respond to external events such as the user dragging the window etc. This change does 3 things: - Puts the data to plot in the self.{x,y}data attributes. - Separates the timer that updates the self.{x,y}data from the timer that updates the canvas - Explains much better the reasoning for the timers' intervals choices in comments, as well as explaining why the timers are attributed to self, although they are not used by other methods of the class. - Use the asynchronous draw_idle() function to further guarantee that the drawing won't be blocking. Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
28af910
to
b86e35e
Compare
Thanks for the suggestions @QuLogic . All applied :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took the liberty of renaming _update_data
-> _update_ydata
.
Sounds better to me. Thanks for merging! |
PR summary
While developing a Qt application that had to retrieve data as quickly as possible from a measurement instrument, and display that in multiple plots, I encountered an issue with my 1ms timers causing the application to not respond to events, and essentially making the app stuck and unusable. ChatGPT helped me to figure out I need to process the events in my timer timeout function. This has bugged me for a week or so, and this is not trivial to figure out by oneself IMO. Hence this PR. See also pyqtgraph/pyqtgraph#3092.
PR checklist