BAMF - NNUnet Liver CT

[rahul99]

No description provided.

[rahul99]

@LennyN95 , this one is ready for review and merge :)

[LennyN95]

Hi Rahul,

Great! Here's a checklist of some points to be resolved (most of which transfer to the other open PRs, too):

• The DsegConverter should use the ROI instead of a custom JSON if the ROI is available in our SegDB (read here).
• Remove the config/meta.json if it is unnecessary or provide a statement about why it may be required (it needs to be significantly different from what we have in SegDB).
• Convert BamfProcessor to a proper module (read here). It is not a RunnerModule and hence should inherit from Module instead. Please use the @IOdecorators as explained.
• Remove /scripts as all MHub models are required to provide a default.yml entry point only (we support scripting within our framework but do not allow it in our models repository for standardization purposes)
• Remove all COPYand ADD instructions from the Dockerfile. We do not allow those.
• The Dockerfile Entrypoint must be ENTRYPOINT ["python3", "-m", "mhubio.run"]. The CMD -["--config", "/app/path/to/default.yml"] then must point to your default config.
• Provide a meta.json at your model's root folder (read here).

Once all points are resolved, please be prepared to provide us with two or three example cases (that is, an example image and a segmentation generated via your model) so that we can run the MHub implementation locally and validate the output against the provided delineation as a sanity check.

Best,
Leo.

[rahul99]

@LennyN95 , I am back to these repos and hoping to close them all in the next 1-2 days. For this PR, I am getting an error while using custom module:

Warning: One or more modules in the execution chain are not available. You may use a custom run script insead.
  - BamfProcessorRunner

Let me know if I missed anything. Once this is good to go, the other two MRs would be minimal work. Thanks!

[LennyN95]

Warning: One or more modules in the execution chain are not available. You may use a custom run script insead.
  - BamfProcessorRunner

I could not reproduce the issue you described. I did a test build and
Are you using the newest base image (mhubai/base:latest)?

If you run docker images | grep mhubai/base you should get 33f37ca2036e 4 weeks ago.
You can run docker pull mhubai/base:latest if you're image is older / different than that.

[rahul99]

HI @LennyN95 , thanks for the detailed comments and suggestions. I have resolved all of them and updated this MR.

Regarding the base image, I have the following:

REPOSITORY             TAG       IMAGE ID       CREATED          SIZE
bamf_nnunet_ct_liver   latest    6f611a5282db   13 minutes ago   11.2GB
mhubai/base            latest    a33ae30cec1b   22 hours ago     3.88GB

[rahul99]

I still get the same error:

root@3092005646cf:/app# mhub.run --config models/bamf_nnunet_ct_liver/config/default.yml
Config file overwritten by cli argument: models/bamf_nnunet_ct_liver/config/default.yml
 Warning: One or more modules in the execution chain are not available. You may use a custom run script insead.
  - BamfProcessorRunner

[LennyN95]

Hey @rahul99, did you re-build the image?

Please try this:

# pull the latest image again
docker pull mhubai/base:latest

# confirm you have the newest base image (d8382753d763)
docker images | grep mhubai/base

# change wd to the dockerfiles folder of the model
cd models/bamf_nnunet_ct_liver/dockerfiles

# build the model (pulling from your fork)
docker build -t dev/bamf_nnunet_ct_liver --build-arg MHUB_MODELS_REPO=https://github.com/bamf-health/mhub-models::bamf_nnunet_ct_liver --no-cache .

Then try running dev/bamf_nnunet_ct_liver.
It should work as it did for me ;)

[rahul99]

It works for me as well. Thanks for all the help Leo. I think we are all set now! :)

[LennyN95]

Great news! We still have some checks failing due to some missing fields in the meta.json.
Once they're resolved, we can move over to the testing phase.

For the test phase, we will require a public example to verify the model works correctly.

• Can you list one (or multiple) suitable IDC collections that should be supported by the model?
• Can you run the MHub implementation of the model on 2-3 example cases, and share the link to the examples with us as well as the generated output (untouched)?

[rahul99]

Hey @LennyN95 , the build was probably failing due to licence versus license spelling. Let me get that fixed

[rahul99]

@LennyN95 , apologies the following fields were still missing from meta.json:

- /summary/inputs/slicethickness
- /use/ethics
- /use/limitations

Updating them now...

[rahul99]

Updated the PR. Let me know if it goes through. I will prepare test data in the mean time

[rahul99]

@LennyN95 ,

the sanity check failed again with the following error:

---------------- CHECK FAILED ----------------
This PR violates one or more MHub compliance rules:
Model meta json is not compliant with the schema: 'license' is a required property ( see https://github.com/MHubAI/documentation/blob/main/documentation/mhub_models/model_json.md for more information)

It looks like its looking for license field. However, in this documentation, we refer to the field as licence

[LennyN95]

It looks like its looking for license field. However, in this documentation, we refer to the field as licence

Great catch @rahul99! Sorry for the confusion, I updated the documentation and we'll stick with the US spelling for now.

[rahul99]

@LennyN95 , I am getting the following error after latest changes:

  File "/app/models/bamf_nnunet_mr_liver/utils/BamfProcessorRunner.py", line 25, in BamfProcessorRunner
    @IO.Output('out_data', 'nrrd:mod=seg:processor=bamf', data='in_data', the="keep the two largest connected components of the segmentation and remove all other ones")
TypeError: Output() missing 1 required positional argument: 'dtype'

I think we are missing one more parameter in the decorator? I checked similar declaration in your modules (for example nnunet-runner) but couldn't find anything. Please advice :)

[LennyN95]

@rahul99 The second argument should be the path (I forgot to mention in my example, I'm sorry...).
The following should work:

@IO.Output('out_data', 'bamf_processed.nrrd', 'nrrd:mod=seg:processor=bamf', data='in_data', the="keep the two largest connected components of the segmentation and remove all other ones")

[rahul99]

/test

sample:
  idc_version: 17.0
  data:
  - SeriesInstanceUID: 1.3.6.1.4.1.14519.5.2.1.3344.4008.288401067690076379005498612621
    aws_url: s3://idc-open-data/722f2824-9c7a-4342-9b96-213b70a94768/*
    path: dicom

reference:
  url: https://www.dropbox.com/scl/fi/50ktdmzp91v5gqgl4ogv7/bamf_nnunet_ct_liver.zip?rlkey=qvya90xufkic5iebpoywl1g9j&dl=0

---

Test Results (24.03.28_17.30.31_kb1YE6N1mX)

id: bb9d29b3-cf32-4a9d-ba08-e6c18cad6bb5
date: '2024-03-28 17:53:33'
checked_files:
- file: bamf_nnunet_ct_liver.seg.dcm
  path: /app/test/src/1.3.6.1.4.1.14519.5.2.1.3344.4008.288401067690076379005498612621/bamf_nnunet_ct_liver.seg.dcm
  checks:
  - checker: DicomsegContentCheck
    notes:
    - label: Segment Count
      description: The number of segments identified in the inspected dicomseg file.
      info: 1
summary:
  files_missing: 0
  files_extra: 0
  checks:
    DicomsegContentCheck:
      files: 1
conclusion: true

[LennyN95]

@rahul99 could you upload 1-5 images for the model card that show the model in action? This is not a hard requirement, thus I moved on with the push. However, from a UX perspective, visuals add a great value to the model page and also help readers to quickly recognize a model (especially as the model card is made to look similar across all models, images can be a great help here).