Skip to content

Conversation

@bhashemian
Copy link
Member

@bhashemian bhashemian commented Sep 21, 2022

Fixes #5188

Description

This PR reimplement SobelGradients and SobelGradientsd using separable kernels and generalize it to images with any spatial dimension (2D, 3D, etc.) and option to calculate the gradient along any given axis.

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>
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>
@bhashemian bhashemian requested a review from KumoLiu September 22, 2022 02:20
@bhashemian bhashemian enabled auto-merge (squash) September 22, 2022 02:20
KumoLiu
KumoLiu previously approved these changes Sep 22, 2022
Copy link
Contributor

@KumoLiu KumoLiu left a comment

Choose a reason for hiding this comment

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

Thanks for your update, it looks good to me now!

wyli
wyli previously requested changes Sep 22, 2022
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.

perhaps this need some refactoring for 3d as well

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

KumoLiu commented Sep 24, 2022

Hi @drbeh,
I have just added unittest in CalcualteInstanceSegmentationMap which used SobelGradients and found some different effects between cv2 and SobelGradients.
The difference between cv2 and SobelGradients also cause CalcualteInstanceSegmentationMap can't get correct instance segmentation results. So I just used cv2 now. I will withdraw my approval of this PR and please check it.
Here is my test code.

import cv2
from monai.transforms.post.array import SobelGradients

image = np.zeros([20,20])
image[5:15, 5:15] = 1

filter = SobelGradients(kernel_size=5)
mo_ret = filter(image[None]).squeeze()
cv_ret_0 = cv2.Sobel(image, cv2.CV_64F, 0, 1, ksize=5)
cv_ret_1 = cv2.Sobel(image, cv2.CV_64F, 1, 0, ksize=5)
mo_ret = 1 - (mo_ret - np.min(mo_ret)) / (np.max(mo_ret) - np.min(mo_ret))
cv_ret_0 = 1 - (cv_ret_0 - np.min(cv_ret_0)) / (np.max(cv_ret_0) - np.min(cv_ret_0))
cv_ret_1 = 1 - (cv_ret_1 - np.min(cv_ret_1)) / (np.max(cv_ret_1) - np.min(cv_ret_1))

fig, ax = plt.subplots(1,5,figsize = (50,10))
ax[0].imshow(mo_ret[0], cmap='gray')
ax[1].imshow(mo_ret[1], cmap='gray')
ax[2].imshow(cv_ret_0, cmap='gray')
ax[3].imshow(cv_ret_1, cmap='gray')
ax[4].imshow(mo_ret[0] - cv_ret_0, cmap='gray')

Screen Shot 2022-09-24 at 17 07 00

Thanks!

@KumoLiu KumoLiu dismissed their stale review September 24, 2022 09:11

as comment

@wyli
Copy link
Contributor

wyli commented Sep 24, 2022

I have just added unittest in CalcualteInstanceSegmentationMap which used SobelGradients and found some different effects between cv2 and SobelGradients

@drbeh and I were discussing this in another thread, opencv uses the separable implementation cv2.getDerivKernels(1, 0, 5), that's slightly different from what we have here. but we haven't looked into the actual segmentation outcome differences.

@KumoLiu
Copy link
Contributor

KumoLiu commented Sep 26, 2022

Just FYI, I showed the instance segmentation result from CalcualteInstanceSegmentationMap. The results are calculated with all the same components except for Sobel gradient.
cv2 - result
Screen Shot 2022-09-26 at 10 13 23
MONAI - result
Screen Shot 2022-09-26 at 10 17 40
Thanks!

@JHancox JHancox closed this Sep 26, 2022
auto-merge was automatically disabled September 26, 2022 10:27

Pull request was closed

@JHancox
Copy link
Contributor

JHancox commented Sep 26, 2022

Sorry - inadvertantly closed the PR

@JHancox JHancox reopened this Sep 26, 2022
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@bhashemian bhashemian force-pushed the update-sobel-direction branch from b126b91 to 7eb6ae5 Compare October 27, 2022 14:56
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>
@bhashemian bhashemian changed the title Update SobelGradients to include direction Generalize SobelGradients to 3D and Any Axis Oct 27, 2022
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@bhashemian
Copy link
Member Author

@wyli @Nic-Ma @KumoLiu any other comments here? Thanks

@bhashemian bhashemian enabled auto-merge (squash) October 28, 2022 12:40
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.

The api looks good to me

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Oct 31, 2022

/black

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Oct 31, 2022

/build

@wyli wyli disabled auto-merge October 31, 2022 12:44
@wyli wyli enabled auto-merge (squash) October 31, 2022 12:44
@wyli
Copy link
Contributor

wyli commented Oct 31, 2022

/build

@wyli wyli merged commit e44f3b9 into Project-MONAI:dev Oct 31, 2022
@bhashemian bhashemian deleted the update-sobel-direction branch October 31, 2022 14:37
KumoLiu pushed a commit that referenced this pull request Nov 2, 2022
Fixes #5188 

### Description

This PR reimplement `SobelGradients` and `SobelGradientsd` using
separable kernels and generalize it to images with any spatial dimension
(2D, 3D, etc.) and option to calculate the gradient along any given
axis.

### 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: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: KumoLiu <yunl@nvidia.com>
yiheng-wang-nv pushed a commit to yiheng-wang-nv/MONAI that referenced this pull request Nov 2, 2022
Fixes Project-MONAI#5188 

### Description

This PR reimplement `SobelGradients` and `SobelGradientsd` using
separable kernels and generalize it to images with any spatial dimension
(2D, 3D, etc.) and option to calculate the gradient along any given
axis.

### 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: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Yiheng Wang <vennw@nvidia.com>
yiheng-wang-nv pushed a commit to yiheng-wang-nv/MONAI that referenced this pull request Nov 2, 2022
Fixes Project-MONAI#5188 

### Description

This PR reimplement `SobelGradients` and `SobelGradientsd` using
separable kernels and generalize it to images with any spatial dimension
(2D, 3D, etc.) and option to calculate the gradient along any given
axis.

### 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: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Yiheng Wang <vennw@nvidia.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.

Optional gradient direction in SobelGradients

5 participants