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

Reworking nnunetv2 predictor #7069

Draft
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

JupiLogy
Copy link
Contributor

@JupiLogy JupiLogy commented Sep 30, 2023

Fixes #7013 .

Description

nnunetv2 version 2.2 introduced breaking changes, so now predict_from_raw_data is no longer a function. Following the changes between their v2.1 and v2.2 I tried to update the nnunetv2 runner.

Haven't documented the changes, not sure if it's required?

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Specific tests passed locally by running python -m tests.test_integration_nnunetv2_runner.

JupiLogy and others added 2 commits September 30, 2023 13:51
@JupiLogy
Copy link
Contributor Author

JupiLogy commented Sep 30, 2023

BTW I ran the test that was causing the issue

***All results:***
nnUNetTrainer_1epoch__nnUNetPlans__3d_fullres: 0.6849226238622849

*Best*: nnUNetTrainer_1epoch__nnUNetPlans__3d_fullres: 0.6849226238622849

***Determining postprocessing for best model/ensemble***
Killed

I didn't kill it but I dunno what the outcome was expected to be. Either way the import error is not there any more :D Let me know if you want any more logs to be sure about the performance or whatever. Last time I tried to run the full coverage tests, though, my PC crashed so I'd rather not try that again (sorry)!!

@JupiLogy JupiLogy force-pushed the 7013nnunetv2-predict branch from ff4f3db to ffcf5e3 Compare September 30, 2023 15:18
@JupiLogy
Copy link
Contributor Author

JupiLogy commented Sep 30, 2023

Also I've now signed both the commits. So I'm not sure why it's not passing the DCO.

@JupiLogy JupiLogy marked this pull request as ready for review September 30, 2023 15:22
@wyli
Copy link
Contributor

wyli commented Sep 30, 2023

/build

use_gaussian=use_gaussian,
use_mirroring=use_mirroring,
perform_everything_on_gpu=perform_everything_on_gpu,
verbose=verbose,
)
predictor.initialise_from_trained_model_folder(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for lookint into the issue, this currently generates an error AttributeError: 'nnUNetPredictor' object has no attribute 'initialise_from_trained_model_folder'. Did you mean: 'initialize_from_trained_model_folder'?

I think you can test within a docker container (docker run --ipc=host --net=host --gpus all -ti --rm projectmonai/monai):

cd /tmp
git clone --depth 1 --branch 7013nnunetv2-predict --single-branch https://github.com/JupiLogy/MONAI.git
python -m pip install git+https://github.com/julien-blanchon/hiddenlayer.git
python -m pip install nnunetv2
cd MONAI/
python -m tests.test_integration_nnunetv2_runner

@JupiLogy
Copy link
Contributor Author

JupiLogy commented Oct 2, 2023

I'm not sure how to make this PR a draft again...

But anyway, Note to self sort of, here's the latest error message.

======================================================================
ERROR: test_nnunetv2runner (__main__.TestnnUNetV2Runner.test_nnunetv2runner)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/me/MONAI/tests/test_integration_nnunetv2_runner.py", line 88, in test_nnunetv2runner
    runner.find_best_configuration(configs="3d_fullres")
  File "/home/me/MONAI/monai/apps/nnunet/nnunetv2_runner.py", line 760, in find_best_configuration
    find_best_configuration(
  File "/home/me/.conda/envs/monai/lib/python3.11/site-packages/nnunetv2/evaluation/find_best_configuration.py", line 91, in find_best_configuration
    allowed_trained_models = filter_available_models(deepcopy(allowed_trained_models), dataset_name_or_id)
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/me/.conda/envs/monai/lib/python3.11/site-packages/nnunetv2/evaluation/find_best_configuration.py", line 43, in filter_available_models
    raise RuntimeError(f"Trained model {trained_model} does not have an output folder. "
RuntimeError: Trained model {'plans': 'nnUNetPlans', 'configuration': '3d_fullres', 'trainer': 'nnUNetTrainer_1epoch'} does not have an output folder. Expected: ./work_dir/nnUNet_trained_models/Dataset001_dataroot/nnUNetTrainer_1epoch__nnUNetPlans__3d_fullres. Please run the training for this model! (don't forget the --npz flag if you want to ensemble multiple configurations)

----------------------------------------------------------------------

@JupiLogy JupiLogy marked this pull request as draft October 2, 2023 08:26
@JupiLogy
Copy link
Contributor Author

JupiLogy commented Oct 2, 2023

Seems like it's creating a nnunetv2 folder, but looking for an nnunet folder...

Signed-off-by: jupilogy <[email protected]>
@JupiLogy
Copy link
Contributor Author

JupiLogy commented Oct 2, 2023

I've been delving into the nnunetv2 code all day and it seems to me like it's not actually putting any logs or models or anything into the trained_models folder. Everything in the documentation suggests that it does, however nowhere in the code can I see it doing that during the train loop. Fed up for now :( I wonder if it's because I'm not running it on docker? Please would you be able to test it and see if you get the same result as me @wyli ? 😭 The error is the exact as the last one I posted, but with correct folder names this time...

@JupiLogy
Copy link
Contributor Author

JupiLogy commented Oct 2, 2023

If it is a problem with nnunet I'll open an issue there

@wyli
Copy link
Contributor

wyli commented Oct 2, 2023

I don't currently have time to look into those API changes... perhaps @dongyang0122 can help

@JupiLogy
Copy link
Contributor Author

JupiLogy commented Oct 5, 2023

Update, I got access to a machine with Docker, but the GPUs are too old to work with the monai docker. :(

Edit to update more: I got it working on a Docker, and it doesn't have the same folder issue there. Note to self: Now it's just not making the "validation" folder for some reason.

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.

test_nnunetv2runner
2 participants