730 retinal oct rpd segmentation

[ybagdasa]

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").

[ybagdasa]

@yiheng-wang-nv here is my draft PR

[yiheng-wang-nv]

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]

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]

I will look at this PR later this week

[ybagdasa]

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]

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

[ybagdasa]

@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.

[yiheng-wang-nv]

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

[yiheng-wang-nv]

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]

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]

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.

[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]

Hi @ybagdasa @yiheng-wang-nv this is definitely making progress. If we have a working build for Python 3.10 we can look into removing the Miniconda use in the CICD system and simplify the implementation. Other than that the code is well commented and a few remaining issues can be addressed easily I think.

[ybagdasa]

@ericspod @yiheng-wang-nv I will change the install instructions in the readme to reflect the current working configuration. Other than that, is there anything else left to resolve for this PR?

[yiheng-wang-nv]

@ericspod @yiheng-wang-nv I will change the install instructions in the readme to reflect the current working configuration. Other than that, is there anything else left to resolve for this PR?

Hi @ybagdasa , can I confirm now the model supports to work on python 3.10? If so, no other comments on my side. After you push the readme changes, I will revert the conda CI changes and ensure the model works.

[yiheng-wang-nv]

Hi @ybagdasa , could you fix the pre commit issue? https://results.pre-commit.ci/run/github/490124346/1753149419.xfcqB10AQdC5-UPubS5znQ

[ybagdasa]

Hi @ybagdasa , could you fix the pre commit issue? https://results.pre-commit.ci/run/github/490124346/1753149419.xfcqB10AQdC5-UPubS5znQ

Done :)

[ybagdasa]

@yiheng-wang-nv @ericspod Is there anything else I need to do for this?

[ericspod]

Hi @ybagdasa if you've addressed things then please turn this into a full PR (not a draft) and @yiheng-wang-nv can have one more look at things. I'll enable tests and see how they do as well.

[yiheng-wang-nv]

/build

[yiheng-wang-nv]

Hi @ybagdasa I reverted the py3.9 related changes, this PR can merge after passing all py3.10 based tests

[yiheng-wang-nv]

/build

[yiheng-wang-nv]

Hi @ybagdasa @ericspod , now this bundle works fine with python 3.10, I will merge the PR. Thanks!