Skip to content
This repository has been archived by the owner on Jan 26, 2022. It is now read-only.

Possibility to support ipykernel #274

Open
saulshanabrook opened this issue Dec 11, 2019 · 37 comments
Open

Possibility to support ipykernel #274

saulshanabrook opened this issue Dec 11, 2019 · 37 comments
Labels
backend Issues and pull requests that depend on the kernel question Further information is requested

Comments

@saulshanabrook
Copy link
Member

We have a client who is interested in this work and got this feedback:

This looks very, very promising. I noticed it requires xeus-python. Do we know if they plan to support the ipykernel in the long run? While I have no objection to the xeus kernel in theory, in practice it seems to both be not be popular (73 stars after over a year of development) nor have all features of the ipykernel. It would seem one or the other would have to give for this to be feasible for us.

So what would we have to add to ipykernel to make it compatible? And do you see any large blockers in implementing that support?

@KsavinN KsavinN added backend Issues and pull requests that depend on the kernel question Further information is requested labels Dec 11, 2019
@jtpio
Copy link
Member

jtpio commented Dec 11, 2019

It would be great to have support for it yes.

To make it compatible ipykernel will have to support the Debug Adapter Protocol, for example by using ptvsd (like xeus-python). As well as a couple of other messages that are not part of the DAP, such as debugInfo (to retrieve the state of the current debug session) and dumpCell (to debug cells).

Probably the main obstacle would be to make sure ipykernel is able to process messages from the control channel in a concurrent manner while shell is blocked (for example when the execution is stopped after hitting a breakpoint).
The control channel is used to send and receive the debug messages: https://jupyter-client.readthedocs.io/en/latest/messaging.html#debug-request

There is an overview of the debug protocol in #64 (comment). The plan is to add more information to it and make an official documention so it's easier for other kernels to start adding support for debugging.

For the JupyterLab extension, this shouldn't require any change.

@saulshanabrook
Copy link
Member Author

Probably the main obstacle would be to make sure ipykernel is able to process messages from the control channel in a concurrent manner while shell is blocked (for example when the execution is stopped after hitting a breakpoint).

Does anyone know if there is an existing issue for this open in ipykernel or have more insight into how much work this would take? cc @Zsailer @Carreau

@SylvainCorlay
Copy link
Member

SylvainCorlay commented Dec 11, 2019

There are some intrinsic limitations with ipykernel which may require either the architecture to be substantially changed, or the experience of the debugger to be degraded.

Also, I don't think that the popularity is a factor at all here. xeus-python or the debugger have not been advertised widely - and I don't think that feature parity is out of reach given the current coverage of the kernel feature.

@saulshanabrook
Copy link
Member Author

saulshanabrook commented Dec 11, 2019

So if we assume that the debugger will grow to be a core feature in JupyterLab that we want to enable for folks, then it seems to suggest that we either:

  1. Re-architect ipykernel to support this message processing issue so we can add debugger support
  2. Promote xeus-python instead of ipykernel as the default Python kernel (possibly leading to it shipped by default instead the ipykernel) and make sure it has feature parity with ipykernel

Although at the moment this is just a debugger conversation, it seems like the implications here are far reaching in terms of what we choose to support going forward with JupyterLab. Ideally, it would be nice to know which direction to move before we invest substantial resources.

@SylvainCorlay
Copy link
Member

SylvainCorlay commented Dec 11, 2019

Promoting xeus-python instead of ipykernel would be controversial, but there is an ongoing JEP on putting xeus and related project under the Jupyter governance in a Jupyter-affiliated GitHub organization.

This would be the case for xeus, xeus-python, xeus-cling, and some related utility projects.

So Xeus will become a reference implementation of the Jupyter protocol, and include the first implementation of the DAP.

@SylvainCorlay
Copy link
Member

SylvainCorlay commented Dec 11, 2019

I would argue that xeus is a safe bet for broader adoption. QuantStack is funded for a little while to continue pushing on the project and downstream projects (the debugger, xeus-python, etc).

Since it is a reboot of the kernel implementation, it has less bagage and already has a broad coverage of the features with a much lighter-weight codebase.

@SylvainCorlay
Copy link
Member

Also, regarding the inclusion into JupyterLab, I don't think that it requires that ipykernel supports the protocol, or that xeus-python is "the official kernel". JupyterLab is a kernel agnostic front-end. It can advertize the debugger features for kernels that support the debugger protocol.

@timkpaine
Copy link
Member

timkpaine commented Dec 11, 2019

As an end user (and non-conda user), xeus-python has always been difficult to build and integrate. The matrix of dependency compatibilities is confusing, and compilation on centos 6 has not been straightforward. If there was a manylinux1 wheel published on pypi, i'd probably have a different opinion (and obviously we can make some of these things more user friendly).

@SylvainCorlay
Copy link
Member

As an end user (and non-conda user), xeus-python has always been difficult to build and integrate. The matrix of dependency compatibilities is confusing, and compilation on centos 6 has not been straightforward. If there was a manylinux1 wheel published on pypi, i'd probably have a different opinion (and obviously we can make some of these things more user friendly).

Issue jupyter-xeus/xeus-python#181 tracks progress on building a PyPI wheel. Beyond the need for a wheel, do you have specific feedback on improving the build process for xeus-python?

@Carreau
Copy link

Carreau commented Dec 11, 2019

I'd love to have the protocol implemented in ipykernel; I understand that it might be difficult to have the same experience as Xeus-Python; One thing I want to avoid as much as possible is having a confusing story.

If users needs to switch between ipykernel and Xeus-python depending on wether they want to use a given magic or the debugger the experience will be sub-par. If we have to choose, a clear distinction of features would be better than two almost similar projects.

On the note of popularity; I believe this is likely due to which kernel is shipped by default and the popularity of Xeus-Python would likely go up quite a bit if we start to ship it by default with conda for example.

@timkpaine
Copy link
Member

@SylvainCorlay Not really, the biggest headache with these types of packages are always that either the wheel has a glibc dependency that wont work on centos 6, or that the source build either tries to bring in the whole world without letting me tell it that i already have slightly different but compatible versions of the dependencies.

The fact that i can build and separately control all the dependencies is great, though the docs only suggest installing them with conda. We could just quickly make a wiki to document some of the bugs that pop out if you go to install every dependency from source, most of which I'm sure you already stumbled upon when you packaged all this stuff up for conda.

@SylvainCorlay
Copy link
Member

I'd love to have the protocol implemented in ipykernel; I understand that it might be difficult to have the same experience as Xeus-Python; One thing I want to avoid as much as possible is having a confusing story.

  • I think that we can have a meaningful (although a bit degraded) experience with ipykernel without too much refactoring around the concurrency model... So there might be a tractable path towards a debugger implementation with ipykernel, but without the ability to e.g. add a breakpoint when code is running etc. A deeper refactoring could follow later.
  • The choice to go with xeus was due to the following factors:
    • the more flexible concurrency model, which was needed for the debugger.
    • the smaller codebase to iterate upon.
    • it was also a very convenient sandbox to iterate on the protocol, while it would have been a much longer process to do this in ipykernel. Now that this is stabilizing, we will go through a formal addition to the protocol.

@SylvainCorlay
Copy link
Member

The fact that i can build and separately control all the dependencies is great, though the docs only suggest installing them with conda.

Yeah, we should probably put more content on that. It is a bit richer for the core xeus package though.

We could just quickly make a wiki to document some of the bugs that pop out if you go to install every dependency from source, most of which I'm sure you already stumbled upon when you packaged all this stuff up for conda.

Yes. building dependencies got much simpler when we moved from cryptopp to openssl which is much more streamlined.

@ccordoba12
Copy link

So there might be a tractable path towards a debugger implementation with ipykernel, but without the ability to e.g. add a breakpoint when code is running etc. A deeper refactoring could follow later.

We can already do that in the Spyder kernel (which is a subclass of ipykernel) by using Comms that run in their own Zmq socket. So this is possible today for ipykernel.

Pinging @impact27, who implemented that for the details.

@SylvainCorlay
Copy link
Member

Comms that run in their own Zmq socket.

Note that Comms of the Jupyter kernel protocol are on the Shell channel and queued behind execution requests.

their own Zmq socket.

This is essentially the problem that we solve by using messages over the control channel, which are not queued behind execution requests and other shell messages.

@ccordoba12
Copy link

Note that Comms of the Jupyter kernel protocol are on the Shell channel and queued behind execution requests.

Yeah, I think we created our own channel just for Comms instead of handling them through the Shell one.

This is essentially the problem that we solve by using messages over the control channel, which are not queued behind execution requests and other shell messages.

So it seems we solved this problem in much the same way.

@SylvainCorlay
Copy link
Member

So it seems we solved this problem in much the same way.

Yes. Actually I wanted to reach out to you and the Spyder team to discuss how we could converge in that regard, and support both approaches in Spyder (although I think we should take discuss this in a different thread).

@ccordoba12
Copy link

You could open an issue at

https://github.com/spyder-ide/spyder-kernels

and start the discussion over there.

By the way, I didn't want to derail the discussion here, just wanted to mention that debugger improvements are doable in ipykernel.

@SylvainCorlay
Copy link
Member

No problem. I litterally had "spyder control channel" on my todo list in front of me (whatever that means). Will be posting an issue in the spyder-kernels repo.

@impact27
Copy link

So there might be a tractable path towards a debugger implementation with ipykernel, but without the ability to e.g. add a breakpoint when code is running etc. A deeper refactoring could follow later.

We can already do that in the Spyder kernel (which is a subclass of ipykernel) by using Comms that run in their own Zmq socket. So this is possible today for ipykernel.

Pinging @impact27, who implemented that for the details.

The idea is to have a new channel running in a thread that can be called even when code is executing in the main thread. Obviously you have to be careful about what you execute there but this means you can set a breakpoint while the debugger is running, and therefore e.g. break into a loop

@SylvainCorlay
Copy link
Member

SylvainCorlay commented Dec 12, 2019

The idea is to have a new channel running in a thread that can be called even when code is executing in the main thread. Obviously you have to be careful about what you execute there but this means you can set a breakpoint while the debugger is running, and therefore e.g. break into a loop

Thanks @impact27 this is what is done in xeus with the control channel, although we don't use comms, but add specific messages to the protocol (debug_[request/reply], and debug_event). The master issue tracking these changes is here: jupyter/jupyter_client#446

@vladimirlagunov
Copy link

vladimirlagunov commented Dec 13, 2019

Hi! We, at JetBrains, want to add support of debugging remote notebooks in PyCharm. We already have a debugger for local notebooks, and our debugger does not support protocol of ptvsd. Instead, we propose to add an abstraction layer like Comm for Control channel in a separate thread. ipython/ipykernel#447

Such abstraction can encapsulate a protocol for ptvsd, a protocol for PyCharm debugger and a lot of other various features that hardly can be implemented using Comm with Shell channel. Moreover, it would be convenient even for ptvsd: its handler can be implemented outside of ipykernel code base and integrated into a kernel using a regular cell execution. No need for integrating many files of code into ipykernel, no need for updating the kernel if support of ptvsd updates.

@SylvainCorlay
Copy link
Member

Thanks for chiming in @Werehuman !

The current implementation in jupyterlab/debugger does not use comms at all. Instead, there are two dedicated message types added to the control channel:

  • debug_[request/reply]
  • debug_event

For more details, you can check out jupyter/jupyter_client#446.

This can be implemented in ipykernel and is already implemented in xeus-python. The benefit of using the control channel is that messages are not queued behind execution requests.

@saulshanabrook
Copy link
Member Author

I believe @Werehuman is advocating for an extensible control channel, similar to how comms are extensible. That way things like the debugger wouldn't have to be present in the core kernel code.

@SylvainCorlay
Copy link
Member

Ah, yes, we though of that (and even started that way), but thought it might be overkill eventually. Comms bring a lot of complications such as a need for a target registry etc.

The debug_[request/reply] / debug_event pattern is fairly simple and can accommodate the debug adapter protocol very well (we are also looking into a backed for the C++ kernel).

@SylvainCorlay
Copy link
Member

There is a demand for a more "async" channel to communicate with the front-end from the kernel, for widgets usecases, but I don't think that it should be the control channel.

The idea is that the user code should not really be able to use the control channel, which is meant to control the kernel (shutdown, interupt, debug)...

@SylvainCorlay
Copy link
Member

That way things like the debugger wouldn't have to be present in the core kernel code.

We plan on proposing for the kernel_info_request to indicate whether the kernel supports the debugging protocol. If the kernel info request does not include any info wrt debugging, we assume that it does not support debugging.

@westurner

This comment has been minimized.

@SylvainCorlay
Copy link
Member

Mixed Python/C debugging

I think this question deserves its own separate issue!

@westurner
Copy link

I think this question deserves its own separate issue!

Would the changes necessary to support multiple attached debuggers substantially change the protocol that ipykernel will be implementing?

@SylvainCorlay
Copy link
Member

Would the changes necessary to support multiple attached debuggers substantially change the protocol that ipykernel will be implementing?

I have not thought too much about it, but almost thinking out loud it might actually be possible to not change the frontend at all which is not python specific.

@Carreau
Copy link

Carreau commented Jan 14, 2020

Following upon this. I'm about to start to work on an IPython 8.0; that is to say the master branch of IPython will soon likely get lots of changes and we can get wild with changing API if necessary. Depending on what we want to do we can decide to also make major release with change of API of all the pieces that are necessary to have the debugger working.

Now if we can minimize the disruption that would be great; but we can decide to do a major refactoring.

@blois
Copy link

blois commented Aug 21, 2020

It seems problematic to use the process being debugged to forward messages to the debug adapter- at a glance, I believe that it's fairly common for debuggers to pause all threads in the process when stopped at a breakpoint. Even if the ipykernel control channel thread could be spawned as a native thread (and outside of the control of ptvsd), does that mean it could not call any Python code while at a breakpoint?

@SylvainCorlay
Copy link
Member

I think we should do the split or control in its own thread right away.

@blois
Copy link

blois commented Aug 22, 2020

And the control thread would not use any Python?

According to the additions to the DAP the two additions that are provided are session resumption and cell source files. My understanding is that the cell source files are not needed for ipykernel since it gives each cell it's own source file already via the code object. The debug session resumption seems like it could be done by any DAP proxy- given that, would it be possible to put the proxy into the kernel manager process instead? Startup may be a bit more complicated from the kernel manager but this seems like a more viable approach.

@marcglobality
Copy link

really nice project! I really would like to use this, but ipykernel is a must. What's the state of this?

@edurenye
Copy link

edurenye commented Oct 2, 2020

Seems like it will be there on the 3.0 release, you can already test on the RC versions.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backend Issues and pull requests that depend on the kernel question Further information is requested
Projects
None yet
Development

No branches or pull requests