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

GridPatch with both count and threshold filtering #6055

Merged
merged 20 commits into from
Mar 1, 2023

Conversation

drbeh
Copy link
Member

@drbeh drbeh commented Feb 23, 2023

Fixes #6049

Description

This PR add support to GridPatch for both filtering by count and threshold. When filtering by threshold, the num_patches will be treated as maximum number of patches.

UPDATE: It also removes the deprecated argument of skimage.measure.regionprops, 'coordinates', which was causing some test to fail.

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.

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@drbeh drbeh requested review from KumoLiu and Nic-Ma February 23, 2023 16:20
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@drbeh
Copy link
Member Author

drbeh commented Mar 1, 2023

@kenza-bouzid @KumoLiu could you please let me know if you have any other comment? Otherwise, we can merge this PR. @wyli @Nic-Ma

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@wyli
Copy link
Contributor

wyli commented Mar 1, 2023

/build

Copy link

@kenza-bouzid kenza-bouzid left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
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, I'm merging this for now, please follow up if you have any concerns @Nic-Ma @KumoLiu

@wyli wyli merged commit 579fe65 into Project-MONAI:dev Mar 1, 2023
@drbeh drbeh deleted the fix-6049-grid-patch branch March 1, 2023 22:01
@drbeh drbeh mentioned this pull request Mar 2, 2023
1 task
@kenza-bouzid
Copy link

@drbeh what is the timeline for this feature to be released?

@drbeh
Copy link
Member Author

drbeh commented Mar 3, 2023

@drbeh what is the timeline for this feature to be released?

Hi @kenza-bouzid, it is merged and is the dev branch of MONAI (and the next weekly release) but it will be released in v1.2 which is due in about a month.

@kenza-bouzid
Copy link

kenza-bouzid commented Mar 10, 2023

Hi @drbeh I am trying to install a version of MONAI that includes this fix
microsoft/hi-ml#834
But my Docker image build fails with the following error:

  Running command git clone -q https://github.com/Project-MONAI/MONAI.git /tmp/pip-req-build-injno4yb
  Running command git checkout -q 958aac78ab8130517638a47175571e081ae03aee
  ERROR: Command errored out with exit status 1:
   command: /azureml-envs/azureml_6f4ca4ccd5cfecbee0a7e6231b99b5fc/bin/python /azureml-envs/azureml_6f4ca4ccd5cfecbee0a7e6231b99b5fc/lib/python3.9/site-packages/pip/_vendor/pep517/_in_process.py get_requires_for_build_wheel /tmp/tmpq3ubf06k
       cwd: /tmp/pip-req-build-injno4yb
  Complete output (36 lines):
  Please set environment variable `BUILD_MONAI=1` to enable Cpp/CUDA extension build.
  BUILD_MONAI_CPP=False, BUILD_MONAI_CUDA=False, TORCH_VERSION=0.
  Traceback (most recent call last):
    File "/tmp/pip-build-env-axzysf86/overlay/lib/python3.9/site-packages/torch/__init__.py", line 172, in _load_global_deps
      ctypes.CDLL(lib_path, mode=ctypes.RTLD_GLOBAL)
    File "/azureml-envs/azureml_6f4ca4ccd5cfecbee0a7e6231b99b5fc/lib/python3.9/ctypes/__init__.py", line 382, in __init__
      self._handle = _dlopen(self._name, mode)
  OSError: /tmp/pip-build-env-axzysf86/overlay/lib/python3.9/site-packages/torch/lib/../../nvidia/cublas/lib/libcublas.so.11: undefined symbol: cublasLtGetStatusString, version libcublasLt.so.11

Any idea how to fix this? I am relying on AzureML environment build so I can't easily set extra environment variables.

The latest available release is 1.1.0 https://pypi.org/project/monai/#history you mentioned a weekly release, is it a different dev package?

@drbeh
Copy link
Member Author

drbeh commented Mar 10, 2023

Hi @kenza-bouzid,
I am not sure what's the source of the error that you get but maybe a weekly pypi build would be an easier option for the meantime. You can find the monai-weekly here: https://pypi.org/project/monai-weekly/, which is built every Saturday.
The latest version monai-weekly==1.2.dev2310 (built on March 4) should include the changes that you need. Please let me know if you encounter any problem.

@kenza-bouzid
Copy link

@drbeh, the weekly pipy build is working fine. Thank you!

kenza-bouzid added a commit to microsoft/hi-ml that referenced this pull request Mar 11, 2023
)

Temporary fix until next release of MONAI
Include fixes: 
Project-MONAI/MONAI#6095
Project-MONAI/MONAI#6055

Install dev version of MONAI pointing at commit
Project-MONAI/MONAI@958aac7
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.

GridPatch transform support for max num_patches while filtering
5 participants