emacs-jupyter / jupyter

An interface to communicate with Jupyter kernels.
GNU General Public License v3.0
941 stars 92 forks source link

Make usage of zmq module optional when using REST interface #176

Open stefanv opened 5 years ago

stefanv commented 5 years ago

I saw in the README that emacs-jupyter can now communicate with Jupyter using its REST API. Instructions are provided on Reddit.

Unfortunately, it looks like the usage of modules (which were required for 0mq communications) is still enforced, making it impossible to use the package on, e.g., a standard Debian installation of Emacs.

Since modules will likely not be enabled by most of the distros, this seems like a big impediment to adoption. Is it feasible to make modules optional?

nnicandro commented 5 years ago

Since modules will likely not be enabled by most of the distros

Hopefully modules will be enabled by default eventually.

Why ZMQ is still used

We still require zmq because the websockets are opened in a separate Emacs subprocess that is responsible for the actual sending, receiving, and encoding/decoding of JSON messages to/from the kernel. The parent Emacs process just sends the subprocess elisp representations of messages to send to the kernel and receives elisp representations to read.

The reason why zmq is necessary in such a situation is because we need to be able to poll stdin in the subprocess. This way we avoid blocking the process waiting for input from the parent process. As far as I can tell, there aren't any functions that allow you to determine if something can be read from stdin without actually waiting for input to arrive. This is unfortunate because we want to be able to do all that encoding/decoding/sending/receiving while waiting for input from the parent process.

So in order to avoid blocking we have to poll file descriptors/sockets using zmq. Actually we don't send input to the subprocess on stdin anymore. You can't poll file descriptors on Windows, so we use a zmq socket instead for both Windows and Unix based systems. We could still send input to stdin on Unix systems, but there is an additional problem. We run into issues related to stdin being a buffered stream and polling only working on lower level file descriptors.

Why subprocesses are still used

Yes all these issues can be avoided if the JSON parsing and message sending/receiving to/from the kernel are moved into the parent Emacs process. We would also be able to get rid of the extra serialization and de-serialization of the elisp representation of messages.

I had previously tried doing all the message handling without a subprocess. Not with the REST API, still with zmq. The jupyter-zmq-channel-comm class, after replacing references to jupyter-channel-ioloop-comm below to jupyter-zmq-channel-comm https://github.com/dzop/emacs-jupyter/blob/aa9b634e7b26347a9b938da4cb97184b73651a64/jupyter-repl.el#L2222-L2226 will do all of the message handling in the parent Emacs process (I think you also need to have threading enabled in your Emacs, but that isn't strictly necessary as things can be done using timers).

I never did a proper benchmark, but running the test suite without using a subprocess gave, very roughly, a 2x slow down compared to using a subprocess. After seeing that, and doing M-x elp-instrument-package RET jupyter RET and then M-x elp-results after using emacs-jupyter for a while, I was satisfied sticking with subprocesses.

Is it feasible to make modules optional?

Yes it is feasible, if someone is willing to put in the effort. I've worked on making the dependency on zmq as minimal as possible. Someone would have to

(1) create a jupyter-comm-layer class adapted from jupyter-server-ioloop and jupyter-ioloop-comm that does all its communication with websockets in the current Emacs instance. The bulk of this would be porting over jupyter-server-ioloop-add-send-event to a jupyter-send method of the new class and calling jupyter-event-handler when a websocket receives a message.

(2) Make a new class like jupyter-server that inherits from the jupyter-comm-layer created in (1) instead of a jupyter-ioloop-comm. We should probably have some kind of base jupyter-server class that implements the functionality of jupyter-server that is independent of jupyter-ioloop-comm. Then, in the new class, inherit from that base class and implement the required functionality. Probably not much needs to be done here.

That should be all that is needed to get something going. I don't see myself implementing this anytime soon. It will probably be up to someone else who requires this functionality. Of course, I'm glad to help anyone who is willing to tackle it.

stefanv commented 5 years ago

Fantastic summary, thank you very much.

dickmao commented 5 years ago

If you think it's "fantastic", you likely didn't make a real attempt at grokking it.

The exposition in this repo tends to be fairly rambly and technically dense (just look at the README). I would like to make a go at this as module-support (while forthcoming in emacs-27) is a sticking point for this humble ubuntu 16.04 user.

stefanv commented 5 years ago

@dickmao The author took their time to write up a background to the problem, and provided avenues for addressing it; of course, I am grateful. It sounds as though you have a more thorough exposition of the problem in mind, though, so I look forward very much to reading it here.

dickmao commented 5 years ago

I spent a couple days grokking the machinery.

The key is to replace the zmq-based messaging core in jupyter-ioloop with the elisp native make-network-process.

Abstracting jupyter-ioloop to be messaging-agnostic, and deriving two objects jupyter-zmq-ioloop and jupyter-native-ioloop is a common but dreaded OOP task.

There is a huge caveat to the undertaking however. In a byzantine arrangement reminiscent of the halting problem, the looping code is expressed as a meticulously constructed elisp string that is fed to a child emacs (I hadn't known the "subprocess" was actually another emacs!). This makes the aforementioned OOP refactoring devilishly difficult. There is also a "ye olde" stream buffering issue that the author believes is only surmountable with zmq. I am skeptical.

I spent some time on an alternative, less onerous plan. I'd preserve jupyter-ioloop as-is, and tastefully monkey patch all the zmq bits. I think if I spent a week on it, I could get it to work. A moment ago, I decided I didn't care enough to verify that conjecture.

Given the praise I've seen for this package, I thought it could be the "once-and-for-all" solution for emacs repls in all languages. Perhaps it still can be once module support becomes universal. But for me, the bloom has come off the rose once I grokked the emacs-within-emacs chicanery. I also believed code could be shared between this package and EIN, but I now see that that is impossible.

stefanv commented 5 years ago

Thank you for taking the time to investigate, @dickmao. You identified a higher level design concern, and it would be interesting to hear if you or @dzop have thoughts on alternatives to executing strings via Emacs (I note @dzop did mention that this is the approach taken in the original problem summary). It would be ideal if the parts of EIN used to do the kernel communications can be reused, but I haven't looked at that code at all so have no appreciation for how practical that would be.

nnicandro commented 4 years ago

dickmao: The key is to replace the zmq-based messaging core in jupyter-ioloop with the elisp native make-network-process.

Yes if you wanted to stick with a jupyter-ioloop for doing the messaging that would work. You could even do something like start a websocket-server in the parent Emacs process and connect to it in the ioloop process with websocket-open. Using make-network-process directly would probably be better since it removes the extra layer that websockets introduce. It requires digging into the lower level bits of jupyter-ioloop though.

My proposal was to get rid of the use of jupyter-ioloop entirely and create a class (specifically a jupyter-comm-layer) that

dzop: does all its communication with websockets in the current Emacs instance

since the goal is to remove the use of Emacs modules and the only point at which modules become relevant is in the jupyter-ioloop class. This seems, to me, to be a simpler route than the one you suggest.

You are focusing on the lower level jupyter-ioloop whereas I am focusing on the higher-level abstraction, jupyter-comm-layer. It would be simpler to do it at the level of jupyter-comm-layer I think. Since at that level one only needs to implement a jupyter-event-handler method that transforms the messages received on the websockets into the form expected by a jupyter-kernel-client, see jupyter-client.el#L519. You can see an example of how the websocket messages are transformed in the implementation of jupyter-server-ioloop, specifically jupyter-server-ioloop--on-message.

dickmao: In a byzantine arrangement reminiscent of the halting problem, the looping code is expressed as a meticulously constructed elisp string that is fed to a child emacs

The looping code is a valid Emacs-Lisp program (an sexp) being built by another Emacs-Lisp program, converted into a printed representation (that can be read), and passed over to another Emacs process to interpret (by calling read on the printed representation).

How can the "code as data" mantra of Lisp be viewed as "byzantine" (or as "chicanery") if it is a core principle of Lisp?

dickmao: (I hadn't known the "subprocess" was actually another emacs!)

The documentation of jupyter-ioloop mentions it:

https://github.com/dzop/emacs-jupyter/blob/d4b06c54d32131b90351b6a83cfe80ee4a9f0cdd/jupyter-ioloop.el#L97-L101

It could use some work as an explanation though. If anyone is willing to update the explanations in the documentation, I would welcome the pull request.

dickmao: There is also a "ye olde" stream buffering issue that the author believes is only surmountable with zmq.

I never said that the only solution to the problem was the one that I gave. I said to make modules optional someone could implement my proposal and doing so

dzop: should be all that is needed to get something going

meaning that it would be a starting point to a solution, and that I would be

dzop: glad to help anyone who is willing to tackle [making modules optional]

I wouldn't expect someone's proposal to ever be the only solution and I hope no one would ever box themselves in to thinking that they have to do something in only one way.

dickmao: I also believed code could be shared between this package and EIN, but I now see that that is impossible

In what sense do you believe it impossible? Both this package and EIN talk to Jupyter kernels, and assuming someone writes the code necessary to remove the use of Emacs modules, one could implement a jupyter-kernel-client class and override the default jupyter-handle-execute-reply kinds of methods.

dickmao commented 4 years ago

It's impossible in the sense that sharing would mean one of us would have to do major rewrite. i.e., getting rid of jupyter-ioloop entirely. Perhaps you consider that a modest undertaking, in which case I say more power to you!

My remarks regarding "ye olde" buffering derive from the three whole paragraphs you wrote explaining why zmq was necessary. If you no longer believe that, then great! On the other hand, many users have sung the praises of the package's zmq use -- zmq is jupyter's original, compact communication layer -- and it would be a mild shame to drop it, especially given the nontrivial integration effort.

It's fun to talk about, but I don't think any of this will get implemented. Most github "enhancements" just get the label, and sit unattended in perpetuity. In any case, grateful to have gotten your feedback.

nnicandro commented 4 years ago

I did not say get rid of jupyter-ioloop, I said get rid of the use of jupyter-ioloop and create a higher level class that probably would have needed to be created anyways since jupyter-ioloop is a lower level part of the current design of the code. My focus is on how to make zmq modules optional, not how to keep the underlying way messages are communicated the same.

I decoupled the underlying way messages are passed from the rest of the code by introducing jupyter-comm-layer, so not using jupyter-ioloop and doing all of the communication in the current Emacs process would allow us to both remove the use of jupyter-ioloop and implement code that would probably need to be implemented anyways in the current design.

Also, I would have never believed that zmq was the necessary (as in only) solution. I answered the question why zmq was still used given that I implemented the REST interface. Necessary may have not been a good choice of words at the time, but there is a context within which it was written.

Finally, there is evidence that your github "enhancements" comment is false from a fact based perspective (https://github.com/search?q=label%3Aenhancement) so I'll assume you are talking about the issues in this project when I say: Anyone is welcome to submit pull requests for features that they think would be beneficial for the project and I will most likely accept it. If you think the change is somewhat controversial, then I would advise an issue be opened first before submitting a pull request. Also, if somone believes the documentation could be worded better, please feel free to change it and submit a pull request. I also believe many parts of the documentation can be worded better.

P.S.

I'm starting to believe that since you view my exposition "rambly and technically dense" and are accusing me of "chicanery" nothing more I write will be viewed in a positive light. Going further, all posts on this issue from me will be related to the topic of the issue or to correct any more incorrect assumptions. Thanks for your support!

vv111y commented 4 years ago

@dzop emacs-zmq fails for emacs 27 & 28 on macos and emacs 28 on linux (arch. I will check emacs 27 on arch today; I have a feeling it will fail there too. Which is easier to do, implement this or fix emacs-zmq?

UPDATE: also fails with emacs 27 on linux.

nnicandro commented 4 years ago

@vv111y I know this package works on Linux systems with Emacs 27 since I have run the tests against that version and other people also have used this package in Emacs 27, see #219. Could you open an issue over at emacs-zmq. In the meantime, maybe you can try and build the module manually instead of downloading a pre-built binary. You can build the module by running, at the command line

cd ~/.emacs.d/elpa/emacs-zmq
rm emacs-zmq.so # Remove the old module
make

Also, I have made progress on this issue. Most of the changes have already been merged into master, albeit accidentally. I'm still working out the kinks, but maybe it already works fine since I was able to get all of the current tests to pass. To try it out, update the emacs-jupyter package to one that includes ee8b5180e58ea64ede77f9433839cab2497b14aa and evaluate

(setq jupyter-server-use-zmq nil)

before trying to connect to a kernel. You should then be able to M-x jupyter-server-list-kernels without ZMQ loading.

dickmao commented 4 years ago

Wow, well, I stand corrected about the likelihood of this issue's resolution.

I couldn't get it to fly in my module-less emacs 26.3 on ubuntu 16.04.

Here's my MRE:

dick@dick:~/emacs-jupyter$ emacs -Q --batch -f package-initialize --eval "(setq org-babel-load-languages (quote ((jupyter . t))))" --eval "(setq jupyter-server-use-zmq nil)" --eval "(setq org-confirm-babel-evaluate nil)" --eval "(with-temp-buffer (org-mode) (insert \"#+BEGIN_SRC jupyter-python :session py\n3.14\n#+END_SRC\n\") (princ (buffer-string)) (call-interactively #'org-ctrl-c-ctrl-c))"

#+BEGIN_SRC jupyter-python :session py
3.14
#+END_SRC

executing Jupyter-Python code block...
Starting python3 kernel process... \
Starting python3 kernel process... |
Starting python3 kernel process...done
Modules are not supported

I think the zmq module calls user-error before we even consider jupyter-server-use-zmq. I tried deleting the zmq elisp module altogether but that resulted in a bigger failure to launch.

nnicandro commented 4 years ago

@vv111y @dickmao Nevermind for right now, I just reverted the changes since, as I said I didn't really mean to push to the master branch yet, and there seems to be other issues popping up #234, #235 related to those changes.

I will upload a pull request branch later in the day for anyone interested in. EDIT: See https://github.com/dzop/emacs-jupyter/tree/soft-zmq

@dickmao Thanks for the MRE.