WIP: fastapi inference server implementation #73
Conversation
che85
force-pushed
the
remote_server
branch
from
a3039de to
c2aef6e
Compare
- model database class that handles all local models including their download and/or deletion - extracted functions into utils module - fastapi server can be run from Slicer directly or from commandline - make sure loaded terminologies are searched
che85
force-pushed
the
remote_server
branch
from
c2aef6e to
a088ebb
Compare
Signed-off-by: Christian Herz <[email protected]>
| packageName = "torch" | ||
| if not self._checkModuleInstalled(packageName): | ||
| logger.info("PyTorch Python package is required. Installing... (it may take several minutes)") | ||
| install(packageName) |
There was a problem hiding this comment.
This only works on macOS. Installation command depends on operating system, hardware, and drivers - see the table here: https://pytorch.org/get-started/locally/ To automate it, you need to use a package like light-the-torch. Even with that it is not completely trivial, that's why we have the Slicer pytorch extension. Please always use that extension for installing pytorch.
There was a problem hiding this comment.
This may have been misleading then. The dependency_handler's task is to differentiate between LocalPythonDependencies (running fastapi server locally) vs SlicerPythonDependencies (running webserver within 3D Slicer).
There was a problem hiding this comment.
Is PyTorch always running in Slicer's Python environment and installed by SlicerPytorch extension?
There was a problem hiding this comment.
When server is started from Slicer, SlicerPython will be used. If server is started outside of Slicer, a local environment is required.
There was a problem hiding this comment.
PyTorch is installed using the SlicerPytorch extension.
There was a problem hiding this comment.
Actually, let me check and I will get back to you
There was a problem hiding this comment.
@lassoan I made a few changes. When starting the server from Slicer, availability of PyTorch extension will be checked. If successful, other Python packages (monai) will be checked and installed.
As mentioned before, the server can be used independently from Slicer by just running uvicorn from the commandline.
| @@ -0,0 +1,130 @@ | |||
| import sys | |||
There was a problem hiding this comment.
I'm not sure if we need this new custom Python dependency installer.
-
There are efforts to move towards more standard way of declaring Python dependencies in Slicer Python scripted modules, while still allowing lazy installation and import. See Allow scripted modules to declare and lazily install pip requirements Slicer/Slicer#7707 and Improving Support for Python Package Dependencies in Slicer Extensions Slicer/Slicer#7171. Therefore, I would not invest time into an alternative way of installing dependencies for one specific extension, but rather spend that time with improving and switching to this new approach.
-
We could add Slicer-specific improvements that are visible to the user. But I don't see such improvements in this class (it seems pretty much the same functionality as before with slightly different design). For example, if we spend time with improving dependency installation then we could show progress information and errors in a popup window during pip install so that the user knows that something is happening and he has to wait.
If this dependency handler was needed for the uvicorn server then it should be moved there, to make it clear that it is only for that special case.
There was a problem hiding this comment.
As long as the Slicer core efforts are not integrated, I am leaning toward using the current version.
- requesting server from Slicer will use SlicerPythonDependencies and check for PyTorch extension prior to server start. Other modules (e.g. monai) will also be installed prior to server start. - using fastapi from commandline will use NonSlicerPythonDependencies.
… check for PyTorch extension
|
Thanks for the updates. I'm reviewing the changes now. Is there any additional changes that you plan to make before this branch is ready for integration? |
|
I've pushed a few minor suggestions instead of commenting on them.
When the inference is performed locally then there is no progress information in the GUI (segmentation completes succesfully). It should be fixed. Another small thing: when starting a local segmentation server, before installing all the dependencies, it would be nice to tell the user that dependencies need to be installed that may take several minutes and let the user decide if he is OK with that or would like to cancel instead. |
|
In my opinion, the information displayed when running locally or remotely is insufficient as there is barely any feedback provided about the ongoing process. |
|
@lassoan I will experiement with the suggestions you made RE BackgroundProcess and LocalInference, InferenceServer. |
It may depend on the hardware and model. On my computer with a GPU all segmentations complete within 1-2 minutes, with a log message printed every couple of seconds (the longest wait between two messages is maybe 10 seconds). That said, the messages are good for showing that something is happening, but they do not give a good indication of how much longer the segmentation will take. It could be nice to implement overall progress reporting (tqdm reporting is done during inference, but that usually takes a couple of seconds, so again not much info about the overall progress). I've added a separate issue for tracking this feature request: #81 |
- extracted SegmentationProcessInfo dataclass instead of using dict
…ss BackgroundProcess
|
@lassoan I made the changes you requested. Now using a parent class Let me know what you think and we can go from there. |
|
These changes are great! If the segmentation works well at least locally then we can merge this. Two things to fix before merging this:
The server does not log anything even though log to console and GUI are enabled. Minor GUI tweaks:
|
|
@lassoan did you check the server logs when trying to run inference with it? Wondering what happened there since it works for me.... |
|
No logs are displayed, even if I click log to GUI. Do you see any logs when using local or server inference? |
|
Not seeing any logs in the UI, but Slicer's error log window. Everything works on Linux.
I will work on the "Log to GUI". Do you want to see the actual log information there or also just some custom statements? |
|
@lassoan I will also try on the Windows server we intended for the SimplifiedViewer. |
|
When doing local (no server) inference: logs are visible in the application log window. Just need to fix logging to the GUI. When doing inference on server: Application log of Slicer that launched the server: Application log on the client side: The server log does not contain any information about a client trying to connect. I've turned off the firewall, but did not make any difference. Server connection succeeded (and opening |
|
I just installed everything on the Windows and can start a server which I can connect to from my local MBP. I can retrieve model information, but when trying to run inference, I get the same error after some timeout. No signs of an attempted connection on the server-side. I will try with uvicorn directly. |
|
Let me know if you get stuck and then I can try to debug it. |
|
@lassoan I committed a fix for running the server on Windows. Maybe you can test it locally on your machine and confirm. I am going to work on the progress reporting now. |
- separating out clearOutputFolder (for inference) and temp model download
|
@lassoan Ready to test. Let me know what you think about the progressbar. A few more thoughts when running remotely. Sometimes, the files to be sent to the server are huge. It may be good to explore more user feedback for uploading/running inference and not having the main loop blocked for the whole process. |
Add "remote processing" checkbox and collapse local server section by default.
- shell was causing issues on machines with conda initialization at shell startup
|
@lassoan I made the requested changes. Improved error reporting on the client side and limiting the number of inference requests to 5/minute. Let me know what you think. |
|
Thank you, I'll test this and let you know! |
|
Thanks for the updates, it is getting really close to be ready. Just a few small things to fix before integration:
We would need to see something like this:
I've fixed a path-related bug that broke things on Windows and pushed it to this branch. |
|
I've tested it and progress reporting now works well for local inference. Only two tasks remain now:
|
|
Will work on it in a bit. Helping Nicolas was something right now. |
Make Remote processing button state persistent. Disable model selection and Apply button if remote processing is enabled but not connected to a server yet.
|
I've implemented persistency for the remote processing button (last state stored in QSettings) + disabling the model selection and Apply button if not connected to server. I've submitted an issue for sending processing output from the server to the client: #85 |
Inference server based on fastapi that can be started from the command line or from 3D Slicer's MONAIAuto3DSeg module
Untitled.mov
addresses #30