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

BAMF NNUnet Lung and Nodules (v2) #92

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

jithenece
Copy link

Pretrained model for 3D semantic image segmentation of the lung and lung nodules from ct scan

@jithenece jithenece marked this pull request as ready for review July 30, 2024 19:50
@jithenece
Copy link
Author

/test

sample:
  idc_version: "Data Release 3.0 September 24, 2021"
  data:
  - SeriesInstanceUID: 1.2.840.113654.2.55.302533635268177260202698156939540188162
    aws_url: s3://idc-open-data/f038cf01-a72a-44a7-88a1-d25d18f08ddf/*
    path: input_data

reference:
  url: https://drive.google.com/file/d/1j6VFntIlsssnzFlbOW1sziBwbpYeN_Ah/view?usp=sharing

@LennyN95
Copy link
Member

@jithenece Is this a separate version? It looks like it's the same model as in #78 if that is the case, please implement all changes in that PR (it's not yet merged).

@jithenece
Copy link
Author

@jithenece Is this a separate version? It looks like it's the same model as in #78 if that is the case, please implement all changes in that PR (it's not yet merged).

@LennyN95 PR #78 has been implemented as part of AIMI1 and it uses 2 different models to finally generate the lung and nodule segmentation. This PR #92 is part of AIMI2 and uses a single model to generate the same output but has better results. Hence I had raised this PR by incrementing version as 2.0.0. Please let me know your thoughts.

@jithenece
Copy link
Author

Both are listed in the zenodo

  • lung-ct.zip
  • lung2-ct.zip

@LennyN95
Copy link
Member

Great! Since we didn't freeze the v1 model yet as part of a MHub release (in fact, the other model is in testing stage and yet to be merged), this would effectively overwrite the v1 implementation. Do you think there is value to have v1 and v2 coexist on MHub? If yes, I suggest to implement this model as a separate model into MHub with a _v2 suffix to the model name.

@vanossj
Copy link
Contributor

vanossj commented Aug 20, 2024

I think its ok to close #78 and have this model as the only bamf lung nodule. The model in #78 can be run from a docker container as described in the repo, so if anyone whats to reproduce our results they can, even if its not on mhub.

Copy link
Member

@LennyN95 LennyN95 left a comment

Choose a reason for hiding this comment

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

Review is done with some minor comments (many overlap with those in the other PRs).

models/bamf_nnunet_ct_lungnodules/utils/NNUnetRunnerV2.py Outdated Show resolved Hide resolved
models/bamf_nnunet_ct_lungnodules/utils/NNUnetRunnerV2.py Outdated Show resolved Hide resolved
models/bamf_nnunet_ct_lungnodules/utils/NNUnetRunnerV2.py Outdated Show resolved Hide resolved
models/bamf_nnunet_ct_lungnodules/utils/NNUnetRunnerV2.py Outdated Show resolved Hide resolved
"format": "DICOM",
"modality": "CT",
"bodypartexamined": "LUNG",
"slicethickness": "10mm",
Copy link
Member

Choose a reason for hiding this comment

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

Just to confirm, the model suports CT scans with a slice thickness up to 1cm?

models/bamf_nnunet_ct_lungnodules/meta.json Outdated Show resolved Hide resolved
models/bamf_nnunet_ct_lungnodules/meta.json Outdated Show resolved Hide resolved
models/bamf_nnunet_ct_lungnodules/meta.json Outdated Show resolved Hide resolved
models/bamf_nnunet_ct_lungnodules/dockerfiles/Dockerfile Outdated Show resolved Hide resolved
models/bamf_nnunet_ct_lungnodules/config/default.yml Outdated Show resolved Hide resolved
@LennyN95 LennyN95 changed the title version 2 model for lung/nodules BAMF NNUnet Lung and Nodules (v2) Aug 20, 2024
@jithenece
Copy link
Author

Adding segmentation file to generate permanent link for input below
output.zip

` idc_version: 2.0
data:

  • SeriesInstanceUID: 1.3.6.1.4.1.14519.5.2.1.4320.5030.194126952887269510493034552293
    aws_url: s3://idc-open-data/cea2aa44-31aa-40ff-95ab-020baae065ff/*
    path: input_data
    `

@jithenece
Copy link
Author

jithenece commented Aug 27, 2024

/test

sample:
  idc_version: 2.0
  data:
  - SeriesInstanceUID: 1.3.6.1.4.1.14519.5.2.1.4320.5030.194126952887269510493034552293
    aws_url: s3://idc-open-data/cea2aa44-31aa-40ff-95ab-020baae065ff/*
    path: input_data

reference:
  url: https://github.com/user-attachments/files/16761594/output.zip

Test Results (24.08.29_15.15.30_AkoraywFNq)
id: e878cb29-fae1-49ae-aec2-6a8510c13eb2
date: '2024-08-29 15:59:21'
missing_files:
- 1.3.6.1.4.1.14519.5.2.1.4320.5030.194126952887269510493034552293/bamf_nnunet_ct_lungnodules.seg.dcm
summary:
  files_missing: 1
  files_extra: 0
  checks: {}
conclusion: false

@jithenece
Copy link
Author

/test

sample:
  idc_version: 2.0
  data:
  - SeriesInstanceUID: 1.3.6.1.4.1.14519.5.2.1.4320.5030.194126952887269510493034552293
    aws_url: s3://idc-open-data/cea2aa44-31aa-40ff-95ab-020baae065ff/*
    path: input_data

reference:
  url: https://github.com/user-attachments/files/16761594/output.zip

Test Results (24.08.29_15.15.30_AkoraywFNq)

Is it possible to get more verbose logs from the docker?

@LennyN95
Copy link
Member

Please note, we updated our base image. All mhub dependencies are now installed in a virtual environment under /app/.venv running Python 3.11. Python, virtual environment and dependencies are now managed with uv. If required, you can create custom virtual environments, e.g., uv venv -p 3.8 .venv38 and use uv pip install -p .venv38 packge-name to install dependencies and uv run -p .venv3.8 python script.py to run a python script.

We also simplified our test routine. Sample and reference data now have to be uploaded to Zenodo and provided in a mhub.tom file at the project root. The process how to create and provide these sample data is explained in the updated testing phase article of our documentation. Under doi.org/10.5281/zenodo.13785615 we provide sample data as a reference.

@jithenece
Copy link
Author

@LennyN95 I have updated this and other pending models too. Please let me know if any changes required.

@LennyN95
Copy link
Member

LennyN95 commented Oct 9, 2024

Static Badge

Test Results
id: 405eb452-2a80-494b-b341-40502d135cd3
name: MHub Test Report (default)
date: '2024-10-09 11:18:27'
missing_files:
- 1.3.6.1.4.1.14519.5.2.1.4320.5030.194126952887269510493034552293/bamf_nnunet_ct_lungnodules.seg.dcm
summary:
  files_missing: 1
  files_extra: 0
  checks: {}
conclusion: false

@LennyN95
Copy link
Member

LennyN95 commented Oct 9, 2024

The NNUnetRunnerV2 module fails with FileNotFoundError: [Errno 2] No such file or directory: '/app/tmp/9402fb71-3026-4da8-9c19-f660e64f5cc2/VOLUME_001.nii.gz'.

@LennyN95
Copy link
Member

@jithenece I see you fixed the version of nnunetv2==2.0 - are there any incompatibilities? We are currently working with the nnunet maintainers on a feature to disable multiprocessing during inference (see MIC-DKFZ/nnUNet#2556). Please let me know if switching to the latest version would cause any problems.

@LennyN95
Copy link
Member

LennyN95 commented Dec 2, 2024

@jithenece in addition to my previous post, @FJDorfner suggested the following setting (8ef8980) and it seems to work fine with the nnunetv2 version. Can you give us some brief feedback on how you tried this so we understand any edge cases. Thanks a lot!

@jithenece
Copy link
Author

@LennyN95 I have set it to 2.0 as it was trained using this version. Also to avoid any issues if the newer nnunetv2 has changes breaking this code base.

I tried above flags on nnunetv2-2.5.1 to check if the newer flags can work but got below error
nnUNetv2_predict: error: unrecognized arguments: --nproc_export 1 --nproc 1 --batchsize 1

Could you share the nnunetv2 version which should be used?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready for Testing
Development

Successfully merging this pull request may close these issues.

3 participants