Inconsistent outputs from geometrical_info and ImageData

[DANAJK]

In the example below, I used a NIfTI dataset that was size 432 x 432 in plane and had 30 slices. Reading into a SIRF ImageData class and getting the geometrical information I can obtain the following. The issues are 1) Some methods (get_voxel_sizes() and get_dimensions()) return native NIfTI parameters - these methods should probably be renamed. They can be confusing because of the 'extra' first index. 2) get_size(), get_spacing() and get offset() seem to have the "slice" as the first dimension, but the data in the array is not like that.

s_imd = Reg.ImageData(ffn)
s_geom_info = s_imd.get_geometrical_info()

s_direction_matrix = s_geom_info.get_direction_matrix()
print("SIRF geom_info direction matrix\n", s_direction_matrix, "\n")

s_offset = s_geom_info.get_offset()
print("SIRF s_geom_info.get_offset()\n", s_offset, "\n")

print("SIRF s_geom_info.get_spacing()\n", s_geom_info.get_spacing(), "\n")
print("SIRF s_imd.get_voxel_sizes() \n", s_imd.get_voxel_sizes(), "\n")

print("SIRF s_geom_info.get_size() \n", s_geom_info.get_size())
print("SIRF s_imd.get_dimensions() \n", s_imd.get_dimensions())
print("SIRF s_imd.dimensions()     \n", s_imd.dimensions())
print("SIRF s_imd.as_array().shape \n", s_imd.as_array().shape)

SIRF geom_info direction matrix [[ 9.9972677e-01 -2.3372350e-02 3.0339886e-06] [-3.0339886e-06 3.5460566e-08 1.0000000e+00] [-2.3372350e-02 -9.9972677e-01 -3.5460566e-08]]

SIRF s_geom_info.get_offset() (384.01007, -21.169691, -180.81993)

SIRF s_geom_info.get_spacing() (5.7360578, 0.9259259, 0.9259259)

SIRF s_imd.get_voxel_sizes() [0. 0.9259259 0.9259259 5.7360578 0. 0. 0.

0. ]

SIRF s_geom_info.get_size() (30, 432, 432) SIRF s_imd.get_dimensions() [ 3 432 432 30 1 1 1 1] SIRF s_imd.dimensions()
(432, 432, 30) SIRF s_imd.as_array().shape (432, 432, 30)

[KrisThielemans]

Related is #632

[AnderBiguri]

Related likely: #812

[KrisThielemans]

This is rather terrible. For unknown reasons, we revert the order https://github.com/SyneRBI/SIRF/blob/f45bbb4d978fe64e3bd3b2f6d965dc11d06a1afd/src/common/SIRF.py#L742-L758

Luckily, the get_size, get_offset and get_spacing functions are nowhere used (except for in the tests, but as the data is cubic both in shape and spacing, it cannot test these things).

@DANAJK I suggest to deprecate these functions, with a warning about the order. We can then sort it out later.

[DANAJK]

I am glad you managed to find where this reversal is taking place. Yes, do whatever to prevent people using them inadvertently.

[AnderBiguri]

This was addressed and closed in #791, and the solution was to reverse those orders. However, on #812 we saw that perhaps the solution we found was not the real solution to the issue (https://github.com/SyneRBI/SIRF/issues/812#issuecomment-779720843).

What I think is happening is that there is a mismatch between geometric printing/getting of arrays, the way we call STIR functions such as zoom_image and python array indexing. The solution to #791 was to make get_size etc return a reversed array, so it matches the order of shape, but then that will make them mismatch the order of functions like zoom_image (among others). I think the only real solution to all these problems is to really enforce 1 ordering all across SIRF, either via deprecating/hiding some functions or (better) just making sure they take always the same order.

I do personally need to use all these, as otherwise SIRF makes it impossible to have custom geometries or to change from one to the other, being able to obtain the geometric info is a must, I believe.

[KrisThielemans]

thanks for the digging @AnderBiguri !

so, I don't really understand https://github.com/SyneRBI/SIRF/commit/bb7690e0c333d673c4f3c712696e45ea827b542b, which was intended to close #791. @danajk made some very nice notebooks in https://github.com/SyneRBI/SIRF-Exercises/pull/111. I'll merge them (after we've resolved this), but here's a screenshot showing that dimensions() and shape are consistent. It is also shows that get_index_to_physical_point_matrix() is correct etc https://github.com/SyneRBI/SIRF/commit/bb7690e0c333d673c4f3c712696e45ea827b542b

However, get_size() returns the reverse order than dimensions() which is exactly what #791 complained about

Note that reversing get_offset() and get_spacing() was independent of the #791 observation. @evgueni-ovtchinnikov made that decision as it seemed logical to do the same thing everywhere. However, that is not so obvious. get_offset() should return an LPS coordinate of the first voxel, whatever the order of the data. One could argue that get_spacing() could return spacing in LPS as well, and indeed get_size(), but these functions would need renaming then.

Note that bb7690e0c333d673c4f3c712696e45ea827b542b was introduced between 3.0.0 and 3.1.0-rc.1, so we probably should revert it now.

[KrisThielemans]

Related https://github.com/SyneRBI/SIRF-Exercises/pull/111#issuecomment-865647282

[AnderBiguri]

Just to add info here that @KrisThielemans and I saw.

791 was fixed by reversing the order of get_size() etc. This is correct, for interfile images, if you remove this reversal, what we see int his issue would happen for those. The issue seems to arise from these both returning different geo_info.

[KrisThielemans]

More discussion in #791