Slicer / ExtensionsIndex

Slicer extensions index
https://github.com/Slicer/ExtensionsIndex/#slicer-extensions-index
66 stars 244 forks source link

2023.12.12: An extension displays a blocking popup related to CUDA installation #1991

Open jcfr opened 11 months ago

jcfr commented 11 months ago

image

lassoan commented 11 months ago

This is so wrong in several aspects:

We can temporarily remove the extension while the maintainers fix the issues.

jcfr commented 11 months ago

I will follow-up here once I identified the culprit.

jcfr commented 11 months ago

Here is the code leading the popup:

https://github.com/DCBIA-OrthoLab/SlicerDentalModelSeg/blob/00816c31c570e41a5d580e291519dbf8b541d95e/CrownSegmentation/CrownSegmentation.py#L216-L226

The extension was originally introduced through this pull request:

The following comment originally posted by @pieper in https://github.com/Slicer/ExtensionsIndex/issues/1867#issuecomment-1206808269 is still relevant:

Please in the readme be sure to point out that this code depends on CUDA. It looks like there's already a fallback to CPU mode so describing the tradeoffs in the readme would be helpful.

Longer term, you should see if you can rework the installation code to use SlicerPyTorch to get the right code installed.

Also it would help if you provide sample data. I see some in the repository so those should be referenced in the readme or even better registered with the SampleData module so it's easy for users to test the module.

In the same vein, it looks like you don't have any tests.

The code was originally contributed by @MathieuLeclercq and seems to be maintained by @GaelleLeroux with some input from @allemangD

jcfr commented 11 months ago

@GaelleLeroux With some guidance from us, would you have the bandwidth to help improve this extension ?

GaelleLeroux commented 11 months ago

@jcfr I'm currently trying to update this module. @juanprietob and @FlorianDAVAUX have released shapeaxi, a new library that includes teeth segmentation. I'm working on changing the code to use it instead of the previous code. @juanprietob, @FlorianDAVAUX do you know if it's still necessary to have a CUDA capable GPU to download and use dentalmodelseg in shapeaxi? If so, how can I improve the code to avoid it ?

jcfr commented 11 months ago

I'm working on changing the code to use it instead of the previous code

This is great. Thanks for working on this :pray:

released shapeaxi

See https://pypi.org/project/shapeaxi/

do you know if it's still necessary to have a CUDA capable GPU

Short answer is yes.

It looks like it depends on pytorch3d, this means that this extension could probably updated to depend on SlicerPyTorch^1

See also:

lassoan commented 11 months ago

It looks like it depends on pytorch3d, this means that this extension could probably updated to depend on SlicerPyTorch

pytorch3d is a one-man project. The developer uses linux and cuda and the package practically does not work in any other environment. pytorch3d uses pytorch and SlicerPyTorch would take care of the installation of pytorch, but that's not sufficient to make pytorch3d work.

It would be great if you could get rid of pytorch3d, because the dependency on pytorch3d makes the extension inaccessible for 99% of Slicer users (12% of Slicer users are on Linux, probably less than 10% have a CUDA-capable GPU).

GaelleLeroux commented 11 months ago

I've managed to use pytorch3d on Windows, so it's more accessible, but it still requires a CUDA-compatible GPU...

lassoan commented 11 months ago

It probably only worked because you are a developer and had a C++ compiler, CUDA SDK, etc. all set up on your computer.

GaelleLeroux commented 11 months ago

Yes, you're right, but we can't get rid of pytorch3d and I haven't found a way to install pytorch3d without CUDA. I think first of all this module needs a powerful computer to run properly, so maybe we can say it's a requirement to have a CUDA compatible GPU. I don't have any idea how to solve this problem... if you have one I would be interested to work on it.

lassoan commented 11 months ago

Making your software work on an average computer (Windows, macOS, Linux / CPU, CUDA, or Apple MPS backend) is essential for your software to make an impact beyond publishing papers. If users can easily try your software on their current system and see that it works (just at reduced quality or longer computation time) then they will use it and can decide to invest into getting a new computer with a strong discrete GPU to get results faster or higher quality.

I understand that it can be hard to change a dependency once you have built a whole lot of things on it and there is no high-quality drop-in replacement. Maybe you could reach out to pytorch3d developers to clarify their available resources, commitment, and long-term plans for the package.

GaelleLeroux commented 11 months ago

Thanks so much @lassoan for the response and for raising such relevant points. I acknowledge that the dependency on Pytorch3D and the requirement for high-performance graphics cards with CUDA capabilities does pose a significant challenge in terms of broadening our user base. At present, we do not have an immediate solution to modify this dependency, which is indeed a critical aspect of our consideration for future developments. When considering the computing infrastructure required for running AI tools, it's crucial to recognize that even robust modules like TotalSegmentator demand substantial computing power. For users interested in effectively utilizing AI tools, having a computer without CUDA capabilities significantly limits efficiency and performance. Consequently, a CUDA-enabled system is not just advantageous but increasingly necessary for engaging with advanced AI applications. I have brought @juanprietob, @bpaniagua, and @allemangD into this conversation, as they can provide more insights and future plans regarding the development of our surface mesh Shape Analysis modules, including DentalModelSeg, ALIIOS, ARegIOS, FlexReg, and ShapeAxi. Their perspectives will be invaluable in addressing the high-level planning and strategic directions we might take.

jamesobutler commented 11 months ago

In the context of the Slicer extensions index, an extension published through this method should probably at minimum have support for macOS or Windows (not windows subsystem for Linux (WSL)) as that will capture a significant user base. This would be for the sake of the Slicer community that extension functionality is cross platform and can be run by a reasonable end-user that doesn’t have to be a developer. There is often frustrations within the Slicer community when new AI tools don’t “just work”.

If an extension is unable to reach those expectations, I think it would be best for the extension to be self-published with distribution handled by those developers as they will likely be providing support anyways for specific dependencies and other setup. It appears SlicerDentalModelSeg is still in the research and development stage which hasn’t yet made things generally easy-to-use for the Slicer community although it may be fine for the relevant research lab.

lassoan commented 11 months ago

@GaelleLeroux Thanks for the consideration. Just a note on TotalSegmentator: one reason it is very widely used because it does not require a GPU, but it can run on CPU as quickly as on GPU but at half resolution; or it can provide full-resolution results in 30-40 minutes (about 10x longer time than computation takes on GPU). This works amazingly well, because users can verify in a few minutes that TotalSegmentator works well on their data and they can decide to either stop there (if they don't have more time or don't need better-quality results) or wait for 40 minutes for full-quality results. Therefore, lack of GPU becomes a small inconvenience in most use cases.

@jamesobutler I agree that it can be frustrating for users when they don't know that the extension is not at that maturity level that they expect. Requiring extensions to fulfill some requirements before allowing them to show up in the Extensions Manager would be a simple solution, but perhaps a better one would be to show the maturity level of the extension clearly in the Extensions Manager, maybe set default filter settings to only show well-tested, widely usable ones. I've added an issue to track this: https://github.com/Slicer/Slicer/issues/7474 (I thought we had an issue for this already but I could not find it)

GaelleLeroux commented 11 months ago

@lassoan @bpaniagua @allemangD @juanprietob Regarding the maturity level indication in the Extensions Manager and your suggestion of setting default filters, it will indeed help in managing user expectations effectively. However, I have a query related to our extension's visibility with these changes. Will users still be able to see and download our extensions, even if they are not categorized under the 'well-tested, widely usable' filter? It's important for us to ensure that our extension remains accessible to those who might benefit from it. Additionally, regarding system compatibility issues, we can include detailed system requirements in our extension's description and add a pop-up window for users, which would appear when first opening the extension. Would this approach be in line with the proposed modifications to the Extensions Manager?

lassoan commented 11 months ago

This Extensions Manager feature will probably take several months to implement and there areany higher-priority tasks, so it inot clear when development would start. But it's good that you have raised these questions.

GaelleLeroux commented 11 months ago

Thank you for your response, I understand that there are numerous higher-priority tasks at hand and that the development of this feature may take some time. Let me know if there is any way I can assist

jcfr commented 5 months ago

On June 21st, 2024, I just observed the following while remove connecting to the windows build machine: