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

AnyIO commit breaks Matplotlib display in separate windows #1235

Open
ianthomas23 opened this issue May 3, 2024 · 5 comments · Fixed by #1265
Open

AnyIO commit breaks Matplotlib display in separate windows #1235

ianthomas23 opened this issue May 3, 2024 · 5 comments · Fixed by #1265

Comments

@ianthomas23
Copy link
Collaborator

With the commit before the AnyIO PR on the main branch (830829f) and using ipython 8.23.0 and matplotlib 3.8.4, the displaying of Matplotlib plots in separate windows (e.g. with qt backend) using jupyter works fine. With the AnyIO commit (772dfb8) onwards the plot windows are no longer displayed.

Code to reproduce (I've been using jupyter qtconsole but jupyter lab, etc, all show the same):

%matplotlib qt
import matplotlib.pyplot as plt
fig, ax = plt.subplots()
ax.plot([1, 3, 2])

In ipython the same example works fine.

@davidbrochart
Copy link
Collaborator

Probably because of external event loop integration.

@ianthomas23
Copy link
Collaborator Author

enter_eventloop is only ever called if kernel.eventloop is set at startup:

async def main(self):
async with create_task_group() as tg:
if self.kernel.eventloop:
tg.start_soon(self.kernel.enter_eventloop)
tg.start_soon(self.kernel.start)

If the eventloop is set later, e.g. via %matplotlib qt, it is never called.

Plus there are 7 uses of kernel.shell_stream in eventloops.py and the shell_stream was replaced with shell_port in the AnyIO changes. So evidently this code isn't covered by any ipykernel tests.

@minrk
Copy link
Member

minrk commented Dec 5, 2024

I've been looking into this today, and it is definitely the case that the way the event loop integration expects to return control has assumptions about socket use that are not satisfied by the changes in how sockets are used.

In particular, the event loop integration pattern is that the event loops block and hold control until there is a message waiting to be received on the shell socket, at which point control is returned to the kernel. But with the new async architecture, there could be any number of events waiting to process, including the coroutines involved in actually handling a message.

It is not hard to test this, because this test:

@pytest.mark.parametrize("gui", [
    "qt",
    "osx",
])
def test_gui_responsive(kernel_client, gui):
    if gui == 'osx':
        if sys.platform == 'darwin':
            pytest.importorskip("matplotlib")
        else:
            pytest.skip("osx test only on mac")
    if gui == 'qt' and len(qt_guis_avail) == 0:
        pytest.skip("No qt backend available")
      
    kc = kernel_client

    # enable gui
    msg_id, reply = execute(f"%gui {gui}", kc=kc)
    assert reply['status'] == 'ok'
    flush_channels(kc)

    # kernel should remain responsive (it doesn't)
    msg_id, reply = execute(f"print(1)", kc=kc, timeout=2)
    assert reply['status'] == 'ok'
    flush_channels(kc)

reproduces the hang.

I think the fix is probably to change the advance_eventloop functions to do what the call appears to expect, which is to step the event loop once, never blocking for an extended period waiting for a specific wake event. This is far less efficient than what we had before, but I think is the only way to properly integrate the two full event loops that we have going now since there is no longer a simple, clear signal that there's something waiting to be processed in the kernel's loop.

@minrk
Copy link
Member

minrk commented Dec 5, 2024

For example, removing the poll here and just returning control on every poll interval enables both the matplotlib window and the kernel to remain responsive with %gui osx.

This seems to stem from a general misunderstanding that we have advance_eventloop functions, when what we actually have is "run eventloop forever until there is something to process in the kernel's loop" functions, and the socket poll we have for that is not an adequate condition for waking for async tasks. I'm not sure there's a way to ask the event loop if there's anything waiting to be processed?

Removing the poll greatly simplifies what these eventloop integration functions need to do, though, because they no longer need to hook up any waking mechanism, they can simply run for a finite period and return control. This is simpler, but means that both eventloops block each other for predefined periods of time.

Some of these event loops like qt now have a mechanism for asyncio cooperation, which we might be able to use. Of course, that's much easier with asyncio directly than it is with anyio.

@davidbrochart
Copy link
Collaborator

I'm wondering if we could use Trio's guest mode, if it was available in AnyIO.

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

Successfully merging a pull request may close this issue.

3 participants