Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

f3d permuted output #680

Closed
AnderBiguri opened this issue May 27, 2020 · 10 comments · Fixed by #681
Closed

f3d permuted output #680

AnderBiguri opened this issue May 27, 2020 · 10 comments · Fixed by #681
Assignees
Labels
external-fix-required someone else needs to fix something

Comments

@AnderBiguri
Copy link
Contributor

When running the f3d registration, I noticed that my outputs/displacement/deformations are X-Y permuted. For a given identity transformation I get correct Displacement fields, they are just non-zero, they describe a X-Y permutation. You can see the output image being permuted too.

You can test this with:

import sirf
import sirf.STIR as stir
import sirf.Reg as reg
import matplotlib.pyplot as plt
import numpy

# load your favorite Nifti Image. 
reg.ImageData(file)
reg_alg = reg.NiftyF3dSym()
reg_alg.set_initial_affine_transformation(reg.AffineTransformation(numpy.eye(4, dtype=numpy.float32))) # not necessary, but making sure.

reg_alg.set_reference_image(image)
reg_alg.add_floating_image(image)
# Process
reg_alg.process()
# Store results. 
## Change for your folders
reg_alg.get_output(0).write( "res/out_"))
reg_alg.get_deformation_field_forward(0).write(os.path.join(path, "res/DefVF_") )
reg_alg.get_deformation_field_inverse(0).write(os.path.join(path, "res/iDefVF_") )
reg_alg.get_displacement_field_forward(0).write(os.path.join(path, "res/DispVF_"))
reg_alg.get_displacement_field_inverse(0).write(os.path.join(path, "res/iDispVF_"))

I can reproduce the same results with different images (i.e. I have not only tried an identity transform). I can also reproduce it by calling sirf_registration --algo f3d --ref .... via the command line.

I can not reproduce it if I call niftyreg directly:

reg_f3d ....

No parameters have been set in any of the cases, whichever are default have been used.

In my particular test, Displacement fields look like this:

Screenshot from 2020-05-27 17-19-18

@KrisThielemans
Copy link
Member

weird and worrying.

Can you confirm that this image is the displacement field output (I guess so, as z=0)? Is your registered output image ok? What happens if you use the output disp field to resample the "floating"?

Are the niftyreg disp fields zero?

@AnderBiguri
Copy link
Contributor Author

AnderBiguri commented May 27, 2020

Answers to @KrisThielemans :

  1. Yes, definitely displacement (and not deformation), but I can see the permute in the deformation too.
  2. Yes, the image looks OK, but permuted. It looks like clean permute (visually), as the displacement fields suggest.
  3. Have not tested, but seeing the displacement fields, if the resampler does not create a permuted image, then that would mean that there is a bug in the resampler, isn't it?
  4. Only checked the output result. How do I store the displacements from the command line? reg_f3d -h does not have that option listed.

@rijobro
Copy link
Contributor

rijobro commented May 27, 2020

Hi. Looks like the problem is in the symmetric part of f3d unfortunately. Add and remove -sym (undocumented) to the following to see the effect:

reg_f3d -flo test.nii -ref test.nii -sym

Since I wrapped the symmetric version of the algorithm, it seems we always have that problem.

I switched our wrapped algorithm from the symmetric to non-symmetric version, and the problem disappeared.

Solutions include:

  1. bug hunting in NiftyReg
  2. switching to not use the symmetric version of the algorithm
  3. applying a transformation to our output images/transformations

I'll have a quick look at NiftyReg to see if I can spot a bug. Second would be a shame, since symmetric algorithms can be useful. Third option is obviously horrible.

@KrisThielemans
Copy link
Member

ouch. if it's a bug in niftyreg, we should create an issue on their github (whichever is the currently most active, if any). and then pray...

@rijobro
Copy link
Contributor

rijobro commented May 27, 2020

Done - KCL-BMEIS/niftyreg#71

@rijobro rijobro added the external-fix-required someone else needs to fix something label May 27, 2020
@rijobro
Copy link
Contributor

rijobro commented May 27, 2020

In the meantime, to fix this I'll add an option to use the symmetric component or not. Then, until the bug is fixed the non-symmetric version can be used.

@KrisThielemans
Copy link
Member

great. would be good to add a test-case for this. @AnderBiguri should be easy to use your script as a basis. It's a python test, but better that than nothing.

@rijobro
Copy link
Contributor

rijobro commented May 27, 2020

I've added a test to the PR. It fails for the old implementation and passes when symmetric is off.

@AnderBiguri
Copy link
Contributor Author

@rijobro should we have an open issue saying that f3d only wraps/exposes non symetric until NiftyReg fixes their stuff?

@rijobro
Copy link
Contributor

rijobro commented May 28, 2020

Sure, would you mind?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external-fix-required someone else needs to fix something
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants