[SyneRBI/SIRF-Exercises] registration demo question (#44)
Kris Thielemans
Just tried with 2.2.0-rc.1 VM.
The registration demo works, but it seems a bit strange. Initial display of images is
Registered images is indeed
So, it seems to have rotated it correctly. However, the matrix is
numpy.set_printoptions(precision=2,suppress=True)
TM = algo.get_transformation_matrix_forward()
print(TM.as_array())
[[ 0.99 0.07 -0.11 -125.71]
[ -0.06 0.99 0.09 383.12]
[ 0.11 -0.08 0.99 138.53]
[ 0. 0. 0. 1. ]]
which seems no rotation but a huge translation.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Richard Brown
Looking into it, the two images have vastly different qto_ijk matrices:
[4.06,0,0,-128] [-4.06,0,0,-0]
[0,4.06,0,-128] [0,0,4.06,256]
[0,0,4.06,-128] [0,4.06,0,0]
[0,0,0,1] [0,0,0,1]
it looks like the first is RAS, whilst the second is LSI.
as_array just returns the floating array as-is, leading to the rotation in the visualisation. And all the translations come out by adding/subtracting the translation column.
This comes back to the bigger question we've had a few times over what as_array should really do. As-is or LPS?
But for the sake of this problem, I guess I could modify the data (would require PR to the data submodule) to rotate the first image such that the superior points upwards? Might mess up some unit tests that use that file though.
Kris Thielemans
it looks like the first is RAS, whilst the second is LSI.
ok
This comes back to the bigger question we've had a few times over what
as_arrayshould really do. As-is or LPS?
I still think "as-is". Otherwise performance penalty will be terrible.
I guess I could modify the data
I'm reluctant to do that for a few reasons: we need to touch the data submodule, we have a risk, it actually might create trouble, and (most importantly) this is a real world problem, so having the demo cope with this is actually nice.
Do we have an (easy) way to "standardise" the orientation? In that case I would display as now, and then after standardisation. I also see that the centre of both images are in very different places. That's fine, but I'd then also add a brief note on it. (It's a nice feature of NiftyReg that it can still register this).
Note however that you're saying that this should be translations (indeed, that's what the TM says). However, the DVF displayed does not look like a translation at all. I guess it encodes the permutation as well? That would be confusing (similar to @AnderBiguri 's observation with the symmetric f3d?).
Richard Brown
Otherwise performance penalty will be terrible.
I'm not sure that's true.
The displacements do look confusing. But I suspect they're correct, since they mostly encode a translation (the cmap has a relatively small range that agrees with the translation column of the TM). According to the TM, there is a rotation of ~10 degrees, so I suspect that this is the source of the variation in the displacement values.
To test, I created a new resampler and resampled the floating using the displaement fields. The output (resampled floating) matched the reference.
Richard Brown
Copying in the displacements for future reference
Kris Thielemans
ok. agreed. if you look at the scales, they look fine. sorry.
Obviously, if you reorient the image, and get a different deformation field out, we're in trouble.
Kris Thielemans
Otherwise performance penalty will be terrible.
I'm not sure that's true.
At the moment, as_array is a straightforward copy. if we reorient, it's going to be a bunch of nested loops. Also, it is conceivable that we could avoid the copy in the future but passing the pointer (fun questions about ownership etc, but apparently solvable, e.g. via XArray).
Richard Brown
I'm afraid not (on the NiftiImageData side):
I don't believe this to be anywhere near our bottlenecks in terms of timing.
Also, I guess the permute could happen on the python/matlab side.
Kris Thielemans
fine. Maybe Nifti doesn't just copy the data, but that loop could be optimised at some point. Or indeed we give up on having "time" in a certain way as well. I still think we shouldn't pretend to the user that the file-order problem doesn't exist. (If they read themselves using some other tools, they have to be aware of this anyway). (By the way, I wouldn't see why a permute on the python/matlab side would be faster than in C++).
More importantly though, do we have reorient tools? i.e. can we already make sure that 2 as_array have the same orientation?
One solution that comes to mind: call the resampler with an ID transform. It seems to take the orientation of the "reference" image as output.
David Atkinson
Without seeing this issue, this demo is confusing to the user - there is both a rotation and flip that don't appear in the transform matrix. I suggest at least a text explanation in the demo.
Richard Brown
@DANAJK good point. @KrisThielemans perhaps after all, the simplest thing is to update the data submodule?
Richard Brown
Or as @DANAJK says, maybe I should just improve the explanations. You're right @KrisThielemans that this is a real-world problem, so there might be some merit in explaining/printing the qto_xyz matrix, etc.
Kris Thielemans
I think this is a real world problem, so no point in hiding it by giving it clean data.
Can you check what happens if you call the resampler with an ID transform? I'm guessing this will put all data in the same order, and then display is nice. If so, I'd
- display images as is
- print geom-info and discuss rotation and (serious) translation between those 2.
- call resample with ID transform and redisplay. images should now be oriented the same, but show translation (but not the translation of the whole "box").
Then in future, implement something to internally reorient in LPS, such that as_array will be ok. (that'll bring the interesting question that in Python/C++, that will probably need to be SPL order, as in UCL/STIR#491)
Richard Brown
Calling the resampler with an identity transform would result in matching qto_xyz matrices, but the two images don't occupy the same space (large transform), so the resampled image would be filled with zeroes, as the brain would be out of the field of view.
Kris Thielemans
Ah. Of course. Hmmm.
David Atkinson
I agree it's real-world so don't hide. You might want to give either the files or the variables in the demo names that draw attention to the different way the underlying data is stored. I think that printing the qto_xyz with a short explanation of how this shows one is LPS and the other is something else would be good.
If you want to go further, you can probably figure out the dominant direction of the displayed image rows and columns (by taking a dot product of the L,P and S directions with something from the matrix).
Kris Thielemans
Probably best to add some comments and printing of geom info only at the moment, and keep the issue open to fix properly later.
Caveat: Is TM in LPS, or is it in RAS? I'm hoping geom-info is in LPS. If TM is still in RAS, say so in a line, and raise a SIRF issue for 3.0.
Kris Thielemans
@rijobro I'm assuming that this therefore won't need fixes in SIRF 2.2, and we can go ahead with tagging that? Will postpone tagging SIRF-exercises until this is adjusted (and others at https://github.com/SyneRBI/SIRF-Exercises/milestone/3).
Richard Brown
Confusingly, the geom info TM (SIRF) is LPS, whereas the qto_xyz TM is RAS, since it's just a wrapper around the nifti object.
Ok to go ahead with tagging SIRF, I'll improve the explanations in this exercise later today.