Python's Numpy bridge - GetImageFromArray - Freed buffer pointer after Graft

Engineering
1

I have been struggling with a bug (in my opinion) of GetImageFromArray in the Numpy bridge. In RTK, I need to graft an Image to a CudaImage when I want to use a Cuda filter using non-Cuda images. See, e.g., this example. However, I sometimes had weird memory behavior which I finally understood: when using GetImageFromArray, the management of the memory is left to numpy by backing up the array in _ndarr, see the code here. When grafting the image created by GetImageFromArray, _ndarr is not copied and the memory is freed if the original image is. Hence the issue. Here is a minimal example to reproduce the problem (hoping the memory is actually overwritten by np.zeros, it worked on my MacOS):

import itk
import numpy as np

img1 = np.ones([128]*3).astype(np.float32)
img1 = itk.GetImageFromArray(img1)
itk.ImageFileWriter(FileName='img1.mha', Input=img1)
img2 = type(img1).New()
img2.Graft(img1)
del img1
np.zeros([128]*3).astype(np.float32)
itk.ImageFileWriter(FileName='img2.mha', Input=img2)

Problem; img1 and img2 differ and they should not (with Cuda, cudaMemcpy using img2’s CPU memory pointer hangs…). I have simple workarounds (keep img1) but I think it’s a bug. I see two solutions:

  • copy the member _ndarr when grafting (when it exists), but I don’t know how to do that with swig.
  • don’t copy the numpy array but create a buffer, copy the data and let the buffer container handle its memory.

I’m happy to try the second solution (or another suggestion) if you agree. What do you think?

2

@simon.rit, I think you are correct. I looked back at the history of the module and I was thinking that at some point that behaviour had been changed. However, I have not found any evidence of that. @matt.mccormick, has the buffer never been copied? I thought originally it was.
Either way, since an ITK Image is created, the image should be responsible of managing its content. This function should be improved to support allocating memory.

3

@simon.rit thanks for the nice example and description :+1:. And thanks for offering to contribute a patch :clap:. I do not think this use case has been accounted for: but this kind of advanced usage is awesome :star:.

Yes, we could improve the behavior for use with .Graft by your second solution: we can allocate an itk::Image in a method similar to the one that @fbudin pointed to. This image wiould have its own PixelContainer that will have its own reference count and lifetime, so it persists when used by img2.

Note that we will want to keep the original method around for GetImageViewFromArray, which avoids a memory copy.

4

As an additional comment, one option would be to create a new method that does the same as the existing one, and another option is to add an argument (boolean) to the existing method and depending on the value of this argument, allocate the memory or not.

5

Thanks for the suggestions. See my proposed patch with some words of discussion on the implementation:

2 Likes
6

Thanks @simon.rit . I have reviewed your PR.