Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestions to update HD-BM #4

Closed
Joeycho opened this issue Apr 9, 2024 · 5 comments
Closed

Suggestions to update HD-BM #4

Joeycho opened this issue Apr 9, 2024 · 5 comments
Assignees

Comments

@Joeycho
Copy link
Member

Joeycho commented Apr 9, 2024

Hi Tassilo,

I'm Joey who recently joined Philipp's group as a PhD student.

I started to use HD-BM to evaluate it on other public datasets, had some issues with HD-BM and came up with the following suggestions:

  1. Add PyTorch reinstallation guide, similar as nnUNet, but with the specific version (?) of PyTorch.

Error: no valid convolution algorithms available in CuDNN
In my case I solved this by running: conda install pytorch==1.10.2 pytorch-cuda=12 -c pytorch -c nvidia

  1. Python version upgrade is required (3.6 -> 3.9 (or newer)). I confronted some package issues (scikit-build, and SimpleITK).
    scikit-build (0.17.6) SimpleITK (2.3.1) could not be installed with Python 3.6. They were necessary to resolve the following issue:

MIC-DKFZ/HD-BET#25

  1. Maybe add description about the predicted labels in ReadME file (Label 1: NEE, Label 2: CE)

  2. Probably include evaluate_folder function as well, if the ground truth files are available. What I did was 1) convert plans.pkl -> plans.json file 2) use nnUNetV2 evaludate_folder function. Do you think.. dataset and plan json files are still necessary if user only has prediction and ground truth segmentation files?

I will be happy to make PR if that sounds better for you.

@TaWald
Copy link
Contributor

TaWald commented Apr 10, 2024

Hey Joey,

thanks for posting this issue.

Add PyTorch reinstallation guide, similar as nnUNet, but with the specific version (?) of PyTorch.

I remember that the installation worked out-of-the-box 2 years ago, but I will check in again if it runs through with newer python versions and the newer Scikit and SimpleITK versions.

Not quite sure why you got the pytorch error, as by now torch comes with pre-compiled CuDNN and shouldn't have to be installed with conda.

ReadME file (Label 1: NEE, Label 2: CE)

Sure, I will include it.

Evaluate Folder

I will add this function call to the current ones. As you probably notices the nnU-Net only provides case-wise DICE values, which is why I used my own inferencing script, to included metastasis-wise instances, but if you are interested in the default nnU-Net eval it's an easy include.

@Joeycho
Copy link
Member Author

Joeycho commented Apr 10, 2024

Thanks for your response, Tassilo.

I see.. if I remember correctly, pip install (current way) tried to install pytorch 1.10.2 cpu version (without Cuda), and I got the mentioned error.

Sure, if you already have a plan to include your own script, I prefer to wait. No problem.

@TaWald
Copy link
Contributor

TaWald commented Apr 10, 2024

Sure, if you already have a plan to include your own script, I prefer to wait. No problem.

I currently do not intend to include the instance eval script, I rather would include the nnU-Net evaluation of semantic segmentation for convenience. Creating instances from segmentations is something I much rather not have in this repository. It will be it's own repository at some time.

@TaWald
Copy link
Contributor

TaWald commented Apr 10, 2024

I added a few more entrypoints that allow you to predict and evaluate in one go. So in case you want to test that out it would be awesome.

Also I could not reproduce the torch issue for any python version.
I tested py3.6 up till py3.12. The only issue I encountered was SimpleITK versioning which I hope should be fixed with the dependencies I added to the setup.py

@Joeycho
Copy link
Member Author

Joeycho commented Apr 21, 2024

@TaWald

Thanks for the update, Tassilo. I checked the setup.py and it looks good to me.

@Joeycho Joeycho closed this as completed Apr 21, 2024
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

No branches or pull requests

2 participants