SIRF-exercises registration demo does
resampler = Reg.NiftyResample() resampler.set_reference_image(ref) resampler.set_floating_image(flo) resampler.add_transformation(tm) resampler.set_padding_value(0) resampler.set_interpolation_type_to_linear() resampler.process()
but gives warning
/usr/local/lib/python2.7/dist-packages/ipykernel_launcher.py:9: DeprecatedWarning: process is deprecated as of 2.1.0. Use forward(image) instead
if __name__ == '__main__':
Aide from the fact that Python/jupyter could work on their user-friendliness of their errors (I'm sure @AnderBiguri would agree 😉), my question is if we should have deprecated this.
For registration people, the original syntax is straghtforward (you set-up the registration that way, so you do the same for the resampler). The forward
is slightly less easy to grasp in this context.
I'm not sure about what the best is. @rijobro @ckolbPTB ?
Of course, ideally we don't have this warning in the SIRF-exercises, but first we need to know what to do.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Hum, two possible solutions:
process
methodsprocess()
with forward(flo)
.Personally, between the first two, I'm in favour of the second. As you say, process
might be an easier concept to grasp than forward
, and mirrors the registration nicely. However, it will only lead to more confusion if later down the line, the user needs to think about backwards/adjoints and inverses. Also, we would have to put in the documentation of the process
method that it only works in the forward direction, which means they'll have to think about what forward and backward mean anyway.
I suppose we could change the signature of process
to process(direction='forward')
(which obviously also accepts process(backward)
).
@rijobro just nitpicking: process
does not only work for forward
right? It may not return the adjoint(reg)
but it does return the DVFs to do so, am I correct?
@AnderBiguri this is the resampler. it doesn't return a DVF (that's the job of the registration)
Not sure what you mean, can you give an example? The DVFs are the same regardless of forward/adjoint.
Anyway, I'm fairly sure that under the hood, process
simply calls forward
.
@KrisThielemans @rijobro let me go and buy stronger coffee, completely misread all this. Sorry!
Obviously, all this forward
backward
terminology is incredibly confusing in registration. (Most people will think that backward
is the inverse of course). Not much we can do about that, except for documenting it, e.g. in the UserGuide (if anyone reads that, but at least we can point them to it). People will have to think about it.
The process()
terminology was suggested by @ckolbPTB as a generic method, I'd rather not drop it at this stage. See #710
As I don't really want to create a lot of work now without proper discussion, I suggest that we undeprecate process()
, document that it calls forward
on the floating
.
I see that the UserGuide is out-of-date on the methods for the resampling in any case.
PR to un-deprecate process
and improve doc in UserGuide.
For what it's worth, I think this is a pretty good warning!
DeprecatedWarning: process is deprecated as of 2.1.0. Use forward(image) instead
Sure, that warning is crystal clear. It's the cruft around it that more distracting than anything else. Anyway. not our issue.