Skip to content
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

Multicursor disappears when not moving on nbagg with useblit=False + burns CPU #19633

Closed
ianhi opened this issue Mar 3, 2021 · 13 comments · Fixed by #19763
Closed

Multicursor disappears when not moving on nbagg with useblit=False + burns CPU #19633

ianhi opened this issue Mar 3, 2021 · 13 comments · Fixed by #19763

Comments

@ianhi
Copy link
Contributor

ianhi commented Mar 3, 2021

Bug report

Bug summary
When on the nbagg backend if you stop moving the mouse the multicursor will disappear. The same example works fine on the qt backend.

Additionally I noticed that when I add the multicursor my cpu usage jumps and the kernel busy indicator constantly flashes on and off.

Showing the plot without the multicursor:
image
and with the multicursor (just displaying, not interacting with the plot):

image
That usage is pretty stable and my laptop's fan goes wild.

The issue with the dissappearing was originally noticed by @ipcoder in matplotlib/ipympl#306

Code for reproduction

%matplotlib nbagg
import numpy as np
import matplotlib.pyplot as plt
from matplotlib.widgets import MultiCursor

t = np.arange(0.0, 2.0, 0.01)
s1 = np.sin(2*np.pi*t)
s2 = np.sin(4*np.pi*t)

fig, (ax1, ax2) = plt.subplots(2, sharex=True)
ax1.plot(t, s1)
ax2.plot(t, s2)

multi = MultiCursor(fig.canvas, (ax1, ax2), color='r', lw=1, useblit=False)
plt.show()

Actual outcome

Peek 2021-03-03 18-12

and the high CPU usage

Expected outcome
Red line doesn't disappear + my CPU doesn't get crushed.

Matplotlib version

  • Operating system: Ubuntu
  • Matplotlib version (import matplotlib; print(matplotlib.__version__)): '3.3.4.post2456+gfd23bb238'
  • Matplotlib backend (print(matplotlib.get_backend())): nbagg
  • Python version: '3.9.1 | packaged by conda-forge | (default, Jan 26 2021, 01:34:10) \n[GCC 9.3.0]'
  • Jupyter version (if applicable): Notebook 6.2.0 - IPython 7.20.0

dev instlal of maptlotlib + conda-forge for the others

@ianhi
Copy link
Contributor Author

ianhi commented Mar 3, 2021

On matplotlib master nbagg supports blitting - so I also tried with that - which prevents the high cpu usage but the smearing of the image (#19116) is renders the widget unusable:

Peek 2021-03-03 18-35

so I think it's still important to fix the useblit=False case.

@ianhi
Copy link
Contributor Author

ianhi commented Mar 4, 2021

I think the CPU burning loop is happening because the multicursor attaches a callback to the draw_event that will it self trigger a draw event and then ♾️ followed by 🔥 💻 🔥

The path is:

self._ciddraw = self.canvas.mpl_connect('draw_event', self.clear)

to

def clear(self, event):
"""Clear the cursor."""
if self.ignore(event):
return
if self.useblit:
self.background = (
self.canvas.copy_from_bbox(self.canvas.figure.bbox))
for line in self.vlines + self.hlines:
line.set_visible(False)

and line.set_visible sets an artist to stale and then a draw happens again.

Confusingly this doesn't happen on the qt backend, but does on the nbagg backend???

You see this behavior with this:

%matplotlib notebook
import numpy as np
import matplotlib.pyplot as plt
from matplotlib.widgets import MultiCursor
import ipywidgets as widgets

t = np.arange(0.0, 2.0, 0.01)
s1 = np.sin(2*np.pi*t)
s2 = np.sin(4*np.pi*t)

fig, (ax1, ax2) = plt.subplots(2, sharex=True)
ax1.plot(t, s1)
ax2.plot(t, s2)

out = widgets.Output()
display(out)
n = 0
def drawn(event):
    global n
    n += 1
    with out:
        print(f'drawn! {n}')
fig.canvas.mpl_connect('draw_event', drawn)
multi = MultiCursor(fig.canvas, (ax1, ax2), color='r', lw=1, useblit=False)
plt.show()

Peek 2021-03-03 19-18

@QuLogic
Copy link
Member

QuLogic commented Mar 4, 2021

Having not looked at the implementation at all, a simple fix might be to cache the mouse position (which may already be available from the existing Line2D's current position), and then not do anything if the mouse hasn't moved?

@ianhi
Copy link
Contributor Author

ianhi commented Mar 4, 2021

@QuLogic looking at this again I think this is about nbagg and the js side rather than anything with multicursor. A simpler reproduction is:

%matplotlib nbagg
import matplotlib.pyplot as plt
from ipywidgets import Output

fig, ax = plt.subplots()
l, = ax.plot([0,1],[0,1])

out = Output()
display(out)
n =0
def drawn(event):
    global n
    n+=1
    with out:
        print(n)
    l.set_visible(False)
fig.canvas.mpl_connect('draw_event', drawn)

which may be due to the the draw message that the frontend sends back from here?

mpl.figure.prototype.send_draw_message = function () {
if (!this.waiting) {
this.waiting = true;
this.ws.send(JSON.stringify({ type: 'draw', figure_id: this.id }));
}
};

@tacaswell
Copy link
Member

What is going on with fig.stale?

The double-buffering that nbagg does may also be contributing here.

@tacaswell tacaswell added this to the v3.4.1 milestone Mar 4, 2021
@ericpre
Copy link
Member

ericpre commented Mar 10, 2021

I have been testing the matplotlib 3.4.0rc1 and I confirm the high CPU usage and significant slow down when using the notebook backend. There are also issue
I don't have a minimum example to reproduce without installing hyperspy but what we uses is fairly similar to the blitting tutorial. See https://github.com/hyperspy/hyperspy/blob/RELEASE_next_minor/hyperspy/drawing/figure.py for more details.

The example of the blitting tutorial doesn't seem to be working:

import matplotlib.pyplot as plt
import numpy as np

x = np.linspace(0, 2 * np.pi, 100)

fig, ax = plt.subplots()

# animated=True tells matplotlib to only draw the artist when we
# explicitly request it
(ln,) = ax.plot(x, np.sin(x), animated=True)

# make sure the window is raised, but the script keeps going
plt.show(block=False)

# stop to admire our empty window axes and ensure it is rendered at
# least once.
#
# We need to fully draw the figure at its final size on the screen
# before we continue on so that :
#  a) we have the correctly sized and drawn background to grab
#  b) we have a cached renderer so that ``ax.draw_artist`` works
# so we spin the event loop to let the backend process any pending operations
plt.pause(0.1)

# get copy of entire figure (everything inside fig.bbox) sans animated artist
bg = fig.canvas.copy_from_bbox(fig.bbox)
# draw the animated artist, this uses a cached renderer
ax.draw_artist(ln)
# show the result to the screen, this pushes the updated RGBA buffer from the
# renderer to the GUI framework so you can see it
fig.canvas.blit(fig.bbox)

It gives an empty figure:
image

and the following error message:

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-2-f625949ed20b> in <module>
     26 bg = fig.canvas.copy_from_bbox(fig.bbox)
     27 # draw the animated artist, this uses a cached renderer
---> 28 ax.draw_artist(ln)
     29 # show the result to the screen, this pushes the updated RGBA buffer from the
     30 # renderer to the GUI framework so you can see it

/opt/miniconda3/lib/python3.8/site-packages/matplotlib/axes/_base.py in draw_artist(self, a)
   2936         """
   2937         if self.figure._cachedRenderer is None:
-> 2938             raise AttributeError("draw_artist can only be used after an "
   2939                                  "initial draw which caches the renderer")
   2940         a.draw(self.figure._cachedRenderer)

AttributeError: draw_artist can only be used after an initial draw which caches the renderer

Using blitting is now slower than without... :( Any chance to have this fix before the 3.4.0 release? Or to have if disable, through the supports_blit property until it is working well enough?

@QuLogic
Copy link
Member

QuLogic commented Mar 19, 2021

What is going on with fig.stale?

The double-buffering that nbagg does may also be contributing here.

Changing to print(n, 'before', l.stale, l.axes.stale, l.axes.figure.stale) (and printing again after l.set_visible) prints out:

1 before False False False
1 after True True True
2 before True False False
2 after True True True
2 before True False False
2 after True True True

and never changes after that.

Whereas on Agg or TkAgg, it's all False, then all True, then stops.

So somehow the draw_event is called before all the Artists are marked up-to-date or something.

@tacaswell
Copy link
Member

I think the issue here is that:

  • the ob.clear method is hooked up to 'draw_event' which fires at the bottom of Figure.draw() (which is called from inside of Canvas.draw()`
  • in clear we set the cursor artists to be not visible (and it appears to have been that way for a long time)
  • in CanvasBase.draw_idle and in the pyplot._auto_draw_if_interactive we have a whole bunch of de-bouncing logic so that the draws triggered while drawing get ignored (this is why tkagg / qtagg does not go into the same infinite loop). I think I am missing some details here, but I do not think it changes the analysis. In IPython we only auto-draw when the user gets the prompt back from executing something (so no loops there!).
  • in nbagg when we trigger draw_idle on the python side we resolve that by sending a note to the front end to please request a draw. This eventually comes back to the python side which triggers the actual render. This extra round trip is what is opening us up to the infinite loop
  • One critical detail I may be missing is what in triggering the draw_idle in the nbagg case?

This goes back to at least 3.3 so is not a recent regression. I think that removing the set_visible(False) lines is the simplest and correct fix (or probably better, pulling the blit logic out into a method not called 'clear' and registering that with draw_event (as when we do a clean re-render (due to changing the size or similar) we need to grab a new background of the correct size).

@QuLogic
Copy link
Member

QuLogic commented Mar 24, 2021

Whereas on Agg or TkAgg, it's all False, then all True, then stops.

But something I missed before, is that the line is actually drawn. So the stale did not trigger a re-draw in other backends. The stale handler for figures in pyplot is:

def _auto_draw_if_interactive(fig, val):
"""
An internal helper function for making sure that auto-redrawing
works as intended in the plain python repl.
Parameters
----------
fig : Figure
A figure object which is assumed to be associated with a canvas
"""
if (val and matplotlib.is_interactive()
and not fig.canvas.is_saving()
and not fig.canvas._is_idle_drawing):
# Some artists can mark themselves as stale in the middle of drawing
# (e.g. axes position & tick labels being computed at draw time), but
# this shouldn't trigger a redraw because the current redraw will
# already take them into account.
with fig.canvas._idle_draw_cntx():
fig.canvas.draw_idle()

And the draw_idle for most backends will set a flag which is cleared when the draw actually happens (since they use event loops to signal this), but WebAgg does not. It always sends a draw message to the frontend, which has some sort of waiting flag, but I have not figured out why that does not limit things yet.

@QuLogic
Copy link
Member

QuLogic commented Mar 24, 2021

The second and subsequent draw_idle come from post_execute:

  File ".../matplotlib/lib/matplotlib/pyplot.py", line 138, in post_execute
    draw_all()
  File ".../matplotlib/lib/matplotlib/_pylab_helpers.py", line 137, in draw_all
    manager.canvas.draw_idle()
  File ".../matplotlib/lib/matplotlib/backends/backend_webagg_core.py", line 164, in draw_idle
    traceback.print_stack(None)

Didn't we have a previous issue with this?

@QuLogic
Copy link
Member

QuLogic commented Mar 24, 2021

Based on the original PR #4091 (comment), there is post_execute and post_run_cell; why did we use the former and not the latter? Do we even need this hook at all, with the stale figure tracking?

@QuLogic
Copy link
Member

QuLogic commented Mar 24, 2021

The previous similar issue was #13971 (comment), and the fix in that case was to avoid causing the figure to get marked stale during draw. As @tacaswell had mentioned earlier, doing the same in MultiCursor is probably the best option here.

QuLogic added a commit to QuLogic/matplotlib that referenced this issue Mar 24, 2021
This can be all handled in the mouse move event handler instead, and
prevents triggering extra draws in nbAgg.

Fixes matplotlib#19633.
QuLogic added a commit to QuLogic/matplotlib that referenced this issue Mar 24, 2021
This can be all handled in the mouse move event handler instead, and
prevents triggering extra draws in nbAgg.

Fixes matplotlib#19633.
@QuLogic QuLogic modified the milestones: v3.4.1, v3.5.0 Mar 30, 2021
@tacaswell
Copy link
Member

The stale machinery was a bit contentious when it went in (and @mdehoon was more correct than I understood at the time 🐑 ).

If we have IPython we use draw_all on the post-execute callback to find and selectively re-draw stale figures. This means we only call draw_idle once per figure per execution. If we do not have IPython, then we call draw_idle directly when the figure is marked as stale (which had its own set of deeply-recursive-draw-triggers-draw issues). This has the advantage of being more direct, but I think would have shown this self-feeding-draw in other backends (which is testable)


Based on the original PR #4091 (comment), there is post_execute and post_run_cell; why did we use the former and not the latter? Do we even need this hook at all, with the stale figure tracking?

I no longer remember why we went with post_execute over post_run_cell which re-reading that now 'post_run_cell' seems better. I think the trade off is a more minimal foot print vs being able to write ipywidget callbacks that just set state and then automatically get updated (maybe I can ret-con that as the reasoning?). Given the issue here, maybe we should re-consider that?

QuLogic added a commit to QuLogic/matplotlib that referenced this issue Apr 7, 2021
This can be all handled in the mouse move event handler instead, and
prevents triggering extra draws in nbAgg.

Fixes matplotlib#19633.
@QuLogic QuLogic modified the milestones: v3.5.0, v3.6.0 Sep 25, 2021
@QuLogic QuLogic modified the milestones: v3.6.0, v3.7.0 Jul 8, 2022
tacaswell pushed a commit to QuLogic/matplotlib that referenced this issue Dec 16, 2022
This can be all handled in the mouse move event handler instead, and
prevents triggering extra draws in nbAgg.

Fixes matplotlib#19633.
Marvvxi pushed a commit to Marvvxi/matplotlib that referenced this issue Dec 20, 2022
This can be all handled in the mouse move event handler instead, and
prevents triggering extra draws in nbAgg.

Fixes matplotlib#19633.
raphaelquast pushed a commit to raphaelquast/matplotlib that referenced this issue Mar 16, 2023
This can be all handled in the mouse move event handler instead, and
prevents triggering extra draws in nbAgg.

Fixes matplotlib#19633.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants