Unit tests for out argument to processors

CHANGELOG.md

@@ -6,10 +6,14 @@
   - Enhancements:
     - Use ravel instead of flat in KullbackLeibler numba backend (#1874)
     - Upgrade Python wrapper (#1873, #1875)
+    - Add checks on out argument passed to processors to ensure corrrect dtype and size (#1805)
+  - Testing:
+    - New unit tests for operators and functions to check for in place errors and the behaviour of `out` (#1805)
   - Bug fixes:
     - `ImageData` removes dimensions of size 1 from the input array. This fixes an issue where single slice reconstructions from 3D data would fail due to shape mismatches (#1885)
     - Make Binner accept accelerated=False (#1887)
 
+
 * 24.1.0
   - New Features:
     - Added method to plot filter in `GenericFilteredBackProjection` (#1667)

Wrappers/Python/cil/framework/framework.py

@@ -2588,7 +2588,9 @@ def get_order_by_label(self, dimension_labels, default_dimension_labels):
 
     def __eq__(self, other):
 
-        if isinstance(other, self.__class__) and self.config == other.config :
+        if isinstance(other, self.__class__) \
+            and self.config == other.config \
+            and self.dtype == other.dtype:
             return True
         return False
 
@@ -3871,6 +3873,7 @@ def __init__(self, **attributes):
         attributes['output'] = None
         attributes['shouldRun'] = True
         attributes['input'] = None
+        attributes['_shape_out'] = None,
 
         for key, value in attributes.items():
             self.__dict__[key] = value
@@ -3890,6 +3893,14 @@ def __setattr__(self, name, value):
                 self.__dict__['shouldRun'] = True
         else:
             raise KeyError('Attribute {0} not found'.format(name))
+
+    def _set_up(self):
+        """
+        Configure processor attributes that require the data to setup
+        Must set _shape_out
+        """
+        dataset = self.get_input()
+        self._shape_out = dataset.shape
 
     def set_input(self, dataset):
         """
@@ -3900,7 +3911,6 @@ def set_input(self, dataset):
         input : DataContainer
             The input DataContainer
         """
-
         if issubclass(type(dataset), DataContainer):
             if self.check_input(dataset):
                 self.__dict__['input'] = weakref.ref(dataset)
@@ -3910,7 +3920,8 @@ def set_input(self, dataset):
         else:
             raise TypeError("Input type mismatch: got {0} expecting {1}"\
                             .format(type(dataset), DataContainer))
-
+
+        self._set_up()
 
     def check_input(self, dataset):
         '''Checks parameters of the input DataContainer
@@ -3927,27 +3938,40 @@ def get_output(self, out=None):
         Parameters
         ----------
         out : DataContainer, optional
-           Fills the referenced DataContainer with the processed data and suppresses the return
+           Fills the referenced DataContainer with the processed data
 
         Returns
         -------
         DataContainer
-            The processed data. Suppressed if `out` is passed
+            The processed data
         """
+        if not self.check_output(out):
+            raise ValueError('Output data not compatible with processor')
+
         if self.output is None or self.shouldRun:
-            if out is None:
-                out = self.process()
-            else:
-                self.process(out=out)
-
+            out = self.process(out=out)
             if self.store_output:
                 self.output = out.copy()
-
             return out
-
         else:
-            return self.output.copy()
-
+            if out is not None:
+                out.fill(self.output)
+                return out
+        return self.output.copy()
+
+    def check_output(self, out):
+
+        if out is not None:
+            data = self.get_input()
+            if data.array.dtype != out.array.dtype:
+                raise TypeError("Input type mismatch: got {0} expecting {1}"\
+                            .format(out.array.dtype, data.array.dtype))
+
+            if self._shape_out != out.shape:
+                raise ValueError("out size mismatch: got {0} expecting {1}"\
+                                 .format(out.shape, self._shape_out))
+
+        return True
 
     def set_input_processor(self, processor):
         if issubclass(type(processor), DataProcessor):
@@ -3976,14 +4000,7 @@ def process(self, out=None):
     def __call__(self, x, out=None):
 
         self.set_input(x)
-
-        if out is None:
-            out = self.get_output()
-        else:
-            self.get_output(out=out)
-
-        return out
-
+        return self.get_output(out=out)
 
 class DataProcessor(Processor):
     '''Basically an alias of Processor Class'''
@@ -4030,17 +4047,20 @@ def __init__(self):
     def check_input(self, dataset):
         return True
 
+    def check_output(self, dataset):
+        return True
+
     def process(self, out=None):
 
         dsi = self.get_input()
         a = self.scalar
         if out is None:
-            y = DataContainer( a * dsi.as_array() , True,
+            out = DataContainer( a * dsi.as_array() , True,
                         dimension_labels=dsi.dimension_labels )
             #self.setParameter(output_dataset=y)
-            return y
         else:
             out.fill(a * dsi.as_array())
+        return out
 
 
 ###### Example of DataProcessors
@@ -4067,6 +4087,9 @@ def __init__(self, dtype=None):
 
     def check_input(self, dataset):
         return True
+
+    def check_output(self, dataset):
+        return True
 
     def process(self, out=None):
 

Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector2D.py

@@ -84,6 +84,13 @@ def check_input(self, dataset):
         else:
             raise ValueError("Expected input dimensions is 1 or 2, got {0}"\
                              .format(dataset.number_of_dimensions))
+
+    def _set_up(self):
+        """
+        Configure processor attributes that require the data to setup
+        Must set _shape_out
+        """
+        self._shape_out = self.volume_geometry.shape
 
     def set_projector(self, proj_id):
         self.proj_id = proj_id

Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector3D.py

@@ -56,13 +56,19 @@ def __init__(self,
 
         self.vol_geom, self.proj_geom = convert_geometry_to_astra_vec_3D(self.volume_geometry, self.sinogram_geometry)
 
-
     def check_input(self, dataset):
 
         if self.sinogram_geometry.shape != dataset.geometry.shape:
             raise ValueError("Dataset not compatible with geometry used to create the projector")
 
         return True
+
+    def _set_up(self):
+        """
+        Configure processor attributes that require the data to setup
+        Must set _shape_out
+        """
+        self._shape_out = self.volume_geometry.shape
 
     def set_ImageGeometry(self, volume_geometry):
 

Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector2D.py

@@ -86,6 +86,13 @@ def check_input(self, dataset):
             raise ValueError("Expected input dimensions is 1 or 2, got {0}"\
                              .format(dataset.number_of_dimensions))
 
+    def _set_up(self):
+        """
+        Configure processor attributes that require the data to setup
+        Must set _shape_out
+        """
+        self._shape_out = self.sinogram_geometry.shape
+
     def set_projector(self, proj_id):
         self.proj_id = proj_id
 

Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector3D.py

@@ -61,6 +61,13 @@ def check_input(self, dataset):
             raise ValueError("Dataset not compatible with geometry used to create the projector")
 
         return True
+
+    def _set_up(self):
+        """
+        Configure processor attributes that require the data to setup
+        Must set _shape_out
+        """
+        self._shape_out = self.sinogram_geometry.shape
 
     def set_ImageGeometry(self, volume_geometry):
 

Wrappers/Python/cil/plugins/astra/processors/FBP.py

@@ -107,6 +107,9 @@ def get_output(self, out=None):
 
     def check_input(self, dataset):
         return self.processor.check_input(dataset)
+
+    def check_output(self, out):
+        return self.processor.check_output(out=out)
 
     def process(self, out=None):
         return self.processor.process(out=out)

Wrappers/Python/cil/plugins/astra/processors/FBP_Flexible.py

@@ -149,7 +149,13 @@ def check_input(self, dataset):
             log.warning("The ASTRA backend FBP will use simple geometry only. Any configuration offsets or rotations may be ignored.")
 
         return True
-
+
+    def _set_up(self):
+        """
+        Configure processor attributes that require the data to setup
+        Must set _shape_out
+        """
+        self._shape_out = self.volume_geometry.shape
 
     def process(self, out=None):
 

Wrappers/Python/cil/plugins/astra/processors/FDK_Flexible.py

@@ -73,8 +73,14 @@ def check_input(self, dataset):
                  .format(self.sinogram_geometry.geom_type))
 
         return True
-
-
+
+    def _set_up(self):
+        """
+        Configure processor attributes that require the data to setup
+        Must set _shape_out
+        """
+        self._shape_out = self.volume_geometry.shape
+
     def process(self, out=None):
 
         # Get DATA

Wrappers/Python/cil/plugins/tigre/FBP.py

@@ -82,6 +82,13 @@ def check_input(self, dataset):
 
         DataOrder.check_order_for_engine('tigre', dataset.geometry)
         return True
+
+    def _set_up(self):
+        """
+        Configure processor attributes that require the data to setup
+        Must set _shape_out
+        """
+        self._shape_out = self.image_geometry.shape
 
     def process(self, out=None):
 

Wrappers/Python/cil/processors/Binner.py

@@ -104,13 +104,13 @@ def _configure(self):
         """
 
         #as binning we only include bins that are inside boundaries
-        self._shape_out = [int((x.stop - x.start)//x.step) for x in self._roi_ordered]
+        self._shape_out_full = [int((x.stop - x.start)//x.step) for x in self._roi_ordered]
         self._pixel_indices = []
 
         # fix roi_ordered for binner based on shape out
         for i in range(4):
             start = self._roi_ordered[i].start
-            stop = self._roi_ordered[i].start + self._shape_out[i] * self._roi_ordered[i].step
+            stop = self._roi_ordered[i].start + self._shape_out_full[i] * self._roi_ordered[i].step
 
             self._roi_ordered[i] = range(
                 start,
@@ -146,7 +146,7 @@ def _bin_array_numpy(self, array_in, array_binned):
 
         for i in range(4):
             # reshape the data to add each 'bin' dimensions
-            shape_object.append(self._shape_out[i])
+            shape_object.append(self._shape_out_full[i])
             shape_object.append(self._roi_ordered[i].step)
 
         shape_object = tuple(shape_object)
@@ -168,7 +168,7 @@ def _bin_array_acc(self, array_in, array_binned):
         indices_start = [x.start for x in self._roi_ordered]
         bins = [x.step for x in self._roi_ordered]
 
-        binner_ipp = Binner_IPP(self._shape_in, self._shape_out, indices_start, bins)
+        binner_ipp = Binner_IPP(self._shape_in, self._shape_out_full, indices_start, bins)
 
         res = binner_ipp.bin(array_in, array_binned)
         if res != 0:

Wrappers/Python/cil/processors/CofR_image_sharpness.py

@@ -358,3 +358,4 @@ def process(self, out=None):
             return AcquisitionData(array=data_full, deep_copy=True, geometry=new_geometry, supress_warning=True)
         else:
             out.geometry = new_geometry
+            return out

Wrappers/Python/cil/processors/CofR_xcorrelation.py

@@ -181,3 +181,4 @@ def process(self, out=None):
             return AcquisitionData(array = data_full, deep_copy = True, geometry = new_geometry, supress_warning=True)
         else:
             out.geometry = new_geometry
+            return out

Wrappers/Python/cil/processors/MaskGenerator.py

@@ -197,6 +197,15 @@ def check_input(self, data):
                                 "Expected {}, got {}.".format(data.dimension_labels, self.axis))
 
         return True
+
+    def check_output(self, out):
+        if out is not None:
+            if out.array.dtype != bool:
+                raise TypeError("Input type mismatch: got {0} expecting {1}"\
+                            .format(out.array.dtype, bool))
+
+        return True
+
 
     def process(self, out=None):
 
@@ -365,10 +374,11 @@ def process(self, out=None):
 
         if out is None:
             mask = numpy.asarray(mask, dtype=bool)
-            out = type(data)(mask, deep_copy=False, dtype=mask.dtype, geometry=data.geometry, suppress_warning=True, dimension_labels=data.dimension_labels)
-            return out
+            out = type(data)(mask, deep_copy=False, dtype=mask.dtype, geometry=data.geometry.copy(), suppress_warning=True, dimension_labels=data.dimension_labels)
         else:
             out.fill(mask)
+
+        return out
 
     def _parse_threshold_value(self, arr, quantile=False):
 

Wrappers/Python/cil/processors/Masker.py

@@ -171,12 +171,11 @@ def process(self, out=None):
 
         data = self.get_input()
 
-        return_arr = False
         if out is None:
             out = data.copy()
             arr = out.as_array()
-            return_arr = True
         else:
+            out.fill(data.as_array())
             arr = out.as_array()
 
         #assumes mask has 'as_array' method, i.e. is a DataContainer or is a numpy array
@@ -283,5 +282,4 @@ def process(self, out=None):
 
         out.fill(arr)
 
-        if return_arr is True:
-            return out
+        return out

Wrappers/Python/cil/processors/Normaliser.py

@@ -116,7 +116,12 @@ def process(self, out=None):
         elif projections.number_of_dimensions == 2:
             a = Normaliser.Normalise_projection(projections.as_array(),
                                                 flat, dark, self.tolerance)
-        y = type(projections)( a , True,
-                    dimension_labels=projections.dimension_labels,
-                    geometry=projections.geometry)
-        return y
+
+        if out is None:
+            out = type(projections)( a , True,
+                        dimension_labels=projections.dimension_labels,
+                        geometry=projections.geometry)
+        else:
+            out.fill(a)
+
+        return out