fix: PNGConverterOperator uses the wrong axis dimension

[swederik]

Hi everyone,

After looking at the very interesting MONAI Bootcamp videos I wanted to test out the MONAI Deploy SDK and very quickly hit a minor issue. The PNG Converter is using the wrong image axis when it sets up the loop, so if you have an image where axis 0 and axis 2 have different lengths, it throws an error (along the lines of IndexError: index 261 is out of bounds for axis 2 with size 261) and fails to produce all of the PNGs.

Thanks!

[gigony]

Thanks @swederik for this PR!

@MMelQin could you please help review this change when you have time?

[MMelQin]

Works with loading a set of DICOM and

[MMelQin]

There is a fundamental issue with the Image class as the it lacks precise semantics of the dims of its encapsulated pixel image array, i.e. for a 3D image, which DIM is meant to be the axial/slice direction in DICOM speaking, first DIM to represent the instance, or the last DIM? What happens if some code created the Image with a pixel array that has the instance as the first DIM.

Anyway, we know the issue with this first version of Image. I had to pay close attention to the what the original image type is, and how and what image array is used when instantiating Image, e.g. DICOM to Volume get a image pixel array with depth dim (slice wise) first, but I had to change that code to swap the depth to last. There is also the swapping of width and height, to as to ensure the shape out of this image.asnumpy() would the same as those from ITK or Nibabel.

This PR may resolve the symptom somewhat, but it is at the mercy of the encapsulated image pixel array. I will ensure an issue is created to address the Image class; I don't think it is usable as intended, as it now simply provides an in-memory storage to the pertinent image array.

[dbericat]

@MMelQin reading your comment above, can you link the issue to this one, and either merge this PR or close it until a PR to solve the other bigger issue is ready? Thanks!

[MMelQin]

PR #238 attempts to provide consistency and deterministic representation of Image object loaded by DICOMSeriesToVolumeOperator from a DICOM series, and in doing so, we chose to be consistent with (Simple) ITK as it is one of most used module when loading DICOM instances.

So, the Image.asnumpy() is expected to have an array index order of DHW (and we can only ensure that the Image object creator, e.g. DICOMSeriesToVolumeOperator to ensure this is the case), same as Simple ITK GetArrayFromImage(). The origin,
spacing , and direction cosines are also equivalent to those returned by Simple ITK API's, GetOrigin(), GetSpacing(), GetDirection().

So, after #238 is merged and this PR is rebased, the Image.asnumpy.shape[0] will indeed be the depth (corresponding to the number of slices/instances), while 2nd and 3rd are H and W respectively. PIL fromarray expects the numpy array, and since the first index is really the depth, each slice i with index order HW then would [i, :, :]

[MMelQin]

@swederik thanks again for creating the PR to fix the indexing inconsistency problem. It would have worked if the inconsistency remained, though the ndarray contained in the Image object created by the DICOM to volume operator had been made consistent, with indexing order being DHW, the same as how ITK returns ndarray from its API serving out DICOM converted image.

I thought about branching out the PR but not possible, hence created a new PR #265 to adjust the indexing to fit the new convention. So this PR will not get merged.

[swederik]

Sounds good! Sorry I haven't had time to make any changes to this!