search

teamtomo / torch-fourier-slice

Fourier slice extraction/insertion in PyTorch
BSD 3-Clause "New" or "Revised" License
1 stars 4 forks source link

question about rotation matrix ordering #9

Open McHaillet opened 2 hours ago

McHaillet commented 2 hours ago

Hey, I was looking at the code to add some 2D->1D slices extraction, but was wondering about this line

https://github.com/teamtomo/torch-fourier-slice/blob/093c87ea2bca2689f192bdc72c166bfe3d31e3f5/src/torch_fourier_slice/slice_extraction/_extract_central_slices_rfft_3d.py#L38

I understand from this that the input matrices are expected to work on xyz coordinates. Could we perhaps add a keyword to the function for zyx matrices?

alisterburt commented 2 hours ago

good spot - I wasn't sure what the best API was here so would appreciate some input/discussion

Previously (in libtilt) I seem to remember having a zyx: bool keyword argument. The options I considered for this package were

I felt like the presence of the keyword argument made reasoning about the function harder. I felt like only accepting zyx matrices was going to lead to a bunch of unexpected failures for people who don't have any idea what that means, it requires a deeper understanding about how the matrices are being used inside the function and numpy (C style) axis ordering/memory layout for arrays. xyz is inelegant in some ways but felt like the least bad choice... I'm very open to changing to either of the other options - my preference would be zyx only I think if we switched...

One reason I considered not switching was that the scipy Rotation object produces matrices which are xyz