Update Rotate displacement! functions to center coordinates before applying the rotation

[pvillacorta]

This change ensures that the rotation is always performed around the geometric center of the phantom, rather than the origin (0, 0, 0). There may be cases where the phantom has an offset in one of its coordinates, and in such cases, rotating around the origin would not be meaningful. This adjustment ensures the rotation is applied correctly relative to the phantom's actual position.

[cncastillo]

Ok!

[JanWP]

Sorry, I just saw this merged and am arriving late to the party. Reading this PR an immediate question came to my mind:

I believe KomaMRI allows for phantoms with very generic placement of spins, which are not necessarily on a regular grid. For such phantoms, centering coordinates prior to rotation may lead to a center of rotation which depends on phantom orientation. For example, for a phantom consisting of three spins placed randomly in the x-y plane, performing two successive rotations around the z-axis will in general lead to use a different center of rotation for the second rotation with respect to the first. As a result, performing a single rotation by the sum of the two angles will not be the same thing as the two successive rotations, even though all rotations use the same orientation of the rotation axis.

So my questions are: Is this intended behavior? Or is irregular placement of spins too exotic to be considered relevant?

Disclaimer: I do not have a use case where this behavior would be an issue, but it strikes me as something that might not be expected by users. Also, I may have completely misunderstood what this PR does and the context in which it is applied, so please disregard this comment in case it is not relevant.

[cncastillo]

@JanWP Thanks for the comment, we didn't think about this case.

So my questions are: Is this intended behavior? Or is irregular placement of spins too exotic to be considered relevant?

This is very much intended to be supported.

One idea that might solve the issue: if we define two typed of rotation center, one that moves with the phantom and one that doesn't, by default it can start at x=(0,0,0) in both cases. @pvillacorta can you check what would make more sense? I want it to be easy to simulate stuff like this and this, but I am not sure how to define the rotations.

[JanWP]

If it is necessary to automatically define a center of rotation that moves with the spins, it would seem to me that it's best to choose a point that is invariant under rotation. The center of the (x,y,z) bounding box is not, but the 3D center of mass of all spins in the phantom for example is invariant, so that should work. For a regular grid of spins, the center of mass is the same as the center of the bounding box as computed in this PR.

Implementation is as simple as cx = mean(x) etc. This is more costly to compute than (maximum(x) + minimum(x)) / 2, but may still negligible with respect to the rotation operation itself. If computation time is a problem, the center of mass could also be just computed once and stored, but that would involve changes in the phantom data structure and require more widespread code changes and may not be worth it, unless other functions need to know with respect to which center a rotation will operate. Rotations, scaling and shear with respect to that center will not change it, and for rigid translations of the phantom the center of mass can be trivially updated with the same translation. Non-affine transformations of the phantom would require re-calculation of the center of mass.

[pvillacorta]

Hi @JanWP, @cncastillo
Sorry for not replying to this thread earlier, and thank you very much @JanWP for your concern and suggestion. I’ll try to check this as soon as possible. Could you create an issue related to this?