Skip to content

Revert "AutoRunner small updates"#5555

Merged
wyli merged 1 commit intodevfrom
revert-5523-autorunner
Nov 22, 2022
Merged

Revert "AutoRunner small updates"#5555
wyli merged 1 commit intodevfrom
revert-5523-autorunner

Conversation

@wyli
Copy link
Copy Markdown
Contributor

@wyli wyli commented Nov 22, 2022

Reverts #5523

@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented Nov 22, 2022

Hi @myron , @mingxin-zheng ,

Should we revert or you already find a quick fix for the test issue?

Thanks.

@mingxin-zheng
Copy link
Copy Markdown
Contributor

Let me give a quick fix now

@wyli
Copy link
Copy Markdown
Contributor Author

wyli commented Nov 22, 2022

I think we should revert, my previous comments were also not addressed, looks like a mistake #5523 (comment) cc @Nic-Ma @mingxin-zheng

@wyli
Copy link
Copy Markdown
Contributor Author

wyli commented Nov 22, 2022

/build

@wyli wyli enabled auto-merge (squash) November 22, 2022 10:23
@mingxin-zheng
Copy link
Copy Markdown
Contributor

mingxin-zheng commented Nov 22, 2022

I think we should revert, my previous comments were also not addressed, looks like a mistake #5523 (comment) cc @Nic-Ma @mingxin-zheng

--------------- Updated -----------------
After some thoughts, I created a new ticket (#5558) to describe the issue in the integration test.

The issue is not brought by PR #5523. Instead, it is triggered by using Auto3DSeg HPO and then ensemble the HPO experiments by AlgoEnsembleBestByFold.

Because it can be fixed separately, it shouldn't be mixed with PR #5523.

--------------- Original ------------------
I provided a quick fix in #5557.

@myron I am be totally wrong but I guess this AutoRunner needs to be updated as soon as possible so it doesn't block other works of yours. So I simply followed @wyli 's suggestion to make algos in the end of the argument list of AutoRunner init in my fix. If more discussion is needed, please disregard this quick fix #5557 and created yours. Thanks!

@mingxin-zheng mingxin-zheng mentioned this pull request Nov 22, 2022
7 tasks
@wyli
Copy link
Copy Markdown
Contributor Author

wyli commented Nov 22, 2022

/build

@myron
Copy link
Copy Markdown
Collaborator

myron commented Nov 22, 2022

I think we should revert, my previous comments were also not addressed, looks like a mistake #5523 (comment) cc @Nic-Ma @mingxin-zheng

I though I addressed those comments. I commented again to clarify. and it does not seem to be reason for this issue.
thank you.

@myron
Copy link
Copy Markdown
Collaborator

myron commented Nov 22, 2022

I think we should revert, my previous comments were also not addressed, looks like a mistake #5523 (comment) cc @Nic-Ma @mingxin-zheng

--------------- Updated ----------------- After some thoughts, I created a new ticket (#5558) to describe the issue in the integration test.

The issue is not brought by PR #5523. Instead, it is triggered by using Auto3DSeg HPO and then ensemble the HPO experiments by AlgoEnsembleBestByFold.

Because it can be fixed separately, it shouldn't be mixed with PR #5523.

Thanks for the analysis. Let me work with you to fix it quickly. I did change the default ensemble method to AlgoEnsembleBestByFold, but the previous one was incorrect. Let me chat to you what the best way to fix quickly.

@wyli wyli marked this pull request as ready for review November 22, 2022 17:12
@wyli wyli merged commit 526ea1f into dev Nov 22, 2022
@wyli wyli deleted the revert-5523-autorunner branch November 22, 2022 17:12
bhashemian pushed a commit to bhashemian/MONAI that referenced this pull request Nov 23, 2022
bhashemian pushed a commit to bhashemian/MONAI that referenced this pull request Nov 23, 2022
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.

4 participants