-
Notifications
You must be signed in to change notification settings - Fork 29
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
Add reg_f3d2 support #943
base: master
Are you sure you want to change the base?
Add reg_f3d2 support #943
Conversation
… paramatarisation
fixed bug where no objective function other than nmi would be used
FYI, addresses #944 for completeness. |
Furthermore to what I mention above, I should mention that I've tested NiftyReg on the command line; I don't see the flip with -vel (but I do with -sym, as expected from KCL-BMEIS/niftyreg#71). In NiftyReg, reg_f3d2 inherits from reg_f3d_sym so it may be assumed that the issue comes from reg_f3d_sym, however people using reg_f3d2 not through SIRF don't see this issue (and I mentioned I tested it on the command line and couldn't reproduce). I can produce the issue with: import sirf.Reg as reg
nifty_f3d_sym = reg.NiftyF3dSym()
nifty_f3d_sym.set_reference_image(reg.NiftiImageData3D("./ref.nii"))
nifty_f3d_sym.set_floating_image(reg.NiftiImageData3D("./flo.nii"))
nifty_f3d_sym.process()
resampler = reg.NiftyResample()
nifty_f3d_sym.get_deformation_field_forward().write("./dvf.nii")
resampler.set_reference_image(reg.NiftiImageData3D("./ref.nii"))
resampler.set_floating_image(reg.NiftiImageData3D("./flo.nii"))
resampler.add_transformation(nifty_f3d_sym.get_deformation_field_forward())
resampler.set_interpolation_type_to_cubic_spline()
resampler.forward(reg.NiftiImageData3D("./flo.nii")).write("./output.nii") I do not see the issue with: import sirf.Reg as reg
nifty_f3d_sym = reg.NiftyF3dSym()
nifty_f3d_sym.set_reference_image(reg.NiftiImageData3D("./ref.nii"))
nifty_f3d_sym.set_floating_image(reg.NiftiImageData3D("./flo.nii"))
nifty_aladin_sym = reg.NiftyAladinSym()
nifty_aladin_sym.set_reference_image(reg.NiftiImageData3D("./ref.nii"))
nifty_aladin_sym.set_floating_image(reg.NiftiImageData3D("./flo.nii"))
nifty_aladin_sym.process()
nifty_f3d_sym.set_initial_affine_transformation(nifty_aladin_sym.get_transformation_matrix_forward())
nifty_f3d_sym.process()
resampler = reg.NiftyResample()
nifty_f3d_sym.get_deformation_field_forward().write("./dvf.nii")
resampler.set_reference_image(reg.NiftiImageData3D("./ref.nii"))
resampler.set_floating_image(reg.NiftiImageData3D("./flo.nii"))
resampler.add_transformation(nifty_f3d_sym.get_deformation_field_forward())
resampler.set_interpolation_type_to_cubic_spline()
resampler.forward(reg.NiftiImageData3D("./flo.nii")).write("./output.nii") or: import sirf.Reg as reg
nifty_f3d_sym = reg.NiftyF3dSym()
nifty_f3d_sym.set_reference_image(reg.NiftiImageData3D("./ref.nii"))
nifty_f3d_sym.set_floating_image(reg.NiftiImageData3D("./flo.nii"))
nifty_f3d_sym.get_deformation_field_forward().write("./dvf.nii")
nifty_f3d_sym.get_output().write("./output.nii") Thus is the problem with the resampler? Would you expect the issue to be resolved if you give an input affine transformation if the issue was the resampler? |
Does #832 have anything to do with it? I have pulled master. |
Example code and results: resample is the first code snippet from above, affine is the second code snippet from above and no_resample is the third code snippet from above. resample: https://drive.google.com/file/d/1vubD6Lx6m7ao_zLILxJ-Ae_stBfFVOW4/view?usp=sharing Alternatively: https://github.com/ALEXJAZZ008008/SIRF_niftyreg_f3d2_test |
is compose needed in input is less than one
get def from cpp needed to get from spline then vel due to f3d2
what's the status of this one? Should we review it again? |
nifti_image* init_cpp; | ||
|
||
// If there is an initial transformation matrix, set it | ||
if (_initial_cpp_sptr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we should check if both are present and then throw an error that we cannot handle it
reg_spline_getDefFieldFromVelocityGrid(cpp.get_raw_nifti_sptr().get(), | ||
this->_nifti_image.get(), | ||
false // the number of step is not automatically updated | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this correct? create_from_cpp
always call this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't correct, this will only work with f3d2, I wrote it this way for speed of implementation. Really we should check to see if _use_velocity is set
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem being that _use_velocity is a member variable of NiftyF3dSym
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure what you mean, but maybe this just needs 2 different functions create_from_cpp
and create_from_velocity_field
. Or it needs some header inspection
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's convert this to Draft at the moment, till you're done.
By the way, for whatever reason the PR removed requirements.txt
reg_f3d2 works, however currently it flips the output by 180 degrees. If an affine transformation is given as input it does not flip, weirdly.
Insight on the flipping would be appreciated, I'm not certain if the issue is in SIRF or NiftyReg.