Skip to content

Redo Auto3dseg ensemble path fix#6485

Merged
wyli merged 5 commits intoProject-MONAI:devfrom
mingxin-zheng:fix-6483-ensemble
May 7, 2023
Merged

Redo Auto3dseg ensemble path fix#6485
wyli merged 5 commits intoProject-MONAI:devfrom
mingxin-zheng:fix-6483-ensemble

Conversation

@mingxin-zheng
Copy link
Copy Markdown
Contributor

redo #6481 .

Description

  • Take the code from Auto3dseg ensemble path fix #6481
  • Deprecate data_src_cfg_filename and replace it with data_src_cfg_name so that the same thing can be consistent across AutoRunner, BundleGen, and Ensemble classes
  • Make data_src_cfg_name a required argument in the ensemble runner because it is prone to error to use a default name.
  • Improves test structure, but nothing major is changed.

What is not done:

  • Add tests to check the output structure, because EnsembleRunner doesn't support changing the pred_param for individual algo seperately, and then I can't really run the ensemble method in EnsembleRunner.

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

Signed-off-by: Mingxin Zheng <[email protected]>
@mingxin-zheng mingxin-zheng requested review from myron and wyli May 7, 2023 04:20
Signed-off-by: Mingxin Zheng <[email protected]>
Signed-off-by: Mingxin Zheng <[email protected]>
@mingxin-zheng
Copy link
Copy Markdown
Contributor Author

/integration-test

Copy link
Copy Markdown
Contributor

@wyli wyli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@wyli
Copy link
Copy Markdown
Contributor

wyli commented May 7, 2023

/build

@wyli wyli enabled auto-merge (squash) May 7, 2023 06:40
@wyli
Copy link
Copy Markdown
Contributor

wyli commented May 7, 2023

/build

@wyli wyli merged commit e0d0ce7 into Project-MONAI:dev May 7, 2023
@myron
Copy link
Copy Markdown
Collaborator

myron commented May 8, 2023

@mingxin-zheng since you've standardized it to use "data_src_cfg_name" , do you want to replace "data_src_cfg_filename" in other places? I still see that name in BundleGen vars ? thanks

@mingxin-zheng
Copy link
Copy Markdown
Contributor Author

Thanks @myron! I forgot to do so but I will for the BundleGen.

@mingxin-zheng mingxin-zheng deleted the fix-6483-ensemble branch May 10, 2023 09:44
wyli pushed a commit that referenced this pull request May 10, 2023
…templates folder (#6505)

Fixes #6502 
Fixes #6501

### Background
- BundleAlgo `template_path` points to `algorithm_templates/<Algo>` <=
MONAI 1.1. Recent PR #6436 changed that to `algorithm_templates` and
make it easier to instantiate the BundleAlgo from "algorithm_templates"
folder when the folders are moved.
- But `BundleGen` did not update accordingly and still save
`algorithm_templates/<Algo>` as the `template_path` , which breaks NNI
HPO feature which relies heavily on these paths.

### Description
- Make `template_path` points to `algorithm_templates`
- Avoid using the latest `filelock` (3.12.0) which breaks nni
- Fix comments in
#6485 (comment)
in #6485

### 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] New tests added to cover the changes.
- [x] Integration tests passed locally by running `./runtests.sh -f -u
--net --coverage`.
- [x] Quick tests passed locally by running `./runtests.sh --quick
--unittests --disttests`.
- [x] In-line docstrings updated.

---------

Signed-off-by: Mingxin <[email protected]>
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