Skip to content

Conversation

@KumoLiu
Copy link
Contributor

@KumoLiu KumoLiu commented Sep 21, 2022

Signed-off-by: KumoLiu yunl@nvidia.com

Fixes #5180.

Description

Add classification post-process script for HoVerNet.

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • 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.

@KumoLiu KumoLiu marked this pull request as ready for review September 21, 2022 15:23
@KumoLiu KumoLiu changed the title [WIP] Add whole post-process script for HoVerNet Add whole post-process script for HoVerNet Sep 21, 2022
Copy link
Member

@bhashemian bhashemian left a comment

Choose a reason for hiding this comment

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

Hi @KumoLiu, thanks for submitting this PR. it seems that this PR is still not ready for reviewing yet. Can you remove redundant files, review and address the checklist in the PR description, resolve the failed tests, and update docs before moving forward with review? Thanks

KumoLiu and others added 2 commits September 22, 2022 14:26
Signed-off-by: KumoLiu <yunl@nvidia.com>
@KumoLiu
Copy link
Contributor Author

KumoLiu commented Sep 22, 2022

Hi @drbeh, I have reviewed items in this PR description, check everything that I have already done, and removed irrelevant lines. Some tests will continue to be unsuccessful since #5173 hasn't been merged. But could you please help review this PR first and let me know if you have any advice? Thanks in advance!

@KumoLiu KumoLiu requested a review from JHancox September 22, 2022 08:28
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@KumoLiu KumoLiu requested a review from bhashemian October 28, 2022 13:21
@KumoLiu KumoLiu changed the title [WIP] Add Classification Result Post Process for HoVerNet Add Classification Result Post Process for HoVerNet Oct 28, 2022
@bhashemian bhashemian self-requested a review November 1, 2022 20:35
Copy link
Member

@bhashemian bhashemian left a comment

Choose a reason for hiding this comment

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

Are you using the testing data? tests/testing_data/hovernet_test_data_raw.npz
If not, please remove it. Thanks

@KumoLiu
Copy link
Contributor Author

KumoLiu commented Nov 2, 2022

Are you using the testing data? tests/testing_data/hovernet_test_data_raw.npz If not, please remove it. Thanks

Yes, thanks for pointing it out, I removed it in the latest commit!

pre-commit-ci bot and others added 5 commits November 2, 2022 11:11
for more information, see https://pre-commit.ci

Signed-off-by: KumoLiu <yunl@nvidia.com>
Signed-off-by: KumoLiu <yunl@nvidia.com>
Signed-off-by: KumoLiu <yunl@nvidia.com>
Signed-off-by: monai-bot <monai.miccai2019@gmail.com>

- auto style fixing
- bump versioneer to 0.23 from 0.19

Signed-off-by: monai-bot <monai.miccai2019@gmail.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Co-authored-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: KumoLiu <yunl@nvidia.com>
Co-authored-by: Behrooz Hashemian <3968947+drbeh@users.noreply.github.com>
Signed-off-by: KumoLiu <yunl@nvidia.com>
@KumoLiu KumoLiu force-pushed the add-whole-postprocess branch from 1cd935d to 3669e4a Compare November 2, 2022 03:12
@bhashemian
Copy link
Member

@JHancox do you have any comments on this PR?

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Nov 7, 2022

Hi @drbeh ,

I see @JHancox approved the PR, and @KumoLiu is developing the whole wrapper transform in #5462 .
Do you still have any other concerns? If not, I will try to merge this PR ASAP.

Thanks in advance.

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Nov 7, 2022

/black

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Nov 7, 2022

/build

1 similar comment
@Nic-Ma
Copy link
Contributor

Nic-Ma commented Nov 7, 2022

/build

@Nic-Ma Nic-Ma enabled auto-merge (squash) November 7, 2022 14:07
@Nic-Ma
Copy link
Contributor

Nic-Ma commented Nov 7, 2022

/build

@Nic-Ma Nic-Ma merged commit 9644b07 into Project-MONAI:dev Nov 7, 2022
@KumoLiu KumoLiu deleted the add-whole-postprocess branch November 8, 2022 02:46
bhashemian added a commit that referenced this pull request Nov 16, 2022
Signed-off-by: KumoLiu <yunl@nvidia.com>

Fixes #5180.

### Description
For the post-process in HoVerNet, now we have PR #5173 to process the
segmentation output(which has been merged) and PR #5186 to process the
classification output(which is under review now). We want a whole
transform containing these two components which can be easily used in
the MONAI model zoo bundle. Here has a draft version of how to combine
these components, if anyone has any suggestions, please feel free to
take it out.

### 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).
- [ ] 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`.
- [x] In-line docstrings updated.
- [ ] Documentation updated, tested `make html` command in the `docs/`
folder.

Signed-off-by: KumoLiu <yunl@nvidia.com>
Signed-off-by: yunliu <yunl@nvidia.com>
Co-authored-by: Behrooz Hashemian <3968947+drbeh@users.noreply.github.com>
bhashemian added a commit to bhashemian/MONAI that referenced this pull request Nov 23, 2022
Signed-off-by: KumoLiu <yunl@nvidia.com>

Fixes Project-MONAI#5180.

### Description
For the post-process in HoVerNet, now we have PR Project-MONAI#5173 to process the
segmentation output(which has been merged) and PR Project-MONAI#5186 to process the
classification output(which is under review now). We want a whole
transform containing these two components which can be easily used in
the MONAI model zoo bundle. Here has a draft version of how to combine
these components, if anyone has any suggestions, please feel free to
take it out.

### 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).
- [ ] 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`.
- [x] In-line docstrings updated.
- [ ] Documentation updated, tested `make html` command in the `docs/`
folder.

Signed-off-by: KumoLiu <yunl@nvidia.com>
Signed-off-by: yunliu <yunl@nvidia.com>
Co-authored-by: Behrooz Hashemian <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
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.

Post-process script for HoVerNet

6 participants