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

Update auto3dseg algorithm templates #184

Merged

Conversation

dongyang0122
Copy link
Contributor

Update auto3dseg algorithm templates for performance (accuracy) and efficiency improvement.

dongy added 2 commits January 19, 2023 12:19
Signed-off-by: dongy <dongy@nvidia.com>
Signed-off-by: dongy <dongy@nvidia.com>
@mingxin-zheng
Copy link
Contributor

I would like to hear @Nic-Ma 's thought on the ConfigParser. Since we already use num_epochs_per_validation in lots of places, such as the unit tests of MONAI, tutorials of Auto3DSeg, I feel hesitated to make such change.

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Feb 2, 2023

Hi @dongyang0122 @mingxin-zheng ,

I tried to reproduce the issue with below config:

{
    "data": [1, 2],
    "data_per_batch": 2,
    "transform": {
        "_target_": "Compose",
        "transforms": [
            {"_target_": "LoadImaged", "keys": "image"},
            {"_target_": "RandTorchVisiond", "keys": "@##0#keys", "name": "ColorJitter", "brightness": 0.25},
        ],
    },
    "dataset": {"_target_": "Dataset", "data": "@data", "transform": "@transform"},
    "dataloader": {
        "_target_": "DataLoader",
        "dataset": "@##dataset",
        "batch_size": "@data_per_batch",
        "collate_fn": "$monai.data.list_data_collate",
    },
}

But seems the first 2 lines can be parsed OK:

"data": [1, 2],
"data_per_batch": 2

Could you please help provide a simple program to reproduce the issue you asked?

Thanks.

Signed-off-by: dongy <dongy@nvidia.com>
@mingxin-zheng
Copy link
Contributor

Hi @dongyang0122 , if the fix in the dev branch has resolved the issue of num_epoch_*, can we revert the name of the variable back to num_epochs_per_validation?

@dongyang0122
Copy link
Contributor Author

Hi @dongyang0122 , if the fix in the dev branch has resolved the issue of num_epoch_*, can we revert the name of the variable back to num_epochs_per_validation?

If we convert it back to previous name, then we have to inform users to install monai from the dev branch. I feel it adds another layer of complexity for general users. how about converting it back once the changes are reflected in the installation via pip install monai? and meanwhile we can merge the changes at the moment.

Signed-off-by: dongy <dongy@nvidia.com>
@dongyang0122 dongyang0122 merged commit 922ac66 into Project-MONAI:main Feb 10, 2023
wyli pushed a commit to Project-MONAI/MONAI that referenced this pull request Mar 15, 2023
Signed-off-by: Mingxin Zheng
<18563433+mingxin-zheng@users.noreply.github.com>

Fixes #5972 .

### Description
Fix and New Feature included:
- fix cache rate:
Project-MONAI/research-contributions#173
- typo fix
Project-MONAI/research-contributions#178
- scheduler step fix
Project-MONAI/research-contributions#182
- enhancement
Project-MONAI/research-contributions#184
- Fixed pretrain weight loading
Project-MONAI/research-contributions#202

### Types of changes
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [x] Non-breaking change (fix or new feature that would not break
existing functionality).
- [x] `python tests/test_integration_autorunner.py` succeeded locally

---------

Signed-off-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
@dongyang0122 dongyang0122 deleted the auto3dseg_update_scripts branch June 5, 2023 13:27
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