730 retinal oct rpd segmentation #748

ybagdasa · 2025-04-29T22:32:24Z

Fixes # .

Description

This is a proposed segmentation model to add to the model zoo.

Status

Ready for review/work in progress.

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.
[ x] Please ensure the naming rules in config files meet our requirements (please refer to: CONTRIBUTING.md).
[x ] Ensure versions of packages such as monai, pytorch and numpy are correct in metadata.json.
[x ] Descriptions should be consistent with the content, such as eval_metrics of the provided weights and TorchScript modules.
[x ] Files larger than 25MB are excluded and replaced by providing download links in large_file.yml.
[ x] Avoid using path that contains personal information within config files (such as use /home/your_name/ for "bundle_root").

…files.yaml. Fixed case-sensitivity for .png/.PNG in dataset/data.py. Made confidence threshold an external argument.

…eparately.

updates.

…oot.

… instructions in readme.

ybagdasa · 2025-05-12T16:34:23Z

@yiheng-wang-nv here is my draft PR

yiheng-wang-nv · 2025-05-13T13:47:35Z

ci/install_scripts/install_retinalOCT_RPD_segmentation_dependency.sh

@@ -0,0 +1,3 @@
+# ensure that using evironment with python==3.9
+pip install -r ./docs/requirements.txt #includes monai
+python -m pip install 'git+https://github.com/facebookresearch/detectron2.git'


Hi @ybagdasa , could you fix the commit for the detectron2 installation?

@yiheng-wang-nv Thanks for considering the contribution. For the detectron2 installation, what is the command I should run to replicate the error you're getting? I know that the install works for python<3.10. Could this be the source of the issue for the ci install checks?

Hi @ybagdasa ,

For the comment of this line (python -m pip install 'git+https://github.com/facebookresearch/detectron2.git'),
based on https://github.com/Project-MONAI/model-zoo/pull/748/files#diff-6d26dc7c0d9fa894d66c944bd9d0a7c970f95371163c70631b8d6b0ed790fc81R19 detectron2 0.6 is used, thus here we can modify the install command into:

python -m pip install 'git+https://github.com/facebookresearch/detectron2.git@v0.6'

Otherwise, the command will install the latest commit, which may have incompatible issues in the future.

For the python version, can you confirm if python 3.9 is necessary?
I'm not 100% sure, but based on https://detectron2.readthedocs.io/en/latest/tutorials/install.html#requirements , it only requires to use python >= 3.7.

Currently, our repo uses python 3.10 in default for CI tests. If we do need python 3.9 for this bundle, I may consider some changes in the CI to support multiple python versions.

For the CI error: https://github.com/Project-MONAI/model-zoo/actions/runs/14742610327/job/42137993317?pr=748#step:7:510 Let me push a commit to your branch to fix it. Since detectron2 cannot simply be installed via pip install detectron2, we should consider some changes in:
https://github.com/Project-MONAI/model-zoo/blob/dev/ci/get_bundle_requirements.py#L54

Since I don't have the permission to push to your forked repo directly, I pushed the changes into: https://github.com/Project-MONAI/model-zoo/tree/730-retinalOCT_RPD_segmentation, could you help to merge it?
The branch also fixed the pre-commit ci errors.

Then we can see if there are other issues in the dependencies.

For the python version, can you confirm if python 3.9 is necessary? I'm not 100% sure, but based on https://detectron2.readthedocs.io/en/latest/tutorials/install.html#requirements , it only requires to use python >= 3.7.

Currently, our repo uses python 3.10 in default for CI tests. If we do need python 3.9 for this bundle, I may consider some changes in the CI to support multiple python versions.

My attempts to build the detectron2 wheel failed with python 3.10 and it unfortunately seems to be a common issue with their repo: facebookresearch/detectron2#5445

Thanks, let me enable py3.9 in the CI

yiheng-wang-nv · 2025-05-13T13:51:01Z

Hi @ybagdasa , thanks for the PR contribution. For pre-commit CI errors, could you fix them? The auto-push seems cannot work since the branch is pushed from an organization account.

yiheng-wang-nv · 2025-05-13T13:51:46Z

Format test can be fixed according to: https://github.com/Project-MONAI/model-zoo/actions/runs/14742610314/job/42137993243?pr=748#step:7:67

yiheng-wang-nv · 2025-05-13T13:52:24Z

I will look at this PR later this week

ybagdasa · 2025-05-13T19:01:07Z

Format test can be fixed according to: https://github.com/Project-MONAI/model-zoo/actions/runs/14742610314/job/42137993243?pr=748#step:7:67

@yiheng-wang-nv When running flake8 on the code it flags a lot of mixed-case naming conventions (N800 flags). A lot of the naming comes from the pycocotools api which used mixed-case naming for functions and variables. Having everything lowercase will probably hinder the readability of the code at this point. Is this a strict requirement or can I somehow skip it?

yiheng-wang-nv · 2025-05-16T03:50:38Z

ci/install_scripts/install_retinalOCT_RPD_segmentation_dependency.sh

@@ -0,0 +1,3 @@
+# ensure that using evironment with python==3.9
+pip install -r ./docs/requirements.txt #includes monai


We may not need the requirements.txt file in the bundle, it's content is included in the metadata.json.
Our CI can generate the requirements file directly via: https://github.com/Project-MONAI/model-zoo/blob/dev/ci/get_bundle_requirements.py#L54

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

yiheng-wang-nv · 2025-05-16T04:28:19Z

Format test can be fixed according to: https://github.com/Project-MONAI/model-zoo/actions/runs/14742610314/job/42137993243?pr=748#step:7:67

@yiheng-wang-nv When running flake8 on the code it flags a lot of mixed-case naming conventions (N800 flags). A lot of the naming comes from the pycocotools api which used mixed-case naming for functions and variables. Having everything lowercase will probably hinder the readability of the code at this point. Is this a strict requirement or can I somehow skip it?

Hi @ybagdasa , sorry I cannot see the error you mentioned, the error I saw is from isort, could you fix it first? Then we can see the other checks like black and flake8

…dical-ml/monai-model-zoo into 730-retinalOCT_RPD_segmentation

ybagdasa · 2025-05-16T18:30:52Z

@yiheng-wang-nv
I merged your changes with my fixes on my organization repo and gave you write privileges for the repo. I fixed all the formatting issues detected by runtests.sh except for the mixed-case ones from flake8. I also added torchvision to metadata.json since that is required.

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

yiheng-wang-nv · 2025-05-19T06:36:49Z

Hi @ybagdasa , thanks for the updates and providing me the permissions. I pushed a commit to fix the black and pre-commit issues. Now, our CI can reach to the flake8 errors: https://github.com/Project-MONAI/model-zoo/actions/runs/15105923819/job/42454628204?pr=748

I think we can skip N802, N803 and N806 errors, but others may need to be solved

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

yiheng-wang-nv · 2025-05-19T07:21:52Z

Hi @ybagdasa , thanks for the updates and providing me the permissions. I pushed a commit to fix the black and pre-commit issues. Now, our CI can reach to the flake8 errors: https://github.com/Project-MONAI/model-zoo/actions/runs/15105923819/job/42454628204?pr=748

I think we can skip N802, N803 and N806 errors, but others may need to be solved

Hi @ybagdasa , I updated, and could you help to resolve other issues? Like line too long (https://github.com/Project-MONAI/model-zoo/actions/runs/15106735909/job/42456940644?pr=748)

ybagdasa · 2025-05-19T23:02:11Z

Hi @ybagdasa , thanks for the updates and providing me the permissions. I pushed a commit to fix the black and pre-commit issues. Now, our CI can reach to the flake8 errors: https://github.com/Project-MONAI/model-zoo/actions/runs/15105923819/job/42454628204?pr=748
I think we can skip N802, N803 and N806 errors, but others may need to be solved

Hi @ybagdasa , I updated, and could you help to resolve other issues? Like line too long (https://github.com/Project-MONAI/model-zoo/actions/runs/15106735909/job/42456940644?pr=748)

@yiheng-wang-nv I was able to do ./runtests.sh up through pytype. The error popped up here:
[163/167] check models.maisi_ct_generative.scripts.rectified_flow

yiheng-wang-nv · 2025-05-26T03:56:07Z

Hi @ybagdasa , thanks for the updates and providing me the permissions. I pushed a commit to fix the black and pre-commit issues. Now, our CI can reach to the flake8 errors: https://github.com/Project-MONAI/model-zoo/actions/runs/15105923819/job/42454628204?pr=748
I think we can skip N802, N803 and N806 errors, but others may need to be solved

Hi @ybagdasa , I updated, and could you help to resolve other issues? Like line too long (https://github.com/Project-MONAI/model-zoo/actions/runs/15106735909/job/42456940644?pr=748)

@yiheng-wang-nv I was able to do ./runtests.sh up through pytype. The error popped up here: [163/167] check models.maisi_ct_generative.scripts.rectified_flow

Hi @ybagdasa , thanks for the updates. The issue does not happen in github's workflow, thus we can ignore it (may due to different environment).

I will help to solve the pre-merge CPU related issues this week, then this PR can be merged.

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

yiheng-wang-nv · 2025-05-29T10:58:48Z

models/retinalOCT_RPD_segmentation/configs/metadata.json

+        "torchvision": "0.22.0",
+        "detectron2": "0.6",
+        "lxml": "5.4.0",
+        "pillow": "9.5.0"


Hi @ybagdasa , I added the pillow requirement due to the issue: facebookresearch/detectron2#5010

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

yiheng-wang-nv · 2025-05-29T13:06:51Z

Hi @ybagdasa , I pushed some commits to enable building customized python environment for CI tests, and now premerge-cpu-check works.

For the premerge-gpu-check, I also updated the script and verified in my local machine. To make it work in Github, there are some internal environment changes (we used internal GPU resources for formal checks). I will do that soon.

yiheng-wang-nv · 2025-05-29T13:07:54Z

Hi @ericspod , could you also help to review this new bundle contribution? Thanks in advance!

ericspod

Hi @ybagdasa and @yiheng-wang-nv I've had a look through the contribution and had a few comments to make. I haven't tried running it but from the conversation it seems it's being tested a lot anyway so I'm sure that's fine. I am concerned that there's a lot of code here and very little actual bundle definition, this code is coming from existing sources so that's understandable as an adaptation into the bundle format. I would still suggest looking at how to reduce this code somehow by using existing libraries perhaps. It's not clear what a lot of the code does either and would benefit with a lot more commentary/docstrings in at least the top level definitions.

I see that Miniconda is being used in the tests now, I think I missed why this was. This required a number of changes to other test scripts so I wonder if it's needed. It might help to provide an environment.yaml file to define what environment to construct rather than rely on commands, this might clean up some of the logic.

The flake complaints relate to variable names as you see, I would suggest fixing these or using the ignore comments if you don't want to change this code vs. the original source.

Thanks!

ericspod · 2025-05-30T12:53:43Z

setup.cfg

-per_file_ignores = __init__.py: F401, __main__.py: F401
+# lowercase checks are not needed for the following files in the retinalOCT_RPD_segmentation model
+# https://github.com/Project-MONAI/model-zoo/pull/748#issuecomment-2877638507
+per_file_ignores =
+    __init__.py: F401
+    __main__.py: F401
+    models/retinalOCT_RPD_segmentation/scripts/analysis_lib.py: N802, N803, N806
+    models/retinalOCT_RPD_segmentation/scripts/datasets/data.py: N802, N803, N806
+    models/retinalOCT_RPD_segmentation/scripts/datasets/volReader.py: N801, N802, N803, N806
+    models/retinalOCT_RPD_segmentation/scripts/inference.py: N802, N803, N806


I would suggest fixing the names to conform to PEP8 instead of adding ignores, though if you must you can add ignore comments like # noqa: N402 in your code rather than here.

ericspod · 2025-05-30T13:04:48Z

.github/workflows/premerge-cpu.yml

Why are we using Miniconda for the tests now?

Hi @ericspod , this bundle requires python 3.9, but our current CI workflows use python 3.10, we did not meet python version requirements thus did not consider it in CI before. Therefore, I made the changes to enable customized python version support via using conda. What do you think?

ericspod · 2025-05-30T13:12:11Z

models/retinalOCT_RPD_segmentation/configs/metadata.json

+        "inputs": {
+            "image": {
+                "type": "image",
+                "format": "PNG",


"Format" is meant to be about what the pixel values are, eg. just magnitude values or Hounsfield units.

ybagdasa added 9 commits March 10, 2025 11:17

First commit for forked branch.

8e57a2e

Deleted the zip file download in inference.py. Updated urls in large_…

1e02cc5

…files.yaml. Fixed case-sensitivity for .png/.PNG in dataset/data.py. Made confidence threshold an external argument.

Implemented unit testing and added sample_data.

3ac9f28

Applied changes from isort, black, flake in runtests.sh. Ran pytype s…

0676427

…eparately.

Removed configargparser and added PNG naming instructions in readme.

f7e9de6

Merge branch 'dev' into 730-retinalOCT_RPD_segmentation. Upstream

0b8f1ef

updates.

Moved large_files.yaml into retinalOCT_RPD_Segmentation into bundle r…

30dabf9

…oot.

Updated dependencies. Updated depreciated pandas api. Updated install…

efb5da8

… instructions in readme.

Merge from upstream.

b831e50

yiheng-wang-nv reviewed May 13, 2025

View reviewed changes

yiheng-wang-nv reviewed May 16, 2025

View reviewed changes

fix isort and update cit for detectron2

7cd1394

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

ybagdasa added 2 commits May 16, 2025 18:02

Fixed pre-commit and flake8 errors. Added torchvision to metadata.json.

d171542

Merge branch '730-retinalOCT_RPD_segmentation' of github.com:uw-biome…

50238f8

…dical-ml/monai-model-zoo into 730-retinalOCT_RPD_segmentation

fix pre-commit and black checks

e271884

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

skip part of tests and rrevert changes in UNEST bundles

db0d0d6

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

Fixed flake8 errors. runtests.sh failing at ptype.

ea0ef49

enable different py env for ci tests

992391e

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

yiheng-wang-nv reviewed May 29, 2025

View reviewed changes

yiheng-wang-nv added 6 commits May 29, 2025 11:45

fix precommit

52cce77

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

add conda init for cpu test

03525b8

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

update premerge cpu script

ee65966

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

add conda init

33cc537

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

adjust conda init way

f767da8

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

update init conda env

a07021c

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

ericspod reviewed May 30, 2025

View reviewed changes

Merge branch 'dev' into 730-retinalOCT_RPD_segmentation

310742b

		@@ -0,0 +1,3 @@
		# ensure that using evironment with python==3.9
		pip install -r ./docs/requirements.txt #includes monai

730 retinal oct rpd segmentation #748

Are you sure you want to change the base?

730 retinal oct rpd segmentation #748

Uh oh!

Conversation

ybagdasa commented Apr 29, 2025

Description

Status

Please ensure all the checkboxes:

Uh oh!

ybagdasa commented May 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yiheng-wang-nv commented May 13, 2025

Uh oh!

yiheng-wang-nv commented May 13, 2025

Uh oh!

yiheng-wang-nv commented May 13, 2025

Uh oh!

ybagdasa commented May 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yiheng-wang-nv commented May 16, 2025

Uh oh!

ybagdasa commented May 16, 2025

Uh oh!

yiheng-wang-nv commented May 19, 2025

Uh oh!

yiheng-wang-nv commented May 19, 2025

Uh oh!

ybagdasa commented May 19, 2025

Uh oh!

yiheng-wang-nv commented May 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yiheng-wang-nv commented May 29, 2025

Uh oh!

yiheng-wang-nv commented May 29, 2025

Uh oh!

ericspod left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!