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

Why not double-buffer Output? #3253

Open
shaperilio opened this issue Aug 13, 2021 · 3 comments
Open

Why not double-buffer Output? #3253

shaperilio opened this issue Aug 13, 2021 · 3 comments

Comments

@shaperilio
Copy link
Contributor

Problem

When using interact with several figures, output is typically cleared when the first change arrives, and so you can "see it drawing as it goes". Here's a simple snippet:

from matplotlib import pyplot
import ipywidgets
from IPython.display import display
import numpy

def long_plots(x, y, z):
    pts = numpy.arange(x)
    pyplot.figure()
    pyplot.plot(pts, pts * y)
    pyplot.figure()
    pyplot.plot(pts, pts * z)
    
display(ipywidgets.interact(long_plots, x=(2, 10), y=(-3, 3), z=(-5, 5)))

As you move the sliders, you'll see the two plots being created. It's snappy on Chrome, but a bit slower in VSCode. It results in some flashing which is not desirable.

Proposed Solution

Why not double-buffer with a second Output instance, so that the one that is being displayed only receives one change? i.e. in interaction.py, replace this code:

with self.out:
    if self.clear_output:
        clear_output(wait=True)
    for widget in self.kwargs_widgets:
        value = widget.get_interact_value()
        self.kwargs[widget._kwarg] = value
    self.result = self.f(**self.kwargs)
    show_inline_matplotlib_plots()
    if self.auto_display and self.result is not None:
        display(self.result)

with something like this:

buffer = Output()
buffer.clear_output()
with buffer:
    for widget in self.kwargs_widgets:
        value = widget.get_interact_value()
        self.kwargs[widget._kwarg] = value
    self.result = self.f(**self.kwargs)
    show_inline_matplotlib_plots()
    # Note: Not sure how to handle this
    # if self.auto_display and self.result is not None:
    #     display(self.result)
with self.out:
    if self.clear_output:
        clear_output(wait=True)
    display(buffer)

(I acknowledge I'm not proposing a complete solution here; just trying to show the idea. I've prototyped it by extracting the code from interact_output and modifying it and the concept seems sound.)

Is there a reason this isn't done?

@blois
Copy link
Contributor

blois commented Oct 1, 2021

An alternative approach (which isn't quite working for me) would be to accumulate the output items on the kernel side then update the output widget with all of the resulting outputs at once.

An example of this would be to use display_pub.register_hook to re-route display messages to the output, based on

def interactive_output(f, controls):
:

from ipywidgets.widgets import interaction
from IPython.display import display, clear_output
import ipywidgets as widgets
import matplotlib.pyplot as plt


import numpy as np

def long_plots(x, y):
    pts = np.arange(100)
    plt.figure()
    plt.plot(pts, np.sin(pts * x))
    plt.figure()
    plt.plot(pts, y * np.sin(pts * x))

def interactive_output(f, controls):
    """Connect widget controls to a function.
    This function does not generate a user interface for the widgets (unlike `interact`).
    This enables customisation of the widget user interface layout.
    The user interface layout must be defined and displayed manually.
    """
    out = widgets.Output()
    def observer(change):
        kwargs = {k:v.value for k,v in controls.items()}
        interaction.show_inline_matplotlib_plots()

        outputs = []
        def output_hook(msg):
          if msg['msg_type'] == 'display_data':
            outputs.append(msg['content'])
            return None
          return msg

        get_ipython().display_pub.register_hook(output_hook)

        f(**kwargs)
        interaction.show_inline_matplotlib_plots()
        
        get_ipython().display_pub.unregister_hook(output_hook)
        out.outputs = outputs

    for k,w in controls.items():
        w.observe(observer, 'value')
    interaction.show_inline_matplotlib_plots()
    observer(None)
    return out


controls = {'x': widgets.IntSlider(min=1, max=10), 'y': widgets.IntSlider(min=-3, max=3)}
for k, w in controls.items():
  display(w)

interactive_output(long_plots, controls)

Unfortunately this doesn't seem to update the output contents... not sure why.

I really wish output widget did this by default as it would simplify a lot of output handling (and would avoid the flickering as the clear_outputs & accumulation of new outputs would be atomic)

maartenbreddels added a commit to widgetti/solara that referenced this issue Apr 14, 2023
maartenbreddels added a commit to widgetti/solara that referenced this issue Apr 14, 2023
maartenbreddels added a commit to widgetti/solara that referenced this issue Apr 14, 2023
maartenbreddels added a commit to widgetti/solara that referenced this issue Apr 14, 2023
@maartenbreddels
Copy link
Member

@blois I totally agree this is the way to do it.

In widgetti/solara#68 (2nd commit widgetti/solara@f1d580a ) I implement this when running under solara-server. It is also a much simpler implementation than the current Output widget __enter__ and __exit__.

Note that your solution doesn't work because you mutate the list, while if you assign, it will probably work.

maartenbreddels added a commit to widgetti/solara that referenced this issue Apr 14, 2023
maartenbreddels added a commit to widgetti/solara that referenced this issue Apr 14, 2023
maartenbreddels added a commit to widgetti/solara that referenced this issue Apr 14, 2023
maartenbreddels added a commit to widgetti/solara that referenced this issue Apr 14, 2023
@jasongrout
Copy link
Member

I really wish output widget did this by default

I gave a bit of context at #3759 (comment), but for convenience, I'll copy it here too:

The fundamental decision we had to make when originally implementing output widgets and evaluating the experiments at ipython/ipykernel#115 were: Do we make the kernel more complicated (with an output redirection mechanism) and the frontend simple (which arguably is a better architecture, like you point out), or do we make the frontend more complicated and need no changes to any kernel? We decided there were likely to be far more kernels than frontends, so the balance shifted to the somewhat awkward architecture we have today, which is kernel agnostic.

In practice, though, there have been fewer kernels implementing ipywidgets support and more frontends implementing ipywidgets support than (at least I) anticipated. So I'm glad that the tradeoff is being reevaluated.

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

No branches or pull requests

4 participants