Developer question: why so many clones?
This came up because of slow performance in ants.resample_image for large images
Lots of copies of the big image happening https://github.com/ANTsX/ANTsPy/blob/43c71745154972068633fc80d8cbe778e36123e3/ants/ops/resample_image.py#L50-L60
Originally posted by @cookpa in #810
Do all these clones need to happen? Would it be possible to initialize a dummy image and pass that pointer to libfn?
Was this @ncullen93 who put this together? If so, hopefully he can weigh in.
Was this @ncullen93 who put this together? If so, hopefully he can weigh in.
Yep - I sent him an email, will follow up here as needed. Thanks
I think cloning to float was done everywhere because float was (or maybe still is) the only pixel type that was wrapped by many operations - resampling in particular, if I remember correctly. This was probably done to reduce the size of the build because wrapping so many pixel types really exploded the package size.
Maybe it's not needed so much since the build / c++ wrapping has gone through two versions of improvements. So now wrapping every pixel type may be both faster from a developer's perspective and may not increase the build size so much.
My other thought was that it was done not for simplicity or size but because there were some kind of like rounding errors or some bad implementation, but that's unlikely. It also may be possible that I just did this because ANTsR did this, but again unlikely.
I would suggest removing it and if it runs and passes some basic unit tests across different pixel types then it is definitely OK to remove.
Thanks @ncullen93 . There definitely might have been inconsistencies in base ANTs where a certain pixel type was hard-coded.
I will take another look at the ANTs code, particularly where it writes output to a pointer. And maybe we can remove some of these clones to speed things up. The median size of images is probably going to keep going up.
I looked into this a bit while dealing with #753
I think the answer is here
https://github.com/ANTsX/ANTs/blob/0fc81b7939f08e12500e3066b23697d04af9fe08/Utilities/ReadWriteData.h#L210-L222
The input must match TImageType already because its pointer is statically cast to that type.
Then a CastImageFilter is used to cast the data, but I'm not sure this is helpful. It might be nice to clone the input to ensure it's not modified, but CastImageFilter inherits from InPlaceImageFilter, so it might not actually copy the data if they already match.
So, I think this is why things have to be cloned on input to float or double as expected by whatever C++ program is being used.
The other potential issue I noticed is that if the "channels_first" boolean is set to True, the input and output to ANTs programs will be wrong.
For output, the pointer is simply overwritten, so I don't think it's necessary to clone the input again, we just need an antsImage with the channels_first == False (the default).
To handle this most efficiently, I think we should either do:
-
Add a helper function in ANTsPy that checks for channels_first and undoes it if necessary, and returns a cloned image with the proper ordering, type, and a dummy output image. We would then pass this to ANTs, and not bother with the CastImageFilter. The firewall of cloning comes on the ANTsPy side, which seems to be how we've done things so far.
-
Clone on the C++ side. Use dynamic casts in ANTs to find the image type, then clone it with CastImageFilter with in place editing explicitly turned off.
We still have to deal with channels_first somehow - maybe adding a check to the libfn code.
- Don't clone - saves memory, but might introduce bugs if any ANTs code is modifying its input. And still, we have to keep track of channels_first.