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

Wrong spacing checking logic in DICOM series combination #6630

Closed
function2-llx opened this issue Jun 19, 2023 · 2 comments · Fixed by #6660
Closed

Wrong spacing checking logic in DICOM series combination #6630

function2-llx opened this issue Jun 19, 2023 · 2 comments · Fixed by #6660
Labels
bug Something isn't working

Comments

@function2-llx
Copy link
Contributor

function2-llx commented Jun 19, 2023

Describe the bug

I found some issues in the following code snip:

first_slice = slices[0]
average_distance = 0.0
first_array = self._get_array_data(first_slice)
shape = first_array.shape
spacing = getattr(first_slice, "PixelSpacing", [1.0, 1.0, 1.0])
pos = getattr(first_slice, "ImagePositionPatient", (0.0, 0.0, 0.0))[2]
stack_array = [first_array]
for idx in range(1, len(slices)):
slc_array = self._get_array_data(slices[idx])
slc_shape = slc_array.shape
slc_spacing = getattr(first_slice, "PixelSpacing", (1.0, 1.0, 1.0))
slc_pos = getattr(first_slice, "ImagePositionPatient", (0.0, 0.0, float(idx)))[2]

  1. In line 520, spacing should be got from slices[idx] instead of first_slice; there's the same issue in line 521;
  2. the default value in line 514 is [1.0, 1.0, 1.0], which is a list instance, while the default value in line 520 is (1.0, 1.0, 1.0), which is a tuple instance, and (1.0, 1.0, 1.0) == [1.0, 1.0, 1.0] will result in False.
@wyli wyli added the bug Something isn't working label Jun 19, 2023
@wyli
Copy link
Contributor

wyli commented Jun 19, 2023

thanks for reporting, would you like to submit a PR to fix it? I think the relevant elementwise checks could be done with allclose

def allclose(a: NdarrayTensor, b: NdarrayOrTensor, rtol=1e-5, atol=1e-8, equal_nan=False) -> bool:

@function2-llx
Copy link
Contributor Author

Hi, I can make a PR later.

function2-llx added a commit to function2-llx/MONAI that referenced this issue Jun 27, 2023
function2-llx added a commit to function2-llx/MONAI that referenced this issue Jun 27, 2023
Signed-off-by: function2 <function2-llx@outlook.com>
@function2-llx function2-llx mentioned this issue Jun 27, 2023
3 tasks
wyli pushed a commit that referenced this issue Jun 27, 2023
Fixes #6630.

### Description

Fix DICOM series combining checking

### 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).
- [ ] Integration tests passed locally by running `./runtests.sh -f -u
--net --coverage`.
- [x] Quick tests passed locally by running `./runtests.sh --quick
--unittests --disttests`.

---------

Signed-off-by: function2 <function2-llx@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants