Skip to content

Add internal server option to avoid asking users to set up a server manually#95

Open
lassoan wants to merge 2 commits into
coendevente:mainfrom
lassoan:add-internal-server
Open

Add internal server option to avoid asking users to set up a server manually#95
lassoan wants to merge 2 commits into
coendevente:mainfrom
lassoan:add-internal-server

Conversation

@lassoan
Copy link
Copy Markdown
Contributor

@lassoan lassoan commented May 23, 2026

This pull request automatically sets up and starts/stops the nnInteractive server and all its dependencies on compatible computers. The user can simply install the Slicer extension and start segmenting.

lassoan added 2 commits May 23, 2026 12:13
On Windows and Linux, user no longer has to set up anything manually.
The server is automatically installed in 3D Slicer's Python environment and started when needed.
DOWNLOAD_DIR now defaults to ~/.nninteractive_weights instead of a relative
path that resolved inside the installation folder.

This saves storage space (when multiple Slicer instances are installed), shortens reinstallation time, and allows installation in the future on macOS.
This is the pattern that is used by other extensions, such as TotalSegmentator and MONAIAuto3DSeg.

PromptManager cretion is deferred until the FastAPI startup event so the new --weights-dir
CLI argument is applied before weights are downloaded.
@lassoan lassoan changed the title Add internal server Add internal server option to avoid asking users to set up a server manually May 23, 2026
@muratmaga
Copy link
Copy Markdown
Contributor

muratmaga commented May 23, 2026

Tested on Linux (NVIDIA GPU, Slicer Preview) and hit a chain of subprocess crashes on first use. After installing the NNUNet extension via Extensions Manager and clicking a prompt:

File "…/server/nninteractive_slicer_server/main.py", line 10, in <module>
    import torch
ModuleNotFoundError: No module named 'torch'

After fixing torch manually, retrying produced:

File "…/server/nninteractive_slicer_server/main.py", line 11, in <module>
    import uvicorn
ModuleNotFoundError: No module named 'uvicorn'

Four issues I think are worth addressing — all variations of the same root cause (the dep-install chain isn't atomic and isn't verified before subprocess.Popen):

  1. No verification that torch actually imports before launching the server. _install_server_launching_dependencies() trusts the return value of SlicerNNUNetLib.InstallLogic().setupPythonRequirements() and never re-checks that torch is importable. Depending on the NNUNet extension version / whether the user confirms its dialog, that call can return without having installed torch in Slicer's Python — and then subprocess.Popen([sys.executable, .../main.py]) crashes immediately with the opaque ModuleNotFoundError above, surfaced only in the Server output panel.

    A one-line guard after _installNNUNetIfNeeded() would turn this into an actionable error:

    if importlib.util.find_spec("torch") is None:
        raise RuntimeError("torch was not installed by NNUNet's setupPythonRequirements(); "
                           "install it manually via slicer.util.pip_install(...) and retry.")
  2. The pip-install loop is not failure-atomic. If any package in _install_server_launching_dependencies() fails (e.g. nnInteractive>=1.1.5 failing because torch isn't there yet), slicer.util.pip_install raises and the loop exits before reaching subsequent packages (uvicorn, fastapi, xxhash, python-multipart, huggingface_hub). _server_launching_dependencies_installed stays False so a retry will try again — but the user has no signal that earlier packages were skipped, and the next failure surfaces only when the subprocess crashes on the next missing import. Consider catching, accumulating, and reporting all failures up front rather than failing fast in the middle of the list.

  3. The "Start Server" button bypasses install_dependencies() entirely.

    def on_start_stop_server_clicked(self):
        if self._is_internal_server_running():
            self.stop_internal_server()
        else:
            self.start_internal_server()       # no install_dependencies() call

    Only the ensure_synched decorator (clicking a prompt tool) installs deps. A user who clicks Start Server first — natural, given that the button is prominently in the Configuration tab — gets an immediate subprocess crash with no install attempt. Easy fix: call self.install_dependencies() at the top of on_start_stop_server_clicked (or start_internal_server), or disable the button until deps are verified.

  4. Per-package import guard before subprocess.Popen. Regardless of how installation is structured, start_internal_server() should loop over the same import list and assert each module is importable in Slicer's Python before spawning. A clear "Missing package: X — run pip_install(...) and retry" message beats the current pattern of letting the subprocess crash on whichever import happens to be on line N of main.py.

  5. SlicerNNUNetLib.InstallLogic is invoked without a clear UX contract in this flow. If the user dismisses its prompt, or it runs asynchronously, _install_server_launching_dependencies() proceeds anyway. Worth either (a) documenting in the README that users should run from SlicerNNUNetLib import InstallLogic; InstallLogic().setupPythonRequirements() from the Python console once before first use, or (b) adding a fallback to slicer.util.pip_install("torch …") with an appropriate CUDA index URL when the post-install torch check fails.

Workaround for anyone hitting this in the meantime — from the Slicer Python console:

from SlicerNNUNetLib import InstallLogic
InstallLogic().setupPythonRequirements()

import slicer
slicer.util.pip_install("nnInteractive>=1.1.5 uvicorn xxhash fastapi python-multipart huggingface_hub")

import torch, uvicorn, fastapi, nnInteractive    # sanity check

Then click a prompt tool (not Start Server) and the subprocess should come up cleanly.

Otherwise the feature works great — server bundling, log streaming into the Configuration tab, and the internal/external toggle all behave as advertised. Thanks for the work on this.

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.

2 participants