this is non-breaking. I'll add more tests after addressing any major comments @Project-MONAI/core-reviewers

(.cu and .cpp were updated to remove some duplicated assignments.)

Hi @wyli ,

Thanks for the quick implementation.
I think the overall design is aligned with our discussion in the ImageWriter PR.
@ericspod Do you have any other concerns to go this way?

Thanks in advance.

/black

One issue is that the purpose of the various transforms is getting less clear, if I want to do an affine transform on an image do I use Affine, SpatialResample, AffineGrid (the docstring for this isn't clear about it's purpose though the arguments for __call__ show it won't work on images), or Resample. SpatialResample uses Affine and AffineTransform so it's clearer looking at the code to use this, but then why have Affine?

A more general issue that I encountered with Rand*DElastic is that the coordinate space for grid_pull (ours) and grid_sample (Pytorch) are different and so conversion has to be done in Resample here and below. I think in my profiling this was a significant cause of slowness with Rand*DElastic that was avoided in the way RandSmoothDeform created it's coord field with meshgrid and used the smooth field. I realize there are functional differences between RandSmoothDeform and Rand*DElastic but eliminating the need to manipulate coordinate fields, and keeping as much object creation in the constructor as possible, improved performance significantly with visually similar results. SpatialResample will have the same issue of performance since it ultimately uses Resample. It's a bit out of scope here but I had to mention it somewhere!

Sure for the first concern I'm revising the docstring to clarify the usage.

For the Resample coordinates issue,

• pytorch uses [-1, 1] and axes (k, j, i), channel last
• ours csrc backend uses [0, size-1] and axes [i, j, k]
• for the grid computations in the data augmentations it seems natural to use [-(size-1)/2, (size-1)/2] (for example we often rotate an image around its centre instead of [0,0]).

These describe the same thing with different conventions, should be converted properly somewhere before calling the backend resampling API. previously it's done in the resampler. Now I added a norm_coords: bool in Resample to make the normalisation optional, it should speedup things a little bit, and the user should take care of the convention if norm_coords=False
in the future we may define a default convention and further optimise it...

Thanks for the quick update, this PR overall looks good to me now, put minor comments inline.
I didn't check carefully about the affine computation and C++ resampling logic as I am not very familiar with that.
@ericspod @rijobro Could you please help double confirm this PR as you put some comments before?
If you guys don't have any other comments or concerns, I will approve this big PR soon.

Thanks in advance.

/integration-test

/integration-test

Integration test is ok https://github.com/Project-MONAI/MONAI/runs/5044690171?check_suite_focus=true

/build