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

Nnunet test #199

Merged
merged 13 commits into from
Aug 8, 2024
Merged

Nnunet test #199

merged 13 commits into from
Aug 8, 2024

Conversation

scarere
Copy link
Collaborator

@scarere scarere commented Jul 30, 2024

PR Type

[Feature | Fix | Documentation | Other ]

Short Description

This PR improves the functionality of nnUNetClient

  • Added a shutdown method to nnUNetClient to ensure processes spawned by dataloaders are terminated
  • Added nnunet_plans to properties returned by get_properties for nnUNetClient
    • If the plans are not provided in the server config, then the client generates them and sends them back to the server
    • This allows nnunet plans to be an optional parameter as the server can request a random client to generate them
  • Added the FlServerWithInitializer server class.
  • Added an NnUNetServer subclass of FlServerWithInitializer
    • uses the initialize hook method to request nnunet plans from a random client if the plans were not provided in the config
  • Added load_msd_data to fl4heath.utils.load_data to dynamically download and extract any MSD dataset
  • Added an nnunet example which requires no set up
    • Environment variables are dynamically set
    • Works with any MSD dataset and will download and prepare the dataset on the fly
  • fixed the startup messages in run_smoke_test

Tests Added

  • Added two smoke tests with the nnunet example. One for a 2d config and one for a 3d_fullres config
    • Uses the Task04_Hippocampus dataset which is the smallest of the MSD datasets and is downloaded on the fly
    • Does not check outputs, just ensures that the code runs without errors

@scarere
Copy link
Collaborator Author

scarere commented Jul 30, 2024

The nnunet smoke tests work locally for me, but none of the other tests do. I get tolerance errors on at least two of them but probably all of them. I tried checking out main and I still get the errors so I think this must be related to my setup somehow.

@scarere
Copy link
Collaborator Author

scarere commented Jul 30, 2024

Not sure why nnunet smoke tests are failing. Here is the error found in the logs

2024-07-30T19:28:01.6031749Z   File "/home/runner/work/FL4Health/FL4Health/.venv/lib/python3.9/site-packages/torch/utils/cpp_extension.py", line 28, in <module>
2024-07-30T19:28:01.6032383Z     from pkg_resources import packaging  # type: ignore[attr-defined]
2024-07-30T19:28:01.6032813Z torch._dynamo.exc.BackendCompilerFailed: backend='inductor' raised:
2024-07-30T19:28:01.6033547Z ImportError: cannot import name 'packaging' from 'pkg_resources' (/home/runner/work/FL4Health/FL4Health/.venv/lib/python3.9/site-packages/pkg_resources/__init__.py)

Copy link
Collaborator

@jewelltaylor jewelltaylor left a comment

Choose a reason for hiding this comment

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

Looks great! I will take one last pass once you have the chance to address a few of the small comments I had. But this is pretty much good to go from my POV

Copy link
Collaborator

@jewelltaylor jewelltaylor left a comment

Choose a reason for hiding this comment

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

LGTM!

@scarere scarere merged commit a5c1b1b into main Aug 8, 2024
6 checks passed
@scarere scarere deleted the nnunet_test branch August 8, 2024 17:05
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.

None yet

2 participants