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

#588 - Pediatric Abdominal CT Segmentation Bundle #589

Conversation

EmergentBehaviour
Copy link
Contributor

@EmergentBehaviour EmergentBehaviour commented May 31, 2024

Description

This is a submission for the MONAI Bundle of a Pediatric Abdominal CT Segmentation model from the Children's Artificial Intelligence Imaging Research (CAIIR) Center at Cincinnati Children's Hospital.

Metadata check passed, torchdynamo could not install / run during code-formatting script check. Open to any fixes or alternatives for code-formatting check / confirmation.
Please feel free to write any questions, comments or suggestions.

Status

Ready for Feedback / Publication

Please ensure all the checkboxes:

  • Codeformat tests passed locally by running ./runtests.sh --codeformat.
  • In-line docstrings updated.
  • Update version and changelog in metadata.json if changing an existing bundle.
  • Please ensure the naming rules in config files meet our requirements (please refer to: CONTRIBUTING.md).
  • Ensure versions of packages such as monai, pytorch and numpy are correct in metadata.json.
  • Descriptions should be consistent with the content, such as eval_metrics of the provided weights and TorchScript modules.
  • Files larger than 25MB are excluded and replaced by providing download links in large_files.yml.
  • Avoid using path that contains personal information within config files (such as use /home/your_name/ for "bundle_root").

@yiheng-wang-nv
Copy link
Collaborator

Hi @EmergentBehaviour , could you check the failed CI tests? Thanks!

@EmergentBehaviour
Copy link
Contributor Author

Hi @EmergentBehaviour , could you check the failed CI tests? Thanks!

Hello, I've since fixed the dependency versioning that caused the pre-merge to fail, however running some of the ci tests locally (like that contained within ci/run_regular_tests_cpu.sh) have errors with the checksum to download the latest bundles:

Traceback (most recent call last):
File "/workspace/model-zoo/ci/download_latest_bundle.py", line 44, in
download_latest_bundle(bundle_name, models_path, download_path)
File "/workspace/model-zoo/ci/download_latest_bundle.py", line 23, in download_latest_bundle
checksum = get_version_checksum(bundle_name=bundle_name, version=version, model_info_path=model_info_path)
File "/workspace/model-zoo/ci/utils.py", line 128, in get_version_checksum
return model_info_dict[f"{bundle_name}_v{version}"]["checksum"]
KeyError: '_v1.0.9'

Is there something I can do to fix this?

@EmergentBehaviour
Copy link
Contributor Author

Awaiting review unless there's a way to run blossom on my end.

@ericspod
Copy link
Member

hi @EmergentBehaviour sorry for the delay. I've run the other tests but can't invoke blossom, there are some things to fix still it seems but then @KumoLiu can trigger the blossom tests. Thanks!

@yiheng-wang-nv
Copy link
Collaborator

HI @EmergentBehaviour , as for the code format check, the current error is:

ERROR: /home/runner/work/model-zoo/model-zoo/models/pediatric_abdominal_ct_segmentation/scripts/compute_metric.py Imports are incorrectly sorted and/or formatted.

Could you use isort to adjust the import order to fix it? Thanks!

@yiheng-wang-nv
Copy link
Collaborator

As for the premerge cpu check, since your bundle does not have model.pt, could you add the bundle name into:
https://github.com/Project-MONAI/model-zoo/blob/dev/ci/bundle_custom_data.py#L26

You can also see the contributing guide: https://github.com/Project-MONAI/model-zoo/blob/dev/CONTRIBUTING.md#preferred-files-and-keys Thanks! @EmergentBehaviour

@EmergentBehaviour
Copy link
Contributor Author

Thank you for the response, and I have made the according modifications. However, this will still not pass the code format test as when the codebase is formatted to conform with isort, there are issues with the black standard according to the code formatting check, and vice versa, despite having used fixes that should in theory keep both standards compliant.

Let me know if there's anything I can do to ameliorate this or if you've run across this with the code formatting scripts.

@yiheng-wang-nv
Copy link
Collaborator

/black

@yiheng-wang-nv
Copy link
Collaborator

Thank you for the response, and I have made the according modifications. However, this will still not pass the code format test as when the codebase is formatted to conform with isort, there are issues with the black standard according to the code formatting check, and vice versa, despite having used fixes that should in theory keep both standards compliant.

Let me know if there's anything I can do to ameliorate this or if you've run across this with the code formatting scripts.

Hi @EmergentBehaviour , I pushed a commit and solves the isort and black issues. You can also do that locally via running:

./runtests.sh --autofix

There are still flake8 issues: https://github.com/Project-MONAI/model-zoo/actions/runs/9952935547/job/27495300660?pr=589#step:7:90
like:

/home/runner/work/model-zoo/model-zoo/models/pediatric_abdominal_ct_segmentation/scripts/compute_metric.py:39:1: F401 'argparse' imported but unused
/home/runner/work/model-zoo/model-zoo/models/pediatric_abdominal_ct_segmentation/scripts/compute_metric.py:41:1: F401 'glob.glob' imported but unused
/home/runner/work/model-zoo/model-zoo/models/pediatric_abdominal_ct_segmentation/scripts/compute_metric.py:43:1: F401 'nibabel as nib' imported but unused
/home/runner/work/model-zoo/model-zoo/models/pediatric_abdominal_ct_segmentation/scripts/compute_metric.py:44:1: F401 'numpy as np' imported but unused
/home/runner/work/model-zoo/model-zoo/models/pediatric_abdominal_ct_segmentation/scripts/compute_metric.py:47:1: F401 'monai.data.create_test_image_3d' imported but unused
/home/runner/work/model-zoo/model-zoo/models/pediatric_abdominal_ct_segmentation/scripts/compute_metric.py:50:1: F401 'monai.transforms.KeepLargestConnectedComponentd' imported but unused
/home/runner/work/model-zoo/model-zoo/models/pediatric_abdominal_ct_segmentation/scripts/compute_metric.py:50:1: F401 'monai.transforms.ScaleIntensityd' imported but unused
...

could you resolve them?

@EmergentBehaviour
Copy link
Contributor Author

EmergentBehaviour commented Aug 14, 2024

Just checking to see if blossom will pass. @yiheng-wang-nv

@yiheng-wang-nv
Copy link
Collaborator

/build

@yiheng-wang-nv
Copy link
Collaborator

/build

@EmergentBehaviour
Copy link
Contributor Author

Would there be a reason why blossom shows as failing in this page but successful on its action page? It looks like it skipped the post-processing step but ran it on the dev branch instead around the same time.

@yiheng-wang-nv
Copy link
Collaborator

Hi @EmergentBehaviour ,
CI failed when check the .ts file:

[2024-09-03T08:27:40.606Z] Traceback (most recent call last):
[2024-09-03T08:27:40.606Z]   File "/home/jenkins/agent/workspace/MONAI-zoo-premerge/ci/verify_bundle.py", line 355, in <module>
[2024-09-03T08:27:40.606Z]     verify(bundle, models_path, mode)
[2024-09-03T08:27:40.606Z]   File "/home/jenkins/agent/workspace/MONAI-zoo-premerge/ci/verify_bundle.py", line 343, in verify
[2024-09-03T08:27:40.606Z]     verify_torchscript(bundle_path, net_id, config_file, model_name, ts_name)
[2024-09-03T08:27:40.606Z]   File "/home/jenkins/agent/workspace/MONAI-zoo-premerge/ci/verify_bundle.py", line 219, in verify_torchscript
[2024-09-03T08:27:40.606Z]     torch.jit.load(ts_model_path)
[2024-09-03T08:27:40.606Z]   File "/home/jenkins/agent/workspace/MONAI-zoo-premerge/model_zoo_venv/lib/python3.10/site-packages/torch/jit/_serialization.py", line 162, in load
[2024-09-03T08:27:40.606Z]     cpp_module = torch._C.import_ir_module(cu, str(f), map_location, _extra_files, _restore_shapes)  # type: ignore[call-arg]
[2024-09-03T08:27:40.606Z] RuntimeError: 
[2024-09-03T08:27:40.606Z] Unknown type name '__torch__.torch.classes.tensorrt.Engine':
[2024-09-03T08:27:40.606Z]   File "code/__torch__/monai/networks/nets/dynunet.py", line 4
[2024-09-03T08:27:40.606Z]   __parameters__ = []
[2024-09-03T08:27:40.606Z]   __buffers__ = []
[2024-09-03T08:27:40.606Z]   __torch___monai_networks_nets_dynunet_DynUNet_trt_engine_0x558a0b5a81a0 : __torch__.torch.classes.tensorrt.Engine
[2024-09-03T08:27:40.606Z]                                                                             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ <--- HERE
[2024-09-03T08:27:40.606Z]   def forward(input_0: __torch__.monai.networks.nets.dynunet.DynUNet_trt,
[2024-09-03T08:27:40.606Z]     input_1: Tensor) -> Tensor:

I also run torch.jit.load locally and met the same issue. May I ask how to use this file?

@EmergentBehaviour
Copy link
Contributor Author

EmergentBehaviour commented Sep 5, 2024

@yiheng-wang-nv

The TensorRT model should be able to load in as-is or if not can be exported from the .pt file as described in the bundle's README and loaded from that new file. May I ask what version of TensorRT / any particular plugins this test is using?

I can alternatively remove the .ts model from the large files list and leave it as an option for users to export from the .pt if desired, as different users may be using different versions.

@yiheng-wang-nv
Copy link
Collaborator

@yiheng-wang-nv

The TensorRT model should be able to load in as-is or if not can be exported from the .pt file as described in the bundle's README and loaded from that new file. May I ask what version of TensorRT / any particular plugins this test is using?

I can alternatively remove the .ts model from the large files list and leave it as an option for users to export from the .pt if desired, as different users may be using different versions.

thanks, let me add a minor change to make the check optional

@yiheng-wang-nv yiheng-wang-nv enabled auto-merge (squash) September 6, 2024 02:23
@yiheng-wang-nv
Copy link
Collaborator

/build

@yiheng-wang-nv yiheng-wang-nv merged commit f9c2be4 into Project-MONAI:dev Sep 6, 2024
4 checks passed
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.

3 participants