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

Inconsistent writing of Metadata. Loading + Saving results in different metadata. #6136

Closed
phcerdan opened this issue Mar 13, 2023 · 5 comments · Fixed by #6147
Closed

Inconsistent writing of Metadata. Loading + Saving results in different metadata. #6136

phcerdan opened this issue Mar 13, 2023 · 5 comments · Fixed by #6147
Assignees
Labels
enhancement New feature or request

Comments

@phcerdan
Copy link

Describe the bug
Using SaveImage after LoadImage results in image saved with different metadata.

To Reproduce
Steps to reproduce the behavior:

    original2 = LoadImage(image_only=True)(original_file)
    SaveImage(output_dir=output_folder + "/monai_modified_meta",
              output_ext = ".nrrd",
              writer= "ITKWriter",
              resample=False,
              separate_folder= False,
              squeeze_end_dims = False,
              output_dtype=itk.SI)(original2)

Where original_file points to the attached image (a dcm 2D slice):
CT.X.1.2.276.0.7230010.3.1.4.0.6.1667732449.1809819.zip

Expected behavior
The metadata should be the same of the original image after save.

Screenshots
The original dcm loaded in Slicer:
image

The result after loading + saving with the above code:
image

Environment

Ensuring you use the relevant python executable, please paste the output of:

python -c 'import monai; monai.config.print_debug_info()'
MONAI flags: HAS_EXT = False, USE_COMPILED = False, USE_META_DICT = False
MONAI rev id: a2ec3752f54bfc3b40e7952234fbeb5452ed63e3

Additional context
I have tested using itk:

original_itk = itk.imread(original_file)
itk.imwrite(original_itk, output_folder + "/monai_modified_meta/original_after_itk_save.nrrd")

and the metadata is conserved (afer opening it in Slicer).
There must be some internal Monai logic messing this up.

The data is a bit odd, but correct, the spacing is negative. ITK corrects this at the moment of reading, flipping the sign in the direction matrix for those indices with negative spacing (and flipping the signs in spacing).

@wyli
Copy link
Contributor

wyli commented Mar 13, 2023

thanks for reporting, original2 = LoadImage(image_only=True, affine_lps_to_ras=False)(original_file) when loading with give the itk result.

I think there's a separate bug for the default image writing, will look into it soon.

@wyli wyli self-assigned this Mar 13, 2023
@wyli wyli added the enhancement New feature or request label Mar 13, 2023
@phcerdan
Copy link
Author

Thanks @wyli.
How can I provide that afine_lps_to_ras = False option to SaveImage instead of LoadImage? I see the option is there for the ITKWriter, but SaveImage doesn't seem to accept it.

@wyli
Copy link
Contributor

wyli commented Mar 13, 2023

yes, I also tried that, currently the usage is

save = SaveImage(
    output_dir=output_folder + "/monai_modified_meta",
    output_ext=".nrrd",
    writer="ITKWriter",
    resample=False,
    separate_folder=False,
    squeeze_end_dims=False,
    output_dtype=itk.SI,
)
save.set_options(init_kwargs={"affine_lps_to_ras": False})
save(original2)

but this logic doesn't look right at the moment...

if isinstance(data_array, MetaTensor) and data_array.meta.get(MetaKeys.SPACE, SpaceKeys.LPS) != SpaceKeys.LPS:
affine_lps_to_ras = False # do the converting from LPS to RAS only if the space type is currently LPS.

(another workaround is to set output_ext=".nii.gz", this works because nifti's affine is in RAS)

@KumoLiu
Copy link
Contributor

KumoLiu commented Mar 16, 2023

I find another bug when taking a look at this issue.
When using PydicomReader to read a dcm slice(2d), we didn't correctly record the spacing. Maybe using (0028, 0030) Pixel Spacing this tag can be safer.

dr, dc = metadata.get("spacing", (1.0, 1.0))[:2]

@phcerdan
Copy link
Author

phcerdan commented Mar 16, 2023

I would recommend to have only one source of truth for geometrical metadata (Origin, Spacing, Orientation), and that to be the affine attribute (the matrix) of the Metatensor.
Having spacing in metadata makes no sense, you can get the spacing from the affine matrix.

wyli added a commit that referenced this issue Mar 16, 2023
Fixes #6136 
fixes #6146 


### 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).
- [x] New tests added to cover the changes.
- [ ] 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.
- [ ] Documentation updated, tested `make html` command in the `docs/`
folder.

---------

Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants