Rewrite integrators script interface as Python code #4713
Conversation
The nogil attribute is required to make the integrator run smoothly with threads (e.g. from the visualizer). Functions marked with the nogil attribute can throw C++ exceptions since Cython 0.19. The Cython functions that handle signals can be replaced with the `signal.raise_signal()` function introduced in Python 3.8.
The old test would send the interrupt signal before the script had a chance to enter the integration loop, and thus the custom signal handler was never tested. The new version waits for the subprocess to write it's ready for the interrupt signal, and the traceback is checked to confirm the signal was re-raised by the script interface.
There was a problem hiding this comment.
There is no code coverage in the core for the signal handling code, probably because the interrupt signal interferes with the profiler. But the signal error message is actually tested in the python testcase now. Also, another benefit of no longer exposing the integrator internal state to Cython is that we can encapsulate its 9 global variables into a single struct.
For a proof-of-concept, see integrate.cpp in c933b1b. I will not work further on it for now, because it is not entirely satisfactory, e.g. some of the core functionality like ICC cannot get a pointer to it via function arguments without a significant refactoring. Also, we need to define what is supposed to go in the integrator struct once we enable per-particle integration. And more urgently, we need to get the observable framework to run in parallel to help further simplify the integrator code.
| cdef extern from "integrate.hpp": | ||
| double get_time_step() | ||
|
|
There was a problem hiding this comment.
Is this necessary, because integrate.pyx is no longer a cython file? Don't we have a better place to access the time_step?
There was a problem hiding this comment.
Unfortunately, it is still necessary in the python branch, because the old LB implementation needs it. That's the last consumer, so this will be gone in the walberla branch.
Fixes #4712
Description of changes: