Skip to content

DataAnalyzer fixes#5536

Merged
wyli merged 17 commits intoProject-MONAI:devfrom
myron:analyzer
Nov 21, 2022
Merged

DataAnalyzer fixes#5536
wyli merged 17 commits intoProject-MONAI:devfrom
myron:analyzer

Conversation

@myron
Copy link
Copy Markdown
Collaborator

@myron myron commented Nov 17, 2022

This adds several non-breaking fixes to DataAnalyzer

  • do_ccp option defaults to False (as we don't currently use it, and it's pretty slow)
  • on GPU, dataloading is in parallel (num_workers = self.workers), previously it was only synchronous
  • when label for one case is completely empty (no foreground), it crashes, so this PR fixes, by putting all zeros in label status

this PR partially addresses #5430

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: myron <[email protected]>
Signed-off-by: myron <[email protected]>
@myron myron removed the request for review from mingxin-zheng November 17, 2022 08:17
@wyli wyli requested review from Nic-Ma and mingxin-zheng November 18, 2022 09:09
Copy link
Copy Markdown
Contributor

@mingxin-zheng mingxin-zheng left a comment

Choose a reason for hiding this comment

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

Thanks @myron for fixing the data analyzer. It looks good to me.

I made some minor comments above and please feel free to skip them. I marked them as "resolved" because neither of it should be blocking the merge.

@wyli
Copy link
Copy Markdown
Contributor

wyli commented Nov 21, 2022

/build

@wyli wyli enabled auto-merge (squash) November 21, 2022 22:26
@wyli wyli merged commit 77651ad into Project-MONAI:dev Nov 21, 2022
bhashemian pushed a commit to bhashemian/MONAI that referenced this pull request Nov 23, 2022
This adds several non-breaking fixes to DataAnalyzer

- do_ccp option defaults to False (as we don't currently use it, and
it's pretty slow)
- on GPU, dataloading is in parallel (num_workers = self.workers),
previously it was only synchronous
- when label for one case is completely empty (no foreground), it
crashes, so this PR fixes, by putting all zeros in label status

this PR partially addresses
Project-MONAI#5430

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

Signed-off-by: myron <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Signed-off-by: Behrooz <[email protected]>
bhashemian pushed a commit to bhashemian/MONAI that referenced this pull request Nov 23, 2022
This adds several non-breaking fixes to DataAnalyzer

- do_ccp option defaults to False (as we don't currently use it, and
it's pretty slow)
- on GPU, dataloading is in parallel (num_workers = self.workers),
previously it was only synchronous
- when label for one case is completely empty (no foreground), it
crashes, so this PR fixes, by putting all zeros in label status

this PR partially addresses
Project-MONAI#5430

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

Signed-off-by: myron <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Signed-off-by: Behrooz <[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