TomographicImaging / CILViewer

A simple Viewer for 3D data built with VTK
Apache License 2.0
8 stars 6 forks source link

cilNumpyMETAImageWriter ordering issues #196

Open lauramurgatroyd opened 2 years ago

lauramurgatroyd commented 2 years ago

I came across issues with the cilNumpyMETAImageWriter when I was trying to make a unit test for the cilMetaImageResampleReader with a .mhd file.

Current state of metaimage writer and links to code below are on branch: https://github.com/vais-ral/CILViewer/tree/metaimageheader_write_read

I tried to write a mhd file with the cilNumpyMETAImageWriter and then read it with cilMetaImageResampleReader but found this resulted in ordering issues. See this example:

https://github.com/vais-ral/CILViewer/blob/b15b77bbcaa688b81a68146161d4514b5afeb303/Wrappers/Python/test/test_conversion.py#L31-L85

It shows that when we write a mhd and numpy with the cilNumpyMETAImageWriter and then read it back in we get vtkImageData where the dimensions are in the wrong order (so we do not match the original extent)

We can get back our original array using Converter.vtk2numpy(read_mhd_vtk, order='F') (specifying the 'F' order especially even though our original array that we were writing out started in contiguous order)

In the case of starting with a fortran-order array it's understandable we would have to do this, but in the case of starting with a contiguous array like above I don't think it's right that we have to do this.

The issue is to do with what happens here and that we can't specify whether we have C or F order in the mhd file: https://github.com/vais-ral/CILViewer/blob/b15b77bbcaa688b81a68146161d4514b5afeb303/Wrappers/Python/ccpi/viewer/utils/conversion.py#L479-L503

Note, using just the cilNumpyMETAImageWriter.WriteMETAImageHeader to just write a metaimage header for a raw file works fine if we reverse the shape we put in the header:

https://github.com/vais-ral/CILViewer/blob/b15b77bbcaa688b81a68146161d4514b5afeb303/Wrappers/Python/test/test_conversion.py#L145-L185

Suggested changes below are on branch: https://github.com/vais-ral/CILViewer/tree/metaimage_writer_changes

If we no longer enforce writing a fortran array when we convert numpy to meta, and simply flip the shape ordering put in the header if we don't have a fortran array (similar to what we have to do in the raw case):

https://github.com/vais-ral/CILViewer/blob/5acafb97ae6a7c7552dc157541eda18273157e92/Wrappers/Python/ccpi/viewer/utils/conversion.py#L480-L500

then the Converter.vtk2numpy(read_mhd_vtk, order='F') is no longer needed in the case we start out with a contiguous order array (see tests here: https://github.com/vais-ral/CILViewer/blob/5acafb97ae6a7c7552dc157541eda18273157e92/Wrappers/Python/test/test_conversion.py#L31)

paskino commented 4 months ago

I think the problem is here

https://github.com/vais-ral/CILViewer/blob/471c82a81f3b95dd6f177a7890596d9c40f4df22/Wrappers/Python/ccpi/viewer/utils/conversion.py#L256-L259

We always enforce writing the npy file in Fortran order, it seems.