ANTsX / ANTsRCore

Rcpp bindings for the C++ ANTs library used by the ANTsR package
9 stars 9 forks source link

antsGetSpacing much slower than getting voxel dims from antsImageHeaderInfo #130

Open dorianps opened 3 years ago

dorianps commented 3 years ago

Is your feature request related to a problem? Please describe. For reasons that I haven't dug up entirely, antsGetSpacing is much slower than getting spacing through antsImageHeaderInfo. I had to extract spacing from ~3000 images and it took just a few seconds with antsImageHeaderInfo while antsGetSpacing took maybe 10-15 minutes. Just leaving a note here in case we want to optimize.

Describe the solution you'd like We can consider making antsGetSpacing a wrapper of antsImageHeaderInfo which returns only voxel dimensions. Right now I am not sure why we are using a separate C function for getting spacing in https://github.com/ANTsX/ANTsRCore/blob/master/R/ants_set_get.R#L105

Describe alternatives you've considered NA

Additional context No bug, just potential for optimization.

muschellij2 commented 3 years ago

Can you send a PR to merge these 2?

dorianps commented 3 years ago

Looks like the two functions are optimized for two different scenarios. antsImageHeaderInfo is very fast for files on disk and extremely slow for antsImages (image loaded into R memory) nii = antsImage, thisfileNii = character filename image

antsGetSpacing is fast for antsImages but slow for files on the disk. image

So, simply wrapping antsGetSpacing around antsImageHeaderInfo will not be optimal, unless we add some if conditions in there to check the type of input.

P.s. Sorry for the pics can't copy paste in x2go.

dorianps commented 3 years ago

The code for antsGetSpacing already checks if the input is antsImage:

image

but the problem seems to be that check_ants is returning an antsImage type for a character vector. Maybe that should be fixed?

image

muschellij2 commented 3 years ago

That's the point of the function of check_ants. Best, John

On Mon, Jan 4, 2021 at 1:26 PM dorianps notifications@github.com wrote:

The code for antsGetSpacing already checks if the input is antsImage:

[image: image] https://user-images.githubusercontent.com/9083517/103566371-e4274200-4e8f-11eb-8870-d54e8158ebc3.png

but the problem seems to be that check_ants is returning an antsImage type for a character vector. Maybe that should be fixed?

[image: image] https://user-images.githubusercontent.com/9083517/103566658-61eb4d80-4e90-11eb-8803-332c4ff9c1f5.png

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsRCore/issues/130#issuecomment-754137753, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIGPLVJ4GOLEQP5ZRLDWNDSYIB6LANCNFSM4VTKPZZA .

dorianps commented 3 years ago

Is the purpose of check_ants to load an image into memory? If that is the case, maybe that is suboptimal. It might be faster to use antsImageHeader and return the result rather than load the file in memory to enable antsGetSpacing do the job.

muschellij2 commented 3 years ago

The purpose of check_ants is to pass in an object (antsImage, nifti object, RNifti image) or character filename and ensure the output is an antsImage object.

dorianps commented 3 years ago

In that case, I can add an initial if statement in antsGet* to get the info from antsImageHeaderInfo for character vectors, and let the script run through the current code for all other types. Would it be ok? Not sure if that if statement will bring some overhead though. I can also imagine that we do not change anything in the code, but add a warning for character inputs to tell the user that antsImageHeaderInfo is faster for files on disk.

Not a big deal anyway, but can make for a more friendly and faster usability.