Skip to content

Add support for asynchronous callbacks, python polling of lisp, multiple python processes#36

Open
ajberkley wants to merge 5 commits into
digikar99:masterfrom
ajberkley:master
Open

Add support for asynchronous callbacks, python polling of lisp, multiple python processes#36
ajberkley wants to merge 5 commits into
digikar99:masterfrom
ajberkley:master

Conversation

@ajberkley
Copy link
Copy Markdown

This is obviously a pretty big change, in particular the multiple python processes one. It passes all the py4cl2-tests though!

I'm not sure how the original code passed all the tests, though --- the with-python-output relied on timing alone which doesn't work perfectly... even after flushing stuff on both sides, there's still time for the data to not reach the lisp side before we exit... relying on a sleep call is not reliable. So I added something a bit more complex but it works 100% of the time.

I haven't started using this heavily, though, so there's absolutely no rush to work through merging this or looking through the changes. I'm just opening this PR so you know where I am. In my cl-matplotlib repo, you can see it running an animated graph updating from some python code while also sending callbacks into lisp as you press buttons which then callback into python to update the graph (the PyQt python side is fragile as I haven't put any time into it yet, so if you close the main window python dies horribly some amount of the time).

Below picture the sine wave is being animated from python code, the button is calling back into lisp and updating the plot.

image

@ajberkley
Copy link
Copy Markdown
Author

Heh, of course I made the PR with a parens error... sorry. Passes tests and actually compiles now.

…ple python processes

parens fix and indentation

fixes
@digikar99
Copy link
Copy Markdown
Owner

I tried it today. After incorporating a fix for iterate, I can confirm that most tests pass. Some still fail. But this looks too large a change for a single commit. I will probably sleep on it for some time and commit it part by part.

Thanks for your work!

@ajberkley
Copy link
Copy Markdown
Author

Added a fix for deleting the numpy pickle files which I had broken. Is there a reason we don't just write the pickled format across the stream to the python process? It would avoid this complexity...

@ajberkley
Copy link
Copy Markdown
Author

For tests, is it that they are not passing on other implementations? I only tried with sbcl... I can run with other ones if that is the issue --- since the CI test runner seems broken I can't learn it from there.

@digikar99
Copy link
Copy Markdown
Owner

Added a fix for deleting the numpy pickle files which I had broken. Is there a reason we don't just write the pickled format across the stream to the python process? It would avoid this complexity...

I suspect the way things are the way they are is that the approach of creating a file is what occurred to me back then. I think the secondary reason is that stdin/stdout/stderr are text streams rather than binary. If numpy is present, one could perhaps create and maintain another binary stream. (Feel free to suggest other approaches!)

For tests, is it that they are not passing on other implementations? I only tried with sbcl... I can run with other ones if that is the issue --- since the CI test runner seems broken I can't learn it from there.

With ba09cca, the CI works again. sbcl is fine as the minimal target. Once it works on sbcl, we can test on ccl, and possibly ecl.

Looking at the latest CI, the tests are passing. Pystart/stop might require some minor fixes to make it to work on repeated calls without raising the python error "threads can only be started once". For failure of other tests, it is likely some problem on my local setup.

@digikar99
Copy link
Copy Markdown
Owner

I started gui-apps branch to incorporate the fixes and changes from this PR.

Based on the things I looked at so far, mainly python-process.lisp:

  • The failing test is MULTIPLE-LISP-PYTHON-THREADS.

  • Do we need to keep the instance of python around after we have stopped the process? In particular, can pystop be simplified? If the python object can simply be discarded, it might also be possible to simplify pystart a bit.

  • Regardingwith-python-output. Was semaphore a wrong tool for the job there? I feel it could be cleaner, but I will need to experiment a bit.

@ajberkley
Copy link
Copy Markdown
Author

  • Do we need to keep the instance of python around after we have stopped the process? In particular, can pystop be simplified? If the python object can simply be discarded, it might also be possible to simplify pystart a bit.

I agree, you are right that it is simpler to just mark it invalid and shut it down and not reuses it.

  • Regardingwith-python-output. Was semaphore a wrong tool for the job there? I feel it could be cleaner, but I will need to experiment a bit.

A semaphore is fine but it communicates the wrong idea. You have one resource and only one thread can use it at a time, so a mutex is clearer.

@ajberkley
Copy link
Copy Markdown
Author

* The failing test is [MULTIPLE-LISP-PYTHON-THREADS](https://github.com/digikar99/py4cl2-tests/blob/334231f339b3e1fb37b3868c26dd45aebc18ccf6/tests.lisp#L132-L161).

]

This is because the test is wrong. The (let ((i i)) ...) needs to be outside the (lambda ()). I'm happy to discuss further if this isn't clear :) Threading is tricky business!

This is the correct version:
image

@digikar99 digikar99 mentioned this pull request Apr 30, 2026
@digikar99
Copy link
Copy Markdown
Owner

Thanks! I have fixed the test: digikar99/py4cl2-tests@abed9ff

About with-python-output: I ended up using io.StringIO and avoiding any need for synchronization altogether! (c0545be)

I'm continuing the refactor in https://github.com/digikar99/py4cl2/pull/37/commits. Let me know if you are okay with the authorships!

I can then base the main changes concerning the gui apps onto the refactor.

@digikar99
Copy link
Copy Markdown
Owner

I wonder if the purpose of the timing code in src/callpython.lisp may be served by the profiling capabilities of SLIME/implementation. For example, SBCL has both the deterministic profiler as well as the statistical profiler. For the task at hand -- measuring the performance of raw-py, the deterministic profiler seems sufficient. Section 3.12 of the SLIME User Manual achieves the same thing -- particularly slime-toggle-profile-fdefinition slime-profile-report slime-profile-reset.

@digikar99
Copy link
Copy Markdown
Owner

digikar99 commented May 24, 2026

More questions:

  1. Why do we need two locks: raw-py lock as well as the interaction-lock?
  2. Why does the error need to be returned as a thunk? EDIT: Understood, you want to avoid interrupting the interaction-thread.

Also, the way this is implemented, if there is an error -- say PyQt6 is not installed, or some syntax error, or some future error -- during the execution of

(raw-py-exec/no-return "import PyQt6_example; PyQt6_example.start_app(try_process_message);")

then it'd appear as a response to (pyeval "1 + 1") or some future command. I'd guess the similar issue -- of not being able to figure out whether some error stems from the current input or the running GUI app -- occurs during the execution of python in the terminal or jupyter notebook. However, there, one can still rely on contingency information to locate the source of error -- if there is an error with the GUI, it'd appear immediately, rather than after I input another command. EDIT: This looks solvable. Instead of passing the error as a thunk to python-interaction-results, use bt:interrupt-thread to transfer the error to the main thread. I also have some working code, but I need to organize it a bit.

@digikar99
Copy link
Copy Markdown
Owner

gui-apps branch now has the major changes. Particularly, the commit bf5541b

It still deadlocks, which I will fix in the upcoming days.

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 this pull request may close these issues.

3 participants