From fea2b4c0ae9eb1adfd79a683e3406d05e1e89da1 Mon Sep 17 00:00:00 2001 From: hrobarts Date: Tue, 14 May 2024 14:19:03 +0000 Subject: [PATCH 01/27] Setup tests --- Wrappers/Python/cil/processors/RingRemover.py | 3 +- Wrappers/Python/test/test_out_in_place.py | 94 +++++++++++++++++++ 2 files changed, 96 insertions(+), 1 deletion(-) diff --git a/Wrappers/Python/cil/processors/RingRemover.py b/Wrappers/Python/cil/processors/RingRemover.py index 59b0d8db5c..554632b99a 100644 --- a/Wrappers/Python/cil/processors/RingRemover.py +++ b/Wrappers/Python/cil/processors/RingRemover.py @@ -85,7 +85,8 @@ def process(self, out = None): vertical = geom.pixel_num_v # allocate datacontainer space - out = 0.*data + if out is None: + out = 0.*data # for non multichannel data if 'channel' not in geom.dimension_labels: diff --git a/Wrappers/Python/test/test_out_in_place.py b/Wrappers/Python/test/test_out_in_place.py index 4e23a2406a..1eaa7af0ec 100644 --- a/Wrappers/Python/test/test_out_in_place.py +++ b/Wrappers/Python/test/test_out_in_place.py @@ -38,6 +38,8 @@ IndicatorBox, TotalVariation, SumFunction, SumScalarFunction, \ WeightedL2NormSquared, MixedL11Norm, ZeroFunction +from cil.processors import AbsorptionTransmissionConverter, Binner, CentreOfRotationCorrector, MaskGenerator, Masker, Normaliser, Padder, \ +RingRemover, Slicer, TransmissionAbsorptionConverter import numpy @@ -313,3 +315,95 @@ def test_proximal_out(self): result=self.get_result(operator, 'adjoint', data) self.out_test(result, operator, 'adjoint', data) self.in_place_test(result,operator, 'adjoint', data) + +class TestProcessorOutandInPlace(CCPiTestClass): + def setUp(self): + + self.data_arrays=[np.random.normal(0,1, (10,20)).astype(np.float32), np.array(range(0,65500, 328), dtype=np.uint16).reshape((10,20)), np.random.uniform(0,1,(10,20)).astype(np.float32)] + + # processors that don't change the shape of the data + self.processor_test_list = [ + TransmissionAbsorptionConverter(min_intensity=0.01), + AbsorptionTransmissionConverter(), + RingRemover() + ] + + # Processors that require data as an input when the processor is created + # Normaliser(),# Masker() + # Normaliser(), + # Masker need to create the correct size mask + # mask = MaskGenerator.median(threshold_factor=3, window=7)(self.data_arrays[0]) + # Masker.interpolate(mask=mask) + # Normaliser needs flat and dark arrays + + # processors that don't necessarily return the same out + # [CentreOfRotationCorrector().xcorrelation(ang_tol=20), + # Slicer(), Binner(), Padder(pad_width=1)] + + ag_parallel_2D = AcquisitionGeometry.create_Parallel2D() + angles = np.linspace(0, 360, 10, dtype=np.float32) + ag_parallel_2D.set_angles(angles) + ag_parallel_2D.set_panel(20) + + ag_parallel_3D = AcquisitionGeometry.create_Parallel3D() + ag_parallel_3D.set_angles(angles) + ag_parallel_3D.set_panel([20,2]) + + ag_cone_2D = AcquisitionGeometry.create_Cone2D(source_position=[0,-10],detector_position=[0,10]) + ag_cone_2D.set_angles(angles) + ag_cone_2D.set_panel(20) + + ag_cone_3D = AcquisitionGeometry.create_Cone3D(source_position=[0,-10,0],detector_position=[0,10,0]) + ag_cone_3D.set_angles(angles) + ag_cone_3D.set_panel([20,2]) + + self.geometry_test_list = [ag_parallel_2D, ag_parallel_3D, ag_cone_2D, ag_cone_3D] + + def get_result(self, processor, data, *args): + input=data.copy() #To check that it isn't changed after function calls + try: + processor.set_input(data) + out = processor.get_output() + self.assertDataArraysInContainerAllClose(input, data, rtol=1e-5, msg= "In case processor.set_input(data), processor.get_output() where processor is " + processor.__class__.__name__+ " the input data has been incorrectly affected by the calculation.") + return out + except NotImplementedError: + print("get_result test not implemented for " + processor.__class__.__name__) + return None + + def in_place_test(self, desired_result, processor, data, *args, ): + out = data.copy() + try: + processor.set_input(data) + processor.get_output(out=out) + self.assertDataArraysInContainerAllClose(desired_result, out, rtol=1e-5, msg= "In place calculation failed for processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ "." ) + + except (InPlaceError, NotImplementedError): + print("in_place_test test not implemented for " + processor.__class__.__name__) + pass + + def out_test(self, desired_result, processor, data, *args, ): + input = data.copy() + out=0*(data.copy()) + try: + processor.set_input(input, *args) + processor.get_output(out=out) + + self.assertDataArraysInContainerAllClose(desired_result, out, rtol=1e-5, msg= "Calculation failed using processor.set_input(data), processor.get_output(out=out) where func is " + processor.__class__.__name__+ ".") + self.assertDataArraysInContainerAllClose(input, data, rtol=1e-5, msg= "In case processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ " the input data has been incorrectly affected by the calculation. ") + + except (InPlaceError, NotImplementedError): + print("in_place_test test not implemented for " + processor.__class__.__name__) + pass + + def test_out(self): + for processor in self.processor_test_list: + for geom in self.geometry_test_list: + for data_array in self.data_arrays: + data=geom.allocate(None) + try: + data.fill(data_array) + except: + data.fill(np.repeat(data_array[:,None, :], repeats=2, axis=1)) + result=self.get_result(processor, data) + self.out_test(result, processor, data) + self.in_place_test(result, processor, data) \ No newline at end of file From 4188f709e279c1cd5284026a70936f1f079157d9 Mon Sep 17 00:00:00 2001 From: hrobarts Date: Tue, 14 May 2024 17:19:34 +0000 Subject: [PATCH 02/27] Add Normaliser, MaskGenerator, Masker --- Wrappers/Python/cil/processors/Masker.py | 2 + Wrappers/Python/cil/processors/Normaliser.py | 13 ++- Wrappers/Python/test/test_out_in_place.py | 100 +++++++++++++------ 3 files changed, 78 insertions(+), 37 deletions(-) diff --git a/Wrappers/Python/cil/processors/Masker.py b/Wrappers/Python/cil/processors/Masker.py index c896803027..e2dab07290 100644 --- a/Wrappers/Python/cil/processors/Masker.py +++ b/Wrappers/Python/cil/processors/Masker.py @@ -177,7 +177,9 @@ def process(self, out=None): 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 try: diff --git a/Wrappers/Python/cil/processors/Normaliser.py b/Wrappers/Python/cil/processors/Normaliser.py index 1e8a12fb33..d11b47f226 100644 --- a/Wrappers/Python/cil/processors/Normaliser.py +++ b/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 diff --git a/Wrappers/Python/test/test_out_in_place.py b/Wrappers/Python/test/test_out_in_place.py index 1eaa7af0ec..4b20f62cee 100644 --- a/Wrappers/Python/test/test_out_in_place.py +++ b/Wrappers/Python/test/test_out_in_place.py @@ -22,7 +22,7 @@ import numpy as np from cil.utilities.errors import InPlaceError -from cil.framework import AcquisitionGeometry, ImageGeometry, VectorGeometry +from cil.framework import AcquisitionGeometry, ImageGeometry, VectorGeometry, DataContainer from cil.optimisation.operators import IdentityOperator, WaveletOperator from cil.optimisation.functions import KullbackLeibler, ConstantFunction, TranslateFunction, soft_shrinkage, L1Sparsity, BlockFunction @@ -319,26 +319,9 @@ def test_proximal_out(self): class TestProcessorOutandInPlace(CCPiTestClass): def setUp(self): - self.data_arrays=[np.random.normal(0,1, (10,20)).astype(np.float32), np.array(range(0,65500, 328), dtype=np.uint16).reshape((10,20)), np.random.uniform(0,1,(10,20)).astype(np.float32)] - - # processors that don't change the shape of the data - self.processor_test_list = [ - TransmissionAbsorptionConverter(min_intensity=0.01), - AbsorptionTransmissionConverter(), - RingRemover() - ] - - # Processors that require data as an input when the processor is created - # Normaliser(),# Masker() - # Normaliser(), - # Masker need to create the correct size mask - # mask = MaskGenerator.median(threshold_factor=3, window=7)(self.data_arrays[0]) - # Masker.interpolate(mask=mask) - # Normaliser needs flat and dark arrays - - # processors that don't necessarily return the same out - # [CentreOfRotationCorrector().xcorrelation(ang_tol=20), - # Slicer(), Binner(), Padder(pad_width=1)] + self.data_arrays=[np.random.normal(0,1, (10,20)).astype(np.float32), + np.array(range(0,65500, 328), dtype=np.uint16).reshape((10,20)), + np.random.uniform(0,1,(10,20)).astype(np.float32)] ag_parallel_2D = AcquisitionGeometry.create_Parallel2D() angles = np.linspace(0, 360, 10, dtype=np.float32) @@ -358,6 +341,23 @@ def setUp(self): ag_cone_3D.set_panel([20,2]) self.geometry_test_list = [ag_parallel_2D, ag_parallel_3D, ag_cone_2D, ag_cone_3D] + self.data_test_list= [geom.allocate(None) for geom in self.geometry_test_list] + + flat_field = self.data_test_list[0]*1 + dark_field = self.data_test_list[0]*1e-5 + + # processors that don't change the shape of the data + self.processor_list = [ + TransmissionAbsorptionConverter(min_intensity=0.01), + AbsorptionTransmissionConverter(), + RingRemover() + ] + + # processors that change the shape of the data + self.processor_list_diff_size = [Slicer(), + Binner(), + Padder(pad_width=1)] + def get_result(self, processor, data, *args): input=data.copy() #To check that it isn't changed after function calls @@ -370,7 +370,7 @@ def get_result(self, processor, data, *args): print("get_result test not implemented for " + processor.__class__.__name__) return None - def in_place_test(self, desired_result, processor, data, *args, ): + def in_place_test(self, desired_result, processor, data ): out = data.copy() try: processor.set_input(data) @@ -381,11 +381,11 @@ def in_place_test(self, desired_result, processor, data, *args, ): print("in_place_test test not implemented for " + processor.__class__.__name__) pass - def out_test(self, desired_result, processor, data, *args, ): + def out_test(self, desired_result, processor, data, output_size=None ): input = data.copy() out=0*(data.copy()) try: - processor.set_input(input, *args) + processor.set_input(input) processor.get_output(out=out) self.assertDataArraysInContainerAllClose(desired_result, out, rtol=1e-5, msg= "Calculation failed using processor.set_input(data), processor.get_output(out=out) where func is " + processor.__class__.__name__+ ".") @@ -396,14 +396,48 @@ def out_test(self, desired_result, processor, data, *args, ): pass def test_out(self): - for processor in self.processor_test_list: - for geom in self.geometry_test_list: - for data_array in self.data_arrays: - data=geom.allocate(None) - try: - data.fill(data_array) - except: - data.fill(np.repeat(data_array[:,None, :], repeats=2, axis=1)) + + for geom in self.geometry_test_list: + for data_array in self.data_arrays: + data=geom.allocate(None) + try: + data.fill(data_array) + except: + data.fill(np.repeat(data_array[:,None, :], repeats=2, axis=1)) + + for processor in self.processor_list: result=self.get_result(processor, data) self.out_test(result, processor, data) - self.in_place_test(result, processor, data) \ No newline at end of file + self.in_place_test(result, processor, data) + + # Test the processors that need data size as an input + processor = Normaliser(flat_field=data.get_slice(angle=0).as_array()*1, dark_field=data.get_slice(angle=0).as_array()*1e-5) + result=self.get_result(processor, data) + self.out_test(result, processor, data) + self.in_place_test(result, processor, data) + + processor = MaskGenerator.median(threshold_factor=3, window=7) + mask=self.get_result(processor, data) + self.out_test(mask, processor, data) + self.in_place_test(mask, processor, data) + + processor = Masker.median(mask=mask) + result=self.get_result(processor, data) + self.out_test(result, processor, data) + self.in_place_test(result, processor, data) + + # processor = CentreOfRotationCorrector() + # result=self.get_result(processor, data) + # self.out_test(result, processor, data) + # self.in_place_test(result, processor, data) + + def test_centre_of_rotation_out(self): + for geom in self.geometry_test_list: + for data_array in self.data_arrays: + processor = CentreOfRotationCorrector() + out = processor.get_output() + # result=self.get_result(processor, data) + # # self.out_test(result, processor, data) + # self.in_place_test(result, processor, data) + + # Test the processors that don't return the same size output \ No newline at end of file From 5faa20eb5fd954f1bec3baadf636c3b3f0c8f6ac Mon Sep 17 00:00:00 2001 From: hrobarts Date: Tue, 14 May 2024 17:22:41 +0000 Subject: [PATCH 03/27] Add CentreOfRotation --- Wrappers/Python/cil/processors/Masker.py | 1 - Wrappers/Python/test/test_out_in_place.py | 1 - 2 files changed, 2 deletions(-) diff --git a/Wrappers/Python/cil/processors/Masker.py b/Wrappers/Python/cil/processors/Masker.py index e2dab07290..00c793f37f 100644 --- a/Wrappers/Python/cil/processors/Masker.py +++ b/Wrappers/Python/cil/processors/Masker.py @@ -179,7 +179,6 @@ def process(self, out=None): 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 try: diff --git a/Wrappers/Python/test/test_out_in_place.py b/Wrappers/Python/test/test_out_in_place.py index 4b20f62cee..043ab9983a 100644 --- a/Wrappers/Python/test/test_out_in_place.py +++ b/Wrappers/Python/test/test_out_in_place.py @@ -435,7 +435,6 @@ def test_centre_of_rotation_out(self): for geom in self.geometry_test_list: for data_array in self.data_arrays: processor = CentreOfRotationCorrector() - out = processor.get_output() # result=self.get_result(processor, data) # # self.out_test(result, processor, data) # self.in_place_test(result, processor, data) From 707c084e2ebf63a445ecd0afe4b1cca398432330 Mon Sep 17 00:00:00 2001 From: hrobarts Date: Wed, 15 May 2024 09:03:43 +0000 Subject: [PATCH 04/27] Add Slicer, Binner, Padder --- Wrappers/Python/test/test_out_in_place.py | 76 +++++++++++++++-------- 1 file changed, 51 insertions(+), 25 deletions(-) diff --git a/Wrappers/Python/test/test_out_in_place.py b/Wrappers/Python/test/test_out_in_place.py index 043ab9983a..93aec1bf73 100644 --- a/Wrappers/Python/test/test_out_in_place.py +++ b/Wrappers/Python/test/test_out_in_place.py @@ -350,15 +350,19 @@ def setUp(self): self.processor_list = [ TransmissionAbsorptionConverter(min_intensity=0.01), AbsorptionTransmissionConverter(), - RingRemover() + RingRemover(), + Slicer(roi={'horizontal':(None,None,2),'angle':(None,None,2)}), + Binner(roi={'horizontal':(None,None,2),'angle':(None,None,2)}), + Padder(pad_width=1)] + self.processor_out_same_size = [ + True, + True, + True, + False, + False, + False ] - # processors that change the shape of the data - self.processor_list_diff_size = [Slicer(), - Binner(), - Padder(pad_width=1)] - - def get_result(self, processor, data, *args): input=data.copy() #To check that it isn't changed after function calls try: @@ -370,8 +374,11 @@ def get_result(self, processor, data, *args): print("get_result test not implemented for " + processor.__class__.__name__) return None - def in_place_test(self, desired_result, processor, data ): - out = data.copy() + def in_place_test(self, desired_result, processor, data, output_same_size=True): + if output_same_size==True: + out = data.copy() + else: + out=desired_result.copy() try: processor.set_input(data) processor.get_output(out=out) @@ -381,9 +388,13 @@ def in_place_test(self, desired_result, processor, data ): print("in_place_test test not implemented for " + processor.__class__.__name__) pass - def out_test(self, desired_result, processor, data, output_size=None ): + def out_test(self, desired_result, processor, data, output_same_size=True): input = data.copy() - out=0*(data.copy()) + if output_same_size==True: + out=0*(data.copy()) + else: + out=0*(desired_result.copy()) + try: processor.set_input(input) processor.get_output(out=out) @@ -392,7 +403,7 @@ def out_test(self, desired_result, processor, data, output_size=None ): self.assertDataArraysInContainerAllClose(input, data, rtol=1e-5, msg= "In case processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ " the input data has been incorrectly affected by the calculation. ") except (InPlaceError, NotImplementedError): - print("in_place_test test not implemented for " + processor.__class__.__name__) + print("out_test test not implemented for " + processor.__class__.__name__) pass def test_out(self): @@ -405,10 +416,12 @@ def test_out(self): except: data.fill(np.repeat(data_array[:,None, :], repeats=2, axis=1)) + i = 0 for processor in self.processor_list: result=self.get_result(processor, data) - self.out_test(result, processor, data) - self.in_place_test(result, processor, data) + self.out_test(result, processor, data, output_same_size=self.processor_out_same_size[i]) + self.in_place_test(result, processor, data, output_same_size=self.processor_out_same_size[i]) + i+=1 # Test the processors that need data size as an input processor = Normaliser(flat_field=data.get_slice(angle=0).as_array()*1, dark_field=data.get_slice(angle=0).as_array()*1e-5) @@ -425,18 +438,31 @@ def test_out(self): result=self.get_result(processor, data) self.out_test(result, processor, data) self.in_place_test(result, processor, data) - - # processor = CentreOfRotationCorrector() - # result=self.get_result(processor, data) - # self.out_test(result, processor, data) - # self.in_place_test(result, processor, data) def test_centre_of_rotation_out(self): - for geom in self.geometry_test_list: + for geom in self.geometry_test_list[0:2]: for data_array in self.data_arrays: - processor = CentreOfRotationCorrector() - # result=self.get_result(processor, data) - # # self.out_test(result, processor, data) - # self.in_place_test(result, processor, data) + data=geom.allocate(None) + try: + data.fill(data_array) + except: + data.fill(np.repeat(data_array[:,None, :], repeats=2, axis=1)) + processor = CentreOfRotationCorrector.xcorrelation(ang_tol=180) + result=self.get_result(processor, data) + # out_test fails because the processor only updates geometry, I think this is expected behaviour + # self.out_test(result, processor, data) + # test geometry instead + input = data.copy() + out=0*(data.copy()) + try: + processor.set_input(input) + processor.get_output(out=out) + + numpy.testing.assert_array_equal(result.geometry.config.system.rotation_axis.position, out.geometry.config.system.rotation_axis.position, err_msg= "Calculation failed using processor.set_input(data), processor.get_output(out=out) where func is " + processor.__class__.__name__+ ".") + numpy.testing.assert_array_equal(input.geometry.config.system.rotation_axis.position, data.geometry.config.system.rotation_axis.position, err_msg= "In case processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ " the input data has been incorrectly affected by the calculation. ") + + except (InPlaceError, NotImplementedError): + print("out_test_for_geometry test not implemented for " + processor.__class__.__name__) + pass - # Test the processors that don't return the same size output \ No newline at end of file + self.in_place_test(result, processor, data) \ No newline at end of file From 263fdd6ca6a4e2ac396556527d70a5b6918e912d Mon Sep 17 00:00:00 2001 From: hrobarts Date: Wed, 15 May 2024 09:42:06 +0000 Subject: [PATCH 05/27] Check out doesn't suppress return --- Wrappers/Python/cil/framework/framework.py | 4 ++-- Wrappers/Python/test/test_out_in_place.py | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Wrappers/Python/cil/framework/framework.py b/Wrappers/Python/cil/framework/framework.py index da02038b27..780ae25c2c 100644 --- a/Wrappers/Python/cil/framework/framework.py +++ b/Wrappers/Python/cil/framework/framework.py @@ -3898,12 +3898,12 @@ 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 self.output is None or self.shouldRun: if out is None: diff --git a/Wrappers/Python/test/test_out_in_place.py b/Wrappers/Python/test/test_out_in_place.py index 93aec1bf73..15ca388f35 100644 --- a/Wrappers/Python/test/test_out_in_place.py +++ b/Wrappers/Python/test/test_out_in_place.py @@ -397,10 +397,11 @@ def out_test(self, desired_result, processor, data, output_same_size=True): try: processor.set_input(input) - processor.get_output(out=out) + output = processor.get_output(out=out) self.assertDataArraysInContainerAllClose(desired_result, out, rtol=1e-5, msg= "Calculation failed using processor.set_input(data), processor.get_output(out=out) where func is " + processor.__class__.__name__+ ".") self.assertDataArraysInContainerAllClose(input, data, rtol=1e-5, msg= "In case processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ " the input data has been incorrectly affected by the calculation. ") + self.assertDataArraysInContainerAllClose(desired_result, output, rtol=1e-5, msg= "In case processor.set_input(data), output=processor.get_output(out=data) where processor is " + processor.__class__.__name__+ " the processor supresses the output. ") except (InPlaceError, NotImplementedError): print("out_test test not implemented for " + processor.__class__.__name__) @@ -456,10 +457,11 @@ def test_centre_of_rotation_out(self): out=0*(data.copy()) try: processor.set_input(input) - processor.get_output(out=out) + output = processor.get_output(out=out) numpy.testing.assert_array_equal(result.geometry.config.system.rotation_axis.position, out.geometry.config.system.rotation_axis.position, err_msg= "Calculation failed using processor.set_input(data), processor.get_output(out=out) where func is " + processor.__class__.__name__+ ".") numpy.testing.assert_array_equal(input.geometry.config.system.rotation_axis.position, data.geometry.config.system.rotation_axis.position, err_msg= "In case processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ " the input data has been incorrectly affected by the calculation. ") + self.assertDataArraysInContainerAllClose(out, output, rtol=1e-5, msg= "In case processor.set_input(data), output=processor.get_output(out=data) where processor is " + processor.__class__.__name__+ " the processor incorrectly supresses the output. ") except (InPlaceError, NotImplementedError): print("out_test_for_geometry test not implemented for " + processor.__class__.__name__) From 1966454e2f87f19aeb8ecb38f3d5a50fa317c3ca Mon Sep 17 00:00:00 2001 From: hrobarts Date: Wed, 15 May 2024 10:21:08 +0000 Subject: [PATCH 06/27] Remove old comment --- Wrappers/Python/test/test_out_in_place.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Wrappers/Python/test/test_out_in_place.py b/Wrappers/Python/test/test_out_in_place.py index 15ca388f35..66f08dedbe 100644 --- a/Wrappers/Python/test/test_out_in_place.py +++ b/Wrappers/Python/test/test_out_in_place.py @@ -346,7 +346,6 @@ def setUp(self): flat_field = self.data_test_list[0]*1 dark_field = self.data_test_list[0]*1e-5 - # processors that don't change the shape of the data self.processor_list = [ TransmissionAbsorptionConverter(min_intensity=0.01), AbsorptionTransmissionConverter(), From f005d3fbb7386e5b4d8e319c93a6053931980393 Mon Sep 17 00:00:00 2001 From: hrobarts Date: Thu, 16 May 2024 09:47:22 +0000 Subject: [PATCH 07/27] Review changes --- Wrappers/Python/test/test_out_in_place.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/Wrappers/Python/test/test_out_in_place.py b/Wrappers/Python/test/test_out_in_place.py index 66f08dedbe..3dc41f7de6 100644 --- a/Wrappers/Python/test/test_out_in_place.py +++ b/Wrappers/Python/test/test_out_in_place.py @@ -375,17 +375,15 @@ def get_result(self, processor, data, *args): def in_place_test(self, desired_result, processor, data, output_same_size=True): if output_same_size==True: - out = data.copy() - else: - out=desired_result.copy() - try: - processor.set_input(data) - processor.get_output(out=out) - self.assertDataArraysInContainerAllClose(desired_result, out, rtol=1e-5, msg= "In place calculation failed for processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ "." ) + + try: + processor.set_input(data) + processor.get_output(out=data) + self.assertDataArraysInContainerAllClose(desired_result, data, rtol=1e-5, msg= "In place calculation failed for processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ "." ) - except (InPlaceError, NotImplementedError): - print("in_place_test test not implemented for " + processor.__class__.__name__) - pass + except (InPlaceError, NotImplementedError): + print("in_place_test test not implemented for " + processor.__class__.__name__) + pass def out_test(self, desired_result, processor, data, output_same_size=True): input = data.copy() @@ -462,6 +460,10 @@ def test_centre_of_rotation_out(self): numpy.testing.assert_array_equal(input.geometry.config.system.rotation_axis.position, data.geometry.config.system.rotation_axis.position, err_msg= "In case processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ " the input data has been incorrectly affected by the calculation. ") self.assertDataArraysInContainerAllClose(out, output, rtol=1e-5, msg= "In case processor.set_input(data), output=processor.get_output(out=data) where processor is " + processor.__class__.__name__+ " the processor incorrectly supresses the output. ") + processor.set_input(data) + processor.get_output(out=data) + numpy.testing.assert_array_equal(result.geometry.config.system.rotation_axis.position, data.geometry.config.system.rotation_axis.position, err_msg= "Calculation failed using processor.set_input(data), processor.get_output(out=data) where func is " + processor.__class__.__name__+ ".") + except (InPlaceError, NotImplementedError): print("out_test_for_geometry test not implemented for " + processor.__class__.__name__) pass From 6ad68eac6bac7184afa7ebc5c74826996431be0b Mon Sep 17 00:00:00 2001 From: hrobarts Date: Mon, 20 May 2024 13:28:54 +0000 Subject: [PATCH 08/27] Remove comment --- Wrappers/Python/test/test_out_in_place.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/Wrappers/Python/test/test_out_in_place.py b/Wrappers/Python/test/test_out_in_place.py index 3dc41f7de6..d5cc0fbed2 100644 --- a/Wrappers/Python/test/test_out_in_place.py +++ b/Wrappers/Python/test/test_out_in_place.py @@ -343,9 +343,6 @@ def setUp(self): self.geometry_test_list = [ag_parallel_2D, ag_parallel_3D, ag_cone_2D, ag_cone_3D] self.data_test_list= [geom.allocate(None) for geom in self.geometry_test_list] - flat_field = self.data_test_list[0]*1 - dark_field = self.data_test_list[0]*1e-5 - self.processor_list = [ TransmissionAbsorptionConverter(min_intensity=0.01), AbsorptionTransmissionConverter(), From d65c54f1706abe271f7ee8cac58af45270722327 Mon Sep 17 00:00:00 2001 From: hrobarts Date: Wed, 22 May 2024 13:34:44 +0000 Subject: [PATCH 09/27] Add extra comments for new processors --- Wrappers/Python/test/test_out_in_place.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Wrappers/Python/test/test_out_in_place.py b/Wrappers/Python/test/test_out_in_place.py index d5cc0fbed2..c3227fdc21 100644 --- a/Wrappers/Python/test/test_out_in_place.py +++ b/Wrappers/Python/test/test_out_in_place.py @@ -343,6 +343,7 @@ def setUp(self): self.geometry_test_list = [ag_parallel_2D, ag_parallel_3D, ag_cone_2D, ag_cone_3D] self.data_test_list= [geom.allocate(None) for geom in self.geometry_test_list] + # Add new Processors here: self.processor_list = [ TransmissionAbsorptionConverter(min_intensity=0.01), AbsorptionTransmissionConverter(), @@ -350,6 +351,8 @@ def setUp(self): Slicer(roi={'horizontal':(None,None,2),'angle':(None,None,2)}), Binner(roi={'horizontal':(None,None,2),'angle':(None,None,2)}), Padder(pad_width=1)] + + # Specify whether new Processors should return the same size output here: self.processor_out_same_size = [ True, True, From 2947d1b34dbc3f60e0b894f6beea794b38fe05d6 Mon Sep 17 00:00:00 2001 From: hrobarts Date: Tue, 18 Jun 2024 08:37:43 +0000 Subject: [PATCH 10/27] Always return from processor --- Wrappers/Python/cil/framework/framework.py | 9 +++------ Wrappers/Python/cil/processors/MaskGenerator.py | 3 ++- Wrappers/Python/cil/processors/Masker.py | 5 +---- Wrappers/Python/cil/processors/Padder.py | 1 + Wrappers/Python/cil/processors/Slicer.py | 4 ++-- .../cil/processors/TransmissionAbsorptionConverter.py | 5 +---- 6 files changed, 10 insertions(+), 17 deletions(-) diff --git a/Wrappers/Python/cil/framework/framework.py b/Wrappers/Python/cil/framework/framework.py index 780ae25c2c..36ab8d8afb 100644 --- a/Wrappers/Python/cil/framework/framework.py +++ b/Wrappers/Python/cil/framework/framework.py @@ -3906,10 +3906,7 @@ def get_output(self, out=None): The processed data """ 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() @@ -4006,12 +4003,12 @@ 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 diff --git a/Wrappers/Python/cil/processors/MaskGenerator.py b/Wrappers/Python/cil/processors/MaskGenerator.py index 075010ef07..c81bc0fbac 100644 --- a/Wrappers/Python/cil/processors/MaskGenerator.py +++ b/Wrappers/Python/cil/processors/MaskGenerator.py @@ -366,9 +366,10 @@ 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 else: out.fill(mask) + + return out def _parse_threshold_value(self, arr, quantile=False): diff --git a/Wrappers/Python/cil/processors/Masker.py b/Wrappers/Python/cil/processors/Masker.py index 00c793f37f..4d83e7c1fe 100644 --- a/Wrappers/Python/cil/processors/Masker.py +++ b/Wrappers/Python/cil/processors/Masker.py @@ -171,11 +171,9 @@ 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() @@ -284,5 +282,4 @@ def process(self, out=None): out.fill(arr) - if return_arr is True: - return out + return out diff --git a/Wrappers/Python/cil/processors/Padder.py b/Wrappers/Python/cil/processors/Padder.py index 20b9fe481b..a9e18997a8 100644 --- a/Wrappers/Python/cil/processors/Padder.py +++ b/Wrappers/Python/cil/processors/Padder.py @@ -666,3 +666,4 @@ def process(self, out=None): raise ValueError("Geometry of `out` not as expected. Got {0}, expected {1}".format(out.geometry, new_geometry)) out.array = self._process_data(data) + return out diff --git a/Wrappers/Python/cil/processors/Slicer.py b/Wrappers/Python/cil/processors/Slicer.py index a2bada9b12..b3ab836823 100644 --- a/Wrappers/Python/cil/processors/Slicer.py +++ b/Wrappers/Python/cil/processors/Slicer.py @@ -430,5 +430,5 @@ def process(self, out=None): self._process_data(data, data_out) - if out is None: - return data_out + + return data_out diff --git a/Wrappers/Python/cil/processors/TransmissionAbsorptionConverter.py b/Wrappers/Python/cil/processors/TransmissionAbsorptionConverter.py index 57f3f64a84..b79deb1359 100644 --- a/Wrappers/Python/cil/processors/TransmissionAbsorptionConverter.py +++ b/Wrappers/Python/cil/processors/TransmissionAbsorptionConverter.py @@ -63,10 +63,8 @@ def process(self, out=None): data = self.get_input() - return_val = False if out is None: out = data.geometry.allocate(None) - return_val = True arr_in = data.as_array() arr_out = out.as_array() @@ -87,5 +85,4 @@ def process(self, out=None): out.fill(arr_out) - if return_val: - return out + return out From 90bbf8b3197989083927640d52d274a27436178c Mon Sep 17 00:00:00 2001 From: hrobarts Date: Tue, 18 Jun 2024 14:20:36 +0000 Subject: [PATCH 11/27] Tidy --- Wrappers/Python/cil/framework/framework.py | 9 +- .../cil/processors/CofR_xcorrelation.py | 1 + Wrappers/Python/test/test_out_in_place.py | 137 +++++++----------- 3 files changed, 58 insertions(+), 89 deletions(-) diff --git a/Wrappers/Python/cil/framework/framework.py b/Wrappers/Python/cil/framework/framework.py index 36ab8d8afb..283b5ad2a2 100644 --- a/Wrappers/Python/cil/framework/framework.py +++ b/Wrappers/Python/cil/framework/framework.py @@ -3944,14 +3944,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''' diff --git a/Wrappers/Python/cil/processors/CofR_xcorrelation.py b/Wrappers/Python/cil/processors/CofR_xcorrelation.py index 62fe47f4a9..9bf5860cbe 100644 --- a/Wrappers/Python/cil/processors/CofR_xcorrelation.py +++ b/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 diff --git a/Wrappers/Python/test/test_out_in_place.py b/Wrappers/Python/test/test_out_in_place.py index c3227fdc21..ad082ccbfe 100644 --- a/Wrappers/Python/test/test_out_in_place.py +++ b/Wrappers/Python/test/test_out_in_place.py @@ -342,67 +342,36 @@ def setUp(self): self.geometry_test_list = [ag_parallel_2D, ag_parallel_3D, ag_cone_2D, ag_cone_3D] self.data_test_list= [geom.allocate(None) for geom in self.geometry_test_list] - - # Add new Processors here: - self.processor_list = [ - TransmissionAbsorptionConverter(min_intensity=0.01), - AbsorptionTransmissionConverter(), - RingRemover(), - Slicer(roi={'horizontal':(None,None,2),'angle':(None,None,2)}), - Binner(roi={'horizontal':(None,None,2),'angle':(None,None,2)}), - Padder(pad_width=1)] - - # Specify whether new Processors should return the same size output here: - self.processor_out_same_size = [ - True, - True, - True, - False, - False, - False - ] - def get_result(self, processor, data, *args): + + def out_tests(self, processor, data, *args): input=data.copy() #To check that it isn't changed after function calls + try: processor.set_input(data) - out = processor.get_output() + out1 = processor.get_output() self.assertDataArraysInContainerAllClose(input, data, rtol=1e-5, msg= "In case processor.set_input(data), processor.get_output() where processor is " + processor.__class__.__name__+ " the input data has been incorrectly affected by the calculation.") - return out except NotImplementedError: print("get_result test not implemented for " + processor.__class__.__name__) - return None - - def in_place_test(self, desired_result, processor, data, output_same_size=True): - if output_same_size==True: - - try: - processor.set_input(data) - processor.get_output(out=data) - self.assertDataArraysInContainerAllClose(desired_result, data, rtol=1e-5, msg= "In place calculation failed for processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ "." ) - - except (InPlaceError, NotImplementedError): - print("in_place_test test not implemented for " + processor.__class__.__name__) - pass - - def out_test(self, desired_result, processor, data, output_same_size=True): - input = data.copy() - if output_same_size==True: - out=0*(data.copy()) - else: - out=0*(desired_result.copy()) - + try: - processor.set_input(input) - output = processor.get_output(out=out) - - self.assertDataArraysInContainerAllClose(desired_result, out, rtol=1e-5, msg= "Calculation failed using processor.set_input(data), processor.get_output(out=out) where func is " + processor.__class__.__name__+ ".") - self.assertDataArraysInContainerAllClose(input, data, rtol=1e-5, msg= "In case processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ " the input data has been incorrectly affected by the calculation. ") - self.assertDataArraysInContainerAllClose(desired_result, output, rtol=1e-5, msg= "In case processor.set_input(data), output=processor.get_output(out=data) where processor is " + processor.__class__.__name__+ " the processor supresses the output. ") + out2 = 0.*(out1.copy()) + out3 = processor.get_output(out=out2) + self.assertDataArraysInContainerAllClose(out1, out2, rtol=1e-5, msg= "Calculation failed using processor.set_input(data), processor.get_output(out=out) where func is " + processor.__class__.__name__+ ".") + self.assertDataArraysInContainerAllClose(input, data, rtol=1e-5, msg= "In case processor.set_input(data), processor.get_output(out=out) where processor is " + processor.__class__.__name__+ " the input data has been incorrectly affected by the calculation. ") + self.assertDataArraysInContainerAllClose(out1, out3, rtol=1e-5, msg= "In case processor.set_input(data), output=processor.get_output(out=out) where processor is " + processor.__class__.__name__+ " the processor supresses the output. ") + except NotImplementedError: + print("out_test test not implemented for " + processor.__class__.__name__) + try: + if out1.shape != data.shape: + with self.assertRaises(ValueError): + processor.get_output(out=data) + else: + processor.get_output(out=data) + self.assertDataArraysInContainerAllClose(out1, data, rtol=1e-5, msg= "In place calculation failed for processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ "." ) except (InPlaceError, NotImplementedError): - print("out_test test not implemented for " + processor.__class__.__name__) - pass + print("in_place_test test not implemented for " + processor.__class__.__name__) def test_out(self): @@ -413,30 +382,27 @@ def test_out(self): data.fill(data_array) except: data.fill(np.repeat(data_array[:,None, :], repeats=2, axis=1)) + + # Add new Processors here + # Works when out is same or different size to the data + processor_list = [ + TransmissionAbsorptionConverter(min_intensity=0.01), + AbsorptionTransmissionConverter(), + RingRemover(), + Slicer(roi={'horizontal':(None,None,None),'angle':(None,None,None)}), + Slicer(roi={'horizontal':(1,3,2),'angle':(None,4,2)}), + Binner(roi={'horizontal':(None,None,None),'angle':(None,None,None)}), + Binner(roi={'horizontal':(1,None,2),'angle':(None,4,2)}), + Padder(pad_width=0), + Padder(pad_width=1), + Normaliser(flat_field=data.get_slice(angle=0).as_array()*1, dark_field=data.get_slice(angle=0).as_array()*1e-5), + MaskGenerator.median(threshold_factor=3, window=7), + Masker.median(mask=data.copy()) + ] - i = 0 - for processor in self.processor_list: - result=self.get_result(processor, data) - self.out_test(result, processor, data, output_same_size=self.processor_out_same_size[i]) - self.in_place_test(result, processor, data, output_same_size=self.processor_out_same_size[i]) - i+=1 - - # Test the processors that need data size as an input - processor = Normaliser(flat_field=data.get_slice(angle=0).as_array()*1, dark_field=data.get_slice(angle=0).as_array()*1e-5) - result=self.get_result(processor, data) - self.out_test(result, processor, data) - self.in_place_test(result, processor, data) - - processor = MaskGenerator.median(threshold_factor=3, window=7) - mask=self.get_result(processor, data) - self.out_test(mask, processor, data) - self.in_place_test(mask, processor, data) - - processor = Masker.median(mask=mask) - result=self.get_result(processor, data) - self.out_test(result, processor, data) - self.in_place_test(result, processor, data) - + for processor in processor_list: + self.out_tests(processor, data) + def test_centre_of_rotation_out(self): for geom in self.geometry_test_list[0:2]: for data_array in self.data_arrays: @@ -446,26 +412,35 @@ def test_centre_of_rotation_out(self): except: data.fill(np.repeat(data_array[:,None, :], repeats=2, axis=1)) processor = CentreOfRotationCorrector.xcorrelation(ang_tol=180) - result=self.get_result(processor, data) + processor.set_input(data) + out1 = processor.get_output() # out_test fails because the processor only updates geometry, I think this is expected behaviour # self.out_test(result, processor, data) # test geometry instead input = data.copy() - out=0*(data.copy()) + out2 = 0*(out1.copy()) try: processor.set_input(input) - output = processor.get_output(out=out) + out3 = processor.get_output(out=out2) - numpy.testing.assert_array_equal(result.geometry.config.system.rotation_axis.position, out.geometry.config.system.rotation_axis.position, err_msg= "Calculation failed using processor.set_input(data), processor.get_output(out=out) where func is " + processor.__class__.__name__+ ".") + numpy.testing.assert_array_equal(out1.geometry.config.system.rotation_axis.position, out2.geometry.config.system.rotation_axis.position, err_msg= "Calculation failed using processor.set_input(data), processor.get_output(out=out) where func is " + processor.__class__.__name__+ ".") numpy.testing.assert_array_equal(input.geometry.config.system.rotation_axis.position, data.geometry.config.system.rotation_axis.position, err_msg= "In case processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ " the input data has been incorrectly affected by the calculation. ") - self.assertDataArraysInContainerAllClose(out, output, rtol=1e-5, msg= "In case processor.set_input(data), output=processor.get_output(out=data) where processor is " + processor.__class__.__name__+ " the processor incorrectly supresses the output. ") + self.assertDataArraysInContainerAllClose(out2, out3, rtol=1e-5, msg= "In case processor.set_input(data), output=processor.get_output(out=data) where processor is " + processor.__class__.__name__+ " the processor incorrectly supresses the output. ") processor.set_input(data) processor.get_output(out=data) - numpy.testing.assert_array_equal(result.geometry.config.system.rotation_axis.position, data.geometry.config.system.rotation_axis.position, err_msg= "Calculation failed using processor.set_input(data), processor.get_output(out=data) where func is " + processor.__class__.__name__+ ".") + numpy.testing.assert_array_equal(out1.geometry.config.system.rotation_axis.position, data.geometry.config.system.rotation_axis.position, err_msg= "Calculation failed using processor.set_input(data), processor.get_output(out=data) where func is " + processor.__class__.__name__+ ".") except (InPlaceError, NotImplementedError): print("out_test_for_geometry test not implemented for " + processor.__class__.__name__) pass - self.in_place_test(result, processor, data) \ No newline at end of file + try: + if out1.shape != data.shape: + with self.assertRaises(ValueError): + processor.get_output(out=data) + else: + processor.get_output(out=data) + self.assertDataArraysInContainerAllClose(out1, data, rtol=1e-5, msg= "In place calculation failed for processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ "." ) + except (InPlaceError, NotImplementedError): + print("in_place_test test not implemented for " + processor.__class__.__name__) \ No newline at end of file From 86a675f77f734608ae377cf8a75b7bb43c3b00c8 Mon Sep 17 00:00:00 2001 From: hrobarts Date: Tue, 18 Jun 2024 14:28:29 +0000 Subject: [PATCH 12/27] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46eea1b49a..904ddf15f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - Operators and functions now also return when out is specified (#1742) - The CIL function class now has a `__neg__` function, so you can write `-YourFunction(x)` rather than `-1*YourFunction(x)` (#1808) - Added CIL vs SIRF tests comparing preconditioned ISTA in CIL and MLEM in SIRF (#1823) + - New unit tests for operators and functions to check for in place errors and the behaviour of `out` (#1805) - Bug fixes: - gradient descent `update_objective` called twice on the initial point.(#1789) - ProjectionMap operator bug fix in adjoint and added documentation (#1743) From 1392dd96c42d8db357a9816ae0bfddc72ce68545 Mon Sep 17 00:00:00 2001 From: hrobarts Date: Wed, 24 Jul 2024 10:20:21 +0000 Subject: [PATCH 13/27] Update tests and add CofR image sharpness --- .../cil/processors/CofR_image_sharpness.py | 1 + Wrappers/Python/test/test_out_in_place.py | 103 +++++++++++++++--- 2 files changed, 87 insertions(+), 17 deletions(-) diff --git a/Wrappers/Python/cil/processors/CofR_image_sharpness.py b/Wrappers/Python/cil/processors/CofR_image_sharpness.py index 31b62f0e77..20db1e9d74 100644 --- a/Wrappers/Python/cil/processors/CofR_image_sharpness.py +++ b/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 \ No newline at end of file diff --git a/Wrappers/Python/test/test_out_in_place.py b/Wrappers/Python/test/test_out_in_place.py index ad082ccbfe..3baaef9238 100644 --- a/Wrappers/Python/test/test_out_in_place.py +++ b/Wrappers/Python/test/test_out_in_place.py @@ -22,6 +22,7 @@ import numpy as np from cil.utilities.errors import InPlaceError +from cil.utilities import dataexample from cil.framework import AcquisitionGeometry, ImageGeometry, VectorGeometry, DataContainer from cil.optimisation.operators import IdentityOperator, WaveletOperator @@ -341,29 +342,48 @@ def setUp(self): ag_cone_3D.set_panel([20,2]) self.geometry_test_list = [ag_parallel_2D, ag_parallel_3D, ag_cone_2D, ag_cone_3D] - self.data_test_list= [geom.allocate(None) for geom in self.geometry_test_list] - def out_tests(self, processor, data, *args): - input=data.copy() #To check that it isn't changed after function calls + def fail(self, error_message): + raise NotImplementedError(error_message) + def get_result_check(self, processor, data, *args): + # Check the original data is not modified when the processor is called + + data_original=data.copy() #To check that data isn't changed after function calls try: processor.set_input(data) out1 = processor.get_output() - self.assertDataArraysInContainerAllClose(input, data, rtol=1e-5, msg= "In case processor.set_input(data), processor.get_output() where processor is " + processor.__class__.__name__+ " the input data has been incorrectly affected by the calculation.") + self.assertDataArraysInContainerAllClose(data_original, data, rtol=1e-5, msg= "In case processor.set_input(data), processor.get_output() where processor is " + processor.__class__.__name__+ " the input data has been incorrectly affected by the calculation.") except NotImplementedError: - print("get_result test not implemented for " + processor.__class__.__name__) - + self.fail("get_result test not implemented for " + processor.__class__.__name__) + + def out_check(self, processor, data, *args): + # Check the processor gives the same result using the out argument + # Also check the out argument doesn't change the orginal data + # and that the processor still returns correctly with the out argument + + data_original=data.copy() #To check that data isn't changed after function calls try: + processor.set_input(data) + out1 = processor.get_output() out2 = 0.*(out1.copy()) out3 = processor.get_output(out=out2) self.assertDataArraysInContainerAllClose(out1, out2, rtol=1e-5, msg= "Calculation failed using processor.set_input(data), processor.get_output(out=out) where func is " + processor.__class__.__name__+ ".") - self.assertDataArraysInContainerAllClose(input, data, rtol=1e-5, msg= "In case processor.set_input(data), processor.get_output(out=out) where processor is " + processor.__class__.__name__+ " the input data has been incorrectly affected by the calculation. ") + self.assertDataArraysInContainerAllClose(data_original, data, rtol=1e-5, msg= "In case processor.set_input(data), processor.get_output(out=out) where processor is " + processor.__class__.__name__+ " the input data has been incorrectly affected by the calculation. ") self.assertDataArraysInContainerAllClose(out1, out3, rtol=1e-5, msg= "In case processor.set_input(data), output=processor.get_output(out=out) where processor is " + processor.__class__.__name__+ " the processor supresses the output. ") except NotImplementedError: - print("out_test test not implemented for " + processor.__class__.__name__) - + self.fail("out_test test not implemented for " + processor.__class__.__name__) + + def in_place_check(self, processor, data, *args): + # Check the processor gives the correct result if the calculation is performed in place + # i.e. data is passed to the out argument + # if the processor output is a different sized array, the calcualtion + # cannot be perfomed in place, so we expect this to raise a ValueError + try: + processor.set_input(data) + out1 = processor.get_output() if out1.shape != data.shape: with self.assertRaises(ValueError): processor.get_output(out=data) @@ -371,10 +391,11 @@ def out_tests(self, processor, data, *args): processor.get_output(out=data) self.assertDataArraysInContainerAllClose(out1, data, rtol=1e-5, msg= "In place calculation failed for processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ "." ) except (InPlaceError, NotImplementedError): - print("in_place_test test not implemented for " + processor.__class__.__name__) + self.fail("in_place_test test not implemented for " + processor.__class__.__name__) def test_out(self): + # perform the tests on different data and geometry types for geom in self.geometry_test_list: for data_array in self.data_arrays: data=geom.allocate(None) @@ -384,7 +405,6 @@ def test_out(self): data.fill(np.repeat(data_array[:,None, :], repeats=2, axis=1)) # Add new Processors here - # Works when out is same or different size to the data processor_list = [ TransmissionAbsorptionConverter(min_intensity=0.01), AbsorptionTransmissionConverter(), @@ -401,9 +421,15 @@ def test_out(self): ] for processor in processor_list: - self.out_tests(processor, data) + self.get_result_check(processor, data) + self.out_check(processor, data) + self.in_place_check(processor, data) - def test_centre_of_rotation_out(self): + def test_centre_of_rotation_xcorrelation_out(self): + # Test the centre of rotation processor + # These tests are different because the processor retusrns changes to the geometry rather than the data + + # First test CentreOfRotationCorrector.xcorrelation() on parallel beam data only for geom in self.geometry_test_list[0:2]: for data_array in self.data_arrays: data=geom.allocate(None) @@ -414,7 +440,7 @@ def test_centre_of_rotation_out(self): processor = CentreOfRotationCorrector.xcorrelation(ang_tol=180) processor.set_input(data) out1 = processor.get_output() - # out_test fails because the processor only updates geometry, I think this is expected behaviour + # out_test fails because the processor only updates geometry, this is expected behaviour # self.out_test(result, processor, data) # test geometry instead input = data.copy() @@ -423,8 +449,8 @@ def test_centre_of_rotation_out(self): processor.set_input(input) out3 = processor.get_output(out=out2) - numpy.testing.assert_array_equal(out1.geometry.config.system.rotation_axis.position, out2.geometry.config.system.rotation_axis.position, err_msg= "Calculation failed using processor.set_input(data), processor.get_output(out=out) where func is " + processor.__class__.__name__+ ".") - numpy.testing.assert_array_equal(input.geometry.config.system.rotation_axis.position, data.geometry.config.system.rotation_axis.position, err_msg= "In case processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ " the input data has been incorrectly affected by the calculation. ") + numpy.testing.assert_allclose(out1.geometry.config.system.rotation_axis.position, out2.geometry.config.system.rotation_axis.position, err_msg= "Calculation failed using processor.set_input(data), processor.get_output(out=out) where func is " + processor.__class__.__name__+ ".") + numpy.testing.assert_allclose(input.geometry.config.system.rotation_axis.position, data.geometry.config.system.rotation_axis.position, err_msg= "In case processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ " the input data has been incorrectly affected by the calculation. ") self.assertDataArraysInContainerAllClose(out2, out3, rtol=1e-5, msg= "In case processor.set_input(data), output=processor.get_output(out=data) where processor is " + processor.__class__.__name__+ " the processor incorrectly supresses the output. ") processor.set_input(data) @@ -443,4 +469,47 @@ def test_centre_of_rotation_out(self): processor.get_output(out=data) self.assertDataArraysInContainerAllClose(out1, data, rtol=1e-5, msg= "In place calculation failed for processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ "." ) except (InPlaceError, NotImplementedError): - print("in_place_test test not implemented for " + processor.__class__.__name__) \ No newline at end of file + print("in_place_test test not implemented for " + processor.__class__.__name__) + + def test_centre_of_rotation_image_sharpness_out(self): + # Next test CentreOfRotationCorrector.image_sharpness() on parallel beam data + # - here we need to use an image rather than random data + data = dataexample.SIMULATED_PARALLEL_BEAM_DATA.get() + + processor = Binner(roi={'horizontal':(0,-1,2),'vertical':(0,-1,2), 'angle':(0,-1,2)}) + processor.set_input(data) + data = processor.get_output() + + processor = CentreOfRotationCorrector.image_sharpness() + processor.set_input(data) + out1 = processor.get_output() + # out_test fails because the processor only updates geometry, this is expected behaviour + # self.out_test(result, processor, data) + # test geometry instead + input = data.copy() + out2 = 0*(out1.copy()) + try: + processor.set_input(input) + out3 = processor.get_output(out=out2) + + numpy.testing.assert_allclose(out1.geometry.config.system.rotation_axis.position, out2.geometry.config.system.rotation_axis.position, err_msg= "Calculation failed using processor.set_input(data), processor.get_output(out=out) where func is " + processor.__class__.__name__+ ".") + numpy.testing.assert_allclose(input.geometry.config.system.rotation_axis.position, data.geometry.config.system.rotation_axis.position, err_msg= "In case processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ " the input data has been incorrectly affected by the calculation. ") + self.assertDataArraysInContainerAllClose(out2, out3, rtol=1e-5, msg= "In case processor.set_input(data), output=processor.get_output(out=data) where processor is " + processor.__class__.__name__+ " the processor incorrectly supresses the output. ") + + processor.set_input(data) + processor.get_output(out=data) + numpy.testing.assert_array_equal(out1.geometry.config.system.rotation_axis.position, data.geometry.config.system.rotation_axis.position, err_msg= "Calculation failed using processor.set_input(data), processor.get_output(out=data) where func is " + processor.__class__.__name__+ ".") + + except (InPlaceError, NotImplementedError): + self.fail("out_test_for_geometry test not implemented for " + processor.__class__.__name__) + pass + + try: + if out1.shape != data.shape: + with self.assertRaises(ValueError): + processor.get_output(out=data) + else: + processor.get_output(out=data) + self.assertDataArraysInContainerAllClose(out1, data, rtol=1e-5, msg= "In place calculation failed for processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ "." ) + except (InPlaceError, NotImplementedError): + self.fail("in_place_test test not implemented for " + processor.__class__.__name__) \ No newline at end of file From e782e812d301b24f8f8060ef7d900138da00a4f1 Mon Sep 17 00:00:00 2001 From: hrobarts Date: Wed, 24 Jul 2024 10:50:27 +0000 Subject: [PATCH 14/27] Add docstrings --- Wrappers/Python/test/test_out_in_place.py | 54 +++++++++++++++-------- 1 file changed, 36 insertions(+), 18 deletions(-) diff --git a/Wrappers/Python/test/test_out_in_place.py b/Wrappers/Python/test/test_out_in_place.py index 3baaef9238..d9bd62b59e 100644 --- a/Wrappers/Python/test/test_out_in_place.py +++ b/Wrappers/Python/test/test_out_in_place.py @@ -348,8 +348,9 @@ def fail(self, error_message): raise NotImplementedError(error_message) def get_result_check(self, processor, data, *args): - # Check the original data is not modified when the processor is called - + """ + Tests to check the original data is not modified when the processor is called + """ data_original=data.copy() #To check that data isn't changed after function calls try: processor.set_input(data) @@ -359,10 +360,11 @@ def get_result_check(self, processor, data, *args): self.fail("get_result test not implemented for " + processor.__class__.__name__) def out_check(self, processor, data, *args): - # Check the processor gives the same result using the out argument - # Also check the out argument doesn't change the orginal data - # and that the processor still returns correctly with the out argument - + """ + Tests to check the processor gives the same result using the out argument + Also check the out argument doesn't change the orginal data + and that the processor still returns correctly with the out argument + """ data_original=data.copy() #To check that data isn't changed after function calls try: processor.set_input(data) @@ -376,11 +378,12 @@ def out_check(self, processor, data, *args): self.fail("out_test test not implemented for " + processor.__class__.__name__) def in_place_check(self, processor, data, *args): - # Check the processor gives the correct result if the calculation is performed in place - # i.e. data is passed to the out argument - # if the processor output is a different sized array, the calcualtion - # cannot be perfomed in place, so we expect this to raise a ValueError - + """ + Check the processor gives the correct result if the calculation is performed in place + i.e. data is passed to the out argument + if the processor output is a different sized array, the calcualtion + cannot be perfomed in place, so we expect this to raise a ValueError + """ try: processor.set_input(data) out1 = processor.get_output() @@ -394,7 +397,16 @@ def in_place_check(self, processor, data, *args): self.fail("in_place_test test not implemented for " + processor.__class__.__name__) def test_out(self): - + """ + Tests to check output from Processors, including: + - Checks that Processors give the same results when the result is obtained + directly or using the out argument, or in place + - Checks that the out argument doesn't change the original data + - Checks that Processors still return the output even when out is used + All the tests are performed on different data types, geometry and Processors + Any new Processors should be added to the processor_list + """ + # perform the tests on different data and geometry types for geom in self.geometry_test_list: for data_array in self.data_arrays: @@ -426,10 +438,12 @@ def test_out(self): self.in_place_check(processor, data) def test_centre_of_rotation_xcorrelation_out(self): - # Test the centre of rotation processor - # These tests are different because the processor retusrns changes to the geometry rather than the data - - # First test CentreOfRotationCorrector.xcorrelation() on parallel beam data only + """ + Test the output of the centre of rotation xcorrelation processor + These tests are performed separately because the processor returns + changes to the geometry rather than the data + """ + # CentreOfRotationCorrector.xcorrelation() works on parallel beam data only for geom in self.geometry_test_list[0:2]: for data_array in self.data_arrays: data=geom.allocate(None) @@ -472,8 +486,12 @@ def test_centre_of_rotation_xcorrelation_out(self): print("in_place_test test not implemented for " + processor.__class__.__name__) def test_centre_of_rotation_image_sharpness_out(self): - # Next test CentreOfRotationCorrector.image_sharpness() on parallel beam data - # - here we need to use an image rather than random data + """ + Test the output of the centre of rotation image_sharpness processor + These tests are performed separately because the processor returns + changes to the geometry rather than the data + """ + # Here we need to use real data rather than random data otherwise the processor fails data = dataexample.SIMULATED_PARALLEL_BEAM_DATA.get() processor = Binner(roi={'horizontal':(0,-1,2),'vertical':(0,-1,2), 'angle':(0,-1,2)}) From f04cbed49650e925071d79fba5bb90fafa68452d Mon Sep 17 00:00:00 2001 From: hrobarts Date: Wed, 24 Jul 2024 10:58:05 +0000 Subject: [PATCH 15/27] Check for TIGRE and GPU in image sharpness test --- Wrappers/Python/test/test_out_in_place.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Wrappers/Python/test/test_out_in_place.py b/Wrappers/Python/test/test_out_in_place.py index d9bd62b59e..184c35af88 100644 --- a/Wrappers/Python/test/test_out_in_place.py +++ b/Wrappers/Python/test/test_out_in_place.py @@ -43,6 +43,7 @@ RingRemover, Slicer, TransmissionAbsorptionConverter import numpy +from utils import has_tigre, has_nvidia from cil.framework import BlockGeometry @@ -485,6 +486,7 @@ def test_centre_of_rotation_xcorrelation_out(self): except (InPlaceError, NotImplementedError): print("in_place_test test not implemented for " + processor.__class__.__name__) + @unittest.skipUnless(has_tigre and has_nvidia, "TIGRE GPU not installed") def test_centre_of_rotation_image_sharpness_out(self): """ Test the output of the centre of rotation image_sharpness processor From 744b85050ca1256084f69d597273cc3addfd6f6e Mon Sep 17 00:00:00 2001 From: hrobarts Date: Wed, 24 Jul 2024 15:41:03 +0000 Subject: [PATCH 16/27] Add tests for PaganinProccesor --- .../Python/cil/processors/PaganinProcessor.py | 16 +++++++++---- Wrappers/Python/test/test_out_in_place.py | 23 +++++++++++-------- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/Wrappers/Python/cil/processors/PaganinProcessor.py b/Wrappers/Python/cil/processors/PaganinProcessor.py index 33ad87841b..f4127b5262 100644 --- a/Wrappers/Python/cil/processors/PaganinProcessor.py +++ b/Wrappers/Python/cil/processors/PaganinProcessor.py @@ -216,7 +216,7 @@ def process(self, out=None): self._set_geometry(data.geometry, self.override_geometry) if out is None: - out = data.geometry.allocate(None) + out = data.copy() # make slice indices to get the projection slice_proj = [slice(None)]*len(data.shape) @@ -232,7 +232,7 @@ def process(self, out=None): data_proj = data.as_array()[tuple(slice_proj)] # create an empty axis if the data is 2D - if len(data_proj.shape) == 1: + if len(data.shape) == 2: data.array = np.expand_dims(data.array, len(data.shape)) slice_proj.append(slice(None)) data_proj = data.as_array()[tuple(slice_proj)] @@ -242,6 +242,9 @@ def process(self, out=None): else: raise(ValueError('Data must be 2D or 3D per channel')) + if len(out.shape) == 2: + out.array = np.expand_dims(out.array, len(out.shape)) + # create a filter based on the shape of the data filter_shape = np.shape(data_proj) self.filter_Nx = filter_shape[0]+self.pad*2 @@ -265,7 +268,7 @@ def process(self, out=None): if channel_axis is not None: slice_proj[channel_axis] = j # loop over the projections - for i in tqdm(range(len(out.geometry.angles))): + for i in tqdm(range(len(data.geometry.angles))): slice_proj[angle_axis] = i padded_buffer[slice_pad] = data.array[(tuple(slice_proj))] @@ -282,11 +285,14 @@ def process(self, out=None): fI = fft2(padded_buffer) padded_buffer = ifft2(fI*self.filter) if data.geometry.channels>1: - out.fill(np.squeeze(padded_buffer[slice_pad]), angle = i, + out.fill(padded_buffer[slice_pad], angle = i, channel=j) else: - out.fill(np.squeeze(padded_buffer[slice_pad]), angle = i) + x = padded_buffer[slice_pad] + out.fill(x, angle = i) + data.array = np.squeeze(data.array) + out.array = np.squeeze(out.array) return out def set_input(self, dataset): diff --git a/Wrappers/Python/test/test_out_in_place.py b/Wrappers/Python/test/test_out_in_place.py index 184c35af88..2fac5754df 100644 --- a/Wrappers/Python/test/test_out_in_place.py +++ b/Wrappers/Python/test/test_out_in_place.py @@ -40,7 +40,7 @@ WeightedL2NormSquared, MixedL11Norm, ZeroFunction from cil.processors import AbsorptionTransmissionConverter, Binner, CentreOfRotationCorrector, MaskGenerator, Masker, Normaliser, Padder, \ -RingRemover, Slicer, TransmissionAbsorptionConverter +RingRemover, Slicer, TransmissionAbsorptionConverter, PaganinProcessor import numpy from utils import has_tigre, has_nvidia @@ -325,22 +325,25 @@ def setUp(self): np.array(range(0,65500, 328), dtype=np.uint16).reshape((10,20)), np.random.uniform(0,1,(10,20)).astype(np.float32)] - ag_parallel_2D = AcquisitionGeometry.create_Parallel2D() + ag_parallel_2D = AcquisitionGeometry.create_Parallel2D(detector_position=[0,1], units='m') angles = np.linspace(0, 360, 10, dtype=np.float32) ag_parallel_2D.set_angles(angles) ag_parallel_2D.set_panel(20) - ag_parallel_3D = AcquisitionGeometry.create_Parallel3D() + ag_parallel_3D = AcquisitionGeometry.create_Parallel3D(detector_position=[0,1,0], units='mm') + # ag = AcquisitionGeometry.create_Parallel3D(detector_position=[0, 1, 0], units='mm').set_panel([20,2], pixel_size=0.1).set_angles(angles) ag_parallel_3D.set_angles(angles) - ag_parallel_3D.set_panel([20,2]) + ag_parallel_3D.set_panel([20,2], pixel_size=0.1) - ag_cone_2D = AcquisitionGeometry.create_Cone2D(source_position=[0,-10],detector_position=[0,10]) + ag_cone_2D = AcquisitionGeometry.create_Cone2D(source_position=[0,-10],detector_position=[0,10], units='mm') ag_cone_2D.set_angles(angles) ag_cone_2D.set_panel(20) - ag_cone_3D = AcquisitionGeometry.create_Cone3D(source_position=[0,-10,0],detector_position=[0,10,0]) + ag_cone_3D = AcquisitionGeometry.create_Cone3D(source_position=[0,-10,0],detector_position=[0,10,0], units='mm') ag_cone_3D.set_angles(angles) - ag_cone_3D.set_panel([20,2]) + ag_cone_3D.set_panel([20,2], pixel_size=0.1) + + ag = AcquisitionGeometry.create_Parallel3D(detector_position=[0, 150, 0], units='mm').set_panel([20,2], pixel_size=0.1).set_angles(angles) self.geometry_test_list = [ag_parallel_2D, ag_parallel_3D, ag_cone_2D, ag_cone_3D] @@ -416,7 +419,6 @@ def test_out(self): data.fill(data_array) except: data.fill(np.repeat(data_array[:,None, :], repeats=2, axis=1)) - # Add new Processors here processor_list = [ TransmissionAbsorptionConverter(min_intensity=0.01), @@ -430,8 +432,9 @@ def test_out(self): Padder(pad_width=1), Normaliser(flat_field=data.get_slice(angle=0).as_array()*1, dark_field=data.get_slice(angle=0).as_array()*1e-5), MaskGenerator.median(threshold_factor=3, window=7), - Masker.median(mask=data.copy()) - ] + Masker.median(mask=data.copy()), + PaganinProcessor() + ] for processor in processor_list: self.get_result_check(processor, data) From 0ec5ea413ec0d5aa0dc5314f3eed025bdb6e44fc Mon Sep 17 00:00:00 2001 From: hrobarts Date: Mon, 29 Jul 2024 12:56:39 +0000 Subject: [PATCH 17/27] Remove data.copy() --- Wrappers/Python/cil/processors/PaganinProcessor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Wrappers/Python/cil/processors/PaganinProcessor.py b/Wrappers/Python/cil/processors/PaganinProcessor.py index f4127b5262..f85c013e6b 100644 --- a/Wrappers/Python/cil/processors/PaganinProcessor.py +++ b/Wrappers/Python/cil/processors/PaganinProcessor.py @@ -216,7 +216,7 @@ def process(self, out=None): self._set_geometry(data.geometry, self.override_geometry) if out is None: - out = data.copy() + out = data.geometry.allocate(None) # make slice indices to get the projection slice_proj = [slice(None)]*len(data.shape) From 78805efe139d2c6a5bb21b4151a0b7772c923d0f Mon Sep 17 00:00:00 2001 From: hrobarts Date: Wed, 31 Jul 2024 08:24:24 +0000 Subject: [PATCH 18/27] Fix test order bug, make Processors check type --- Wrappers/Python/cil/framework/framework.py | 17 +++-- .../Python/cil/processors/MaskGenerator.py | 7 ++ .../Python/cil/processors/PaganinProcessor.py | 18 +++-- Wrappers/Python/test/test_out_in_place.py | 71 +++++++++++-------- Wrappers/Python/test/testclass.py | 6 ++ 5 files changed, 76 insertions(+), 43 deletions(-) diff --git a/Wrappers/Python/cil/framework/framework.py b/Wrappers/Python/cil/framework/framework.py index 8b7e370375..f2eedf0393 100644 --- a/Wrappers/Python/cil/framework/framework.py +++ b/Wrappers/Python/cil/framework/framework.py @@ -3935,17 +3935,24 @@ def get_output(self, out=None): DataContainer The processed data """ + self.check_output(out) if self.output is None or self.shouldRun: 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): + data = self.get_input() + if out is not None: + if data.array.dtype != out.array.dtype: + raise TypeError("Input type mismatch: got {0} expecting {1}"\ + .format(out.array.dtype, data.array.dtype)) def set_input_processor(self, processor): if issubclass(type(processor), DataProcessor): diff --git a/Wrappers/Python/cil/processors/MaskGenerator.py b/Wrappers/Python/cil/processors/MaskGenerator.py index c81bc0fbac..e0939ee8ee 100644 --- a/Wrappers/Python/cil/processors/MaskGenerator.py +++ b/Wrappers/Python/cil/processors/MaskGenerator.py @@ -197,6 +197,13 @@ 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)) + def process(self, out=None): diff --git a/Wrappers/Python/cil/processors/PaganinProcessor.py b/Wrappers/Python/cil/processors/PaganinProcessor.py index f85c013e6b..c8ac567b89 100644 --- a/Wrappers/Python/cil/processors/PaganinProcessor.py +++ b/Wrappers/Python/cil/processors/PaganinProcessor.py @@ -255,7 +255,7 @@ def process(self, out=None): scaling_factor = -(1/self.mu) # allocate padded buffer - padded_buffer = np.zeros(tuple(x+self.pad*2 for x in data_proj.shape)) + padded_buffer = np.zeros(tuple(x+self.pad*2 for x in data_proj.shape), dtype=data.dtype) # make slice indices to unpad the data if self.pad>0: @@ -264,6 +264,7 @@ def process(self, out=None): else: slice_pad = tuple([slice(None)]*len(padded_buffer.shape)) # loop over the channels + mag2 = self.magnification**2 for j in range(data.geometry.channels): if channel_axis is not None: slice_proj[channel_axis] = j @@ -271,25 +272,28 @@ def process(self, out=None): for i in tqdm(range(len(data.geometry.angles))): slice_proj[angle_axis] = i + padded_buffer.fill(0) padded_buffer[slice_pad] = data.array[(tuple(slice_proj))] if self.full_retrieval==True: # apply the filter in fourier space, apply log and scale # by magnification - fI = fft2(self.magnification**2*padded_buffer) - iffI = ifft2(fI*self.filter) + padded_buffer*=mag2 + fI = fft2(padded_buffer) + iffI = ifft2(fI*self.filter).real + np.log(iffI, out=padded_buffer) # apply scaling factor - padded_buffer = scaling_factor*np.log(iffI) + np.multiply(scaling_factor, padded_buffer, out=padded_buffer) else: # apply the filter in fourier space fI = fft2(padded_buffer) - padded_buffer = ifft2(fI*self.filter) + padded_buffer[:] = ifft2(fI*self.filter).real + if data.geometry.channels>1: out.fill(padded_buffer[slice_pad], angle = i, channel=j) else: - x = padded_buffer[slice_pad] - out.fill(x, angle = i) + out.fill(padded_buffer[slice_pad], angle = i) data.array = np.squeeze(data.array) out.array = np.squeeze(out.array) diff --git a/Wrappers/Python/test/test_out_in_place.py b/Wrappers/Python/test/test_out_in_place.py index 2fac5754df..db913f1046 100644 --- a/Wrappers/Python/test/test_out_in_place.py +++ b/Wrappers/Python/test/test_out_in_place.py @@ -323,7 +323,7 @@ def setUp(self): self.data_arrays=[np.random.normal(0,1, (10,20)).astype(np.float32), np.array(range(0,65500, 328), dtype=np.uint16).reshape((10,20)), - np.random.uniform(0,1,(10,20)).astype(np.float32)] + np.random.uniform(0,1,(10,20)).astype(np.float64)] ag_parallel_2D = AcquisitionGeometry.create_Parallel2D(detector_position=[0,1], units='m') angles = np.linspace(0, 360, 10, dtype=np.float32) @@ -350,55 +350,65 @@ def setUp(self): def fail(self, error_message): raise NotImplementedError(error_message) - - def get_result_check(self, processor, data, *args): - """ - Tests to check the original data is not modified when the processor is called - """ - data_original=data.copy() #To check that data isn't changed after function calls - try: - processor.set_input(data) - out1 = processor.get_output() - self.assertDataArraysInContainerAllClose(data_original, data, rtol=1e-5, msg= "In case processor.set_input(data), processor.get_output() where processor is " + processor.__class__.__name__+ " the input data has been incorrectly affected by the calculation.") - except NotImplementedError: - self.fail("get_result test not implemented for " + processor.__class__.__name__) - - def out_check(self, processor, data, *args): + + def out_check(self, processor, data, data_array_index, *args): """ Tests to check the processor gives the same result using the out argument Also check the out argument doesn't change the orginal data and that the processor still returns correctly with the out argument """ - data_original=data.copy() #To check that data isn't changed after function calls + data_copy=data.copy() #To check that data isn't changed after function calls try: - processor.set_input(data) + processor.set_input(data_copy) out1 = processor.get_output() - out2 = 0.*(out1.copy()) + out2 = out1.copy() + out2.fill(0) out3 = processor.get_output(out=out2) - self.assertDataArraysInContainerAllClose(out1, out2, rtol=1e-5, msg= "Calculation failed using processor.set_input(data), processor.get_output(out=out) where func is " + processor.__class__.__name__+ ".") - self.assertDataArraysInContainerAllClose(data_original, data, rtol=1e-5, msg= "In case processor.set_input(data), processor.get_output(out=out) where processor is " + processor.__class__.__name__+ " the input data has been incorrectly affected by the calculation. ") - self.assertDataArraysInContainerAllClose(out1, out3, rtol=1e-5, msg= "In case processor.set_input(data), output=processor.get_output(out=out) where processor is " + processor.__class__.__name__+ " the processor supresses the output. ") + + self.assertDataContainerAllClose(out1, out2, rtol=1e-10, strict=True) + self.assertDataContainerAllClose(data_copy, data, rtol=1e-10, strict=True) + self.assertEqual(id(out2), id(out3)) + except NotImplementedError: self.fail("out_test test not implemented for " + processor.__class__.__name__) + except Exception as e: + error_message = '\nFor processor: ' + processor.__class__.__name__ + \ + '\nOn data_array index: ' + str(data_array_index) +\ + '\nFor geometry type: \n' + str(data.geometry) + raise type(e)(error_message + '\n\n' + str(e)) - def in_place_check(self, processor, data, *args): + def in_place_check(self, processor, data, data_array_index, *args): """ Check the processor gives the correct result if the calculation is performed in place i.e. data is passed to the out argument if the processor output is a different sized array, the calcualtion cannot be perfomed in place, so we expect this to raise a ValueError """ + data_copy = data.copy() try: - processor.set_input(data) + processor.set_input(data_copy) out1 = processor.get_output() + + # Check an error is raised for Processor's that change the dimensions of the data if out1.shape != data.shape: with self.assertRaises(ValueError): processor.get_output(out=data) + # Check an error is raised if the Processor returns a different data type (only MaskGenerator at the moment) + elif isinstance(processor, MaskGenerator): + with self.assertRaises(TypeError): + processor.get_output(out=data) + # Otherwise check Processor's work correctly in place else: - processor.get_output(out=data) - self.assertDataArraysInContainerAllClose(out1, data, rtol=1e-5, msg= "In place calculation failed for processor.set_input(data), processor.get_output(out=data) where processor is " + processor.__class__.__name__+ "." ) + processor.get_output(out=data_copy) + self.assertDataContainerAllClose(out1, data_copy, rtol=1e-10, strict=True) + except (InPlaceError, NotImplementedError): self.fail("in_place_test test not implemented for " + processor.__class__.__name__) + except Exception as e: + error_message = '\nFor processor: ' + processor.__class__.__name__ + \ + '\nOn data_array index: ' + str(data_array_index) +\ + '\nFor geometry type: \n' + str(data.geometry) + raise type(e)(error_message + '\n\n' + str(e)) def test_out(self): """ @@ -413,11 +423,11 @@ def test_out(self): # perform the tests on different data and geometry types for geom in self.geometry_test_list: - for data_array in self.data_arrays: + for i, data_array in enumerate(self.data_arrays): data=geom.allocate(None) - try: + if geom.dimension == '2D': data.fill(data_array) - except: + else: data.fill(np.repeat(data_array[:,None, :], repeats=2, axis=1)) # Add new Processors here processor_list = [ @@ -437,9 +447,8 @@ def test_out(self): ] for processor in processor_list: - self.get_result_check(processor, data) - self.out_check(processor, data) - self.in_place_check(processor, data) + self.out_check(processor, data, i) + self.in_place_check(processor, data, i) def test_centre_of_rotation_xcorrelation_out(self): """ diff --git a/Wrappers/Python/test/testclass.py b/Wrappers/Python/test/testclass.py index 30164954f4..91211c62a2 100644 --- a/Wrappers/Python/test/testclass.py +++ b/Wrappers/Python/test/testclass.py @@ -76,3 +76,9 @@ def assertDataArraysInContainerAllClose(self, container1, container2, rtol=1e-07 self.assertDataArraysInContainerAllClose(container1.get_item(col),container2.get_item(col), rtol=rtol, msg=msg) else: np.testing.assert_allclose(container1.as_array(), container2.as_array(), rtol=rtol, err_msg=msg) + + def assertDataContainerAllClose(self, container1, container2, rtol=1e-07, msg=None, strict=False): + np.testing.assert_allclose(container1.as_array(), container2.as_array(), rtol=rtol, err_msg=msg) + np.testing.assert_equal(container1.geometry, container2.geometry, err_msg=msg) + if strict: + np.testing.assert_equal(container1.dtype, container2.dtype, err_msg=msg) \ No newline at end of file From 8253d5f1fcb86e77cf25a1b1da75feae1b7d45f2 Mon Sep 17 00:00:00 2001 From: hrobarts Date: Mon, 5 Aug 2024 11:09:33 +0000 Subject: [PATCH 19/27] Fix check_output error --- Wrappers/Python/cil/framework/framework.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Wrappers/Python/cil/framework/framework.py b/Wrappers/Python/cil/framework/framework.py index f2eedf0393..99d44a09de 100644 --- a/Wrappers/Python/cil/framework/framework.py +++ b/Wrappers/Python/cil/framework/framework.py @@ -4028,6 +4028,9 @@ 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() @@ -4065,6 +4068,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): From a8e7abdce72ffa930036c3d29ce01a6f7185486c Mon Sep 17 00:00:00 2001 From: hrobarts Date: Mon, 5 Aug 2024 14:31:36 +0000 Subject: [PATCH 20/27] PaganinProcessor check for dtype=float32 --- Wrappers/Python/cil/processors/PaganinProcessor.py | 3 +++ Wrappers/Python/test/test_DataProcessor.py | 12 ++++++------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/Wrappers/Python/cil/processors/PaganinProcessor.py b/Wrappers/Python/cil/processors/PaganinProcessor.py index c8ac567b89..86d967bb83 100644 --- a/Wrappers/Python/cil/processors/PaganinProcessor.py +++ b/Wrappers/Python/cil/processors/PaganinProcessor.py @@ -200,6 +200,9 @@ def __init__(self, delta=1, beta=1e-2, energy=40000, def check_input(self, data): if not isinstance(data, (AcquisitionData)): raise TypeError('Processor only supports AcquisitionData') + + if data.dtype!=np.float32: + raise TypeError('Processor only support dtype=float32') return True diff --git a/Wrappers/Python/test/test_DataProcessor.py b/Wrappers/Python/test/test_DataProcessor.py index a821bba105..f9dd76142c 100644 --- a/Wrappers/Python/test/test_DataProcessor.py +++ b/Wrappers/Python/test/test_DataProcessor.py @@ -2984,7 +2984,7 @@ def test_PaganinProcessor(self): for data in data_array: data.geometry.config.units = 'm' data_abs = -(1/mu)*numpy.log(data) - processor = PaganinProcessor(full_retrieval=True) + processor = PaganinProcessor(full_retrieval=True, return_units='m') processor.set_input(data) thickness = processor.get_output(override_geometry={'propagation_distance':1}) self.assertLessEqual(quality_measures.mse(thickness, data_abs), 1e-5) @@ -2994,7 +2994,7 @@ def test_PaganinProcessor(self): self.assertLessEqual(quality_measures.mse(filtered_image, data), 1e-5) # test with GPM - processor = PaganinProcessor(full_retrieval=True, filter_type='generalised_paganin_method') + processor = PaganinProcessor(full_retrieval=True, filter_type='generalised_paganin_method', return_units='m') processor.set_input(data) thickness = processor.get_output(override_geometry={'propagation_distance':1}) self.assertLessEqual(quality_measures.mse(thickness, data_abs), 1e-5) @@ -3004,7 +3004,7 @@ def test_PaganinProcessor(self): self.assertLessEqual(quality_measures.mse(filtered_image, data), 1e-5) # test with padding - processor = PaganinProcessor(full_retrieval=True, pad=10) + processor = PaganinProcessor(full_retrieval=True, pad=10, return_units='m') processor.set_input(data) thickness = processor.get_output(override_geometry={'propagation_distance':1}) self.assertLessEqual(quality_measures.mse(thickness, data_abs), 1e-5) @@ -3014,7 +3014,7 @@ def test_PaganinProcessor(self): self.assertLessEqual(quality_measures.mse(filtered_image, data), 1e-5) # test in-line - thickness_inline = PaganinProcessor(full_retrieval=True, pad=10)(data, override_geometry={'propagation_distance':1}) + thickness_inline = PaganinProcessor(full_retrieval=True, pad=10, return_units='m')(data, override_geometry={'propagation_distance':1}) numpy.testing.assert_allclose(thickness.as_array(), thickness_inline.as_array()) filtered_image_inline = PaganinProcessor(full_retrieval=False, pad=10)(data, override_geometry={'propagation_distance':1}) numpy.testing.assert_allclose(filtered_image.as_array(), filtered_image_inline.as_array()) @@ -3022,7 +3022,7 @@ def test_PaganinProcessor(self): # check with different data order data.reorder('astra') data_abs = -(1/mu)*numpy.log(data) - processor = PaganinProcessor(full_retrieval=True, pad=10) + processor = PaganinProcessor(full_retrieval=True, pad=10, return_units='m') processor.set_input(data) with self.assertLogs(level='WARN') as log: thickness = processor.get_output(override_geometry={'propagation_distance':1}) @@ -3037,7 +3037,7 @@ def test_PaganinProcessor(self): if data.geometry.channels>1: data.reorder(('vertical','channel','horizontal','angle')) data_abs = -(1/mu)*numpy.log(data) - processor = PaganinProcessor(full_retrieval=True, pad=10) + processor = PaganinProcessor(full_retrieval=True, pad=10, return_units='m') processor.set_input(data) with self.assertLogs(level='WARN') as log: thickness = processor.get_output(override_geometry={'propagation_distance':1}) From 5b075398d92a79661c992bb49f6883c9268a6f42 Mon Sep 17 00:00:00 2001 From: hrobarts Date: Fri, 9 Aug 2024 10:41:31 +0000 Subject: [PATCH 21/27] Add test for out argument with wrong dtype --- Wrappers/Python/test/test_out_in_place.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/Wrappers/Python/test/test_out_in_place.py b/Wrappers/Python/test/test_out_in_place.py index 0e61b38509..f0d0d1fd2a 100644 --- a/Wrappers/Python/test/test_out_in_place.py +++ b/Wrappers/Python/test/test_out_in_place.py @@ -353,9 +353,10 @@ def fail(self, error_message): def out_check(self, processor, data, data_array_index, *args): """ - Tests to check the processor gives the same result using the out argument - Also check the out argument doesn't change the orginal data - and that the processor still returns correctly with the out argument + - Test to check the processor gives the same result using the out argument + - Check the out argument doesn't change the orginal data + - Check the processor still returns correctly with the out argument + - Check the processor returns an error if the dtype of out is different to expected """ data_copy=data.copy() #To check that data isn't changed after function calls try: @@ -365,10 +366,22 @@ def out_check(self, processor, data, data_array_index, *args): out2.fill(0) out3 = processor.get_output(out=out2) + # check the processor gives the same result using the out argument self.assertDataContainerAllClose(out1, out2, rtol=1e-10, strict=True) + # check the out argument doesn't change the orginal data self.assertDataContainerAllClose(data_copy, data, rtol=1e-10, strict=True) + # check the processor still returns correctly with the out argument self.assertEqual(id(out2), id(out3)) - + # check the processor returns an error if the dtype of out is different to expected + out_wrong_type = out1.copy() + out_wrong_type.array = numpy.array(out_wrong_type.array, dtype=bool) + # if the processor does return a different out type, put an exception here + if isinstance(processor, MaskGenerator): + processor.get_output(out=out_wrong_type) + else: + with self.assertRaises(TypeError): + processor.get_output(out=out_wrong_type) + except NotImplementedError: self.fail("out_test test not implemented for " + processor.__class__.__name__) except Exception as e: From fa08f5dca02abe245bdc852210f29b3e12448fd2 Mon Sep 17 00:00:00 2001 From: hrobarts Date: Tue, 13 Aug 2024 10:05:18 +0000 Subject: [PATCH 22/27] Check geometry and shape are correct in processors check_output --- Wrappers/Python/cil/framework/framework.py | 30 +++++++++++--- Wrappers/Python/cil/processors/Binner.py | 8 ++-- .../Python/cil/processors/MaskGenerator.py | 4 +- Wrappers/Python/cil/processors/Padder.py | 19 +++++---- Wrappers/Python/cil/processors/Slicer.py | 32 +++++++-------- Wrappers/Python/test/test_DataProcessor.py | 39 ++++++++++++------- Wrappers/Python/test/test_out_in_place.py | 8 +--- Wrappers/Python/test/testclass.py | 30 +++++++++++++- 8 files changed, 114 insertions(+), 56 deletions(-) diff --git a/Wrappers/Python/cil/framework/framework.py b/Wrappers/Python/cil/framework/framework.py index 99d44a09de..7b3b3ef5c4 100644 --- a/Wrappers/Python/cil/framework/framework.py +++ b/Wrappers/Python/cil/framework/framework.py @@ -2587,7 +2587,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 @@ -3872,6 +3874,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 @@ -3891,6 +3894,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): """ @@ -3901,7 +3912,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) @@ -3911,7 +3921,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 @@ -3935,7 +3946,9 @@ def get_output(self, out=None): DataContainer The processed data """ - self.check_output(out) + if not self.check_output(out): + raise ValueError('Output data not compatible with processor') + if self.output is None or self.shouldRun: out = self.process(out=out) if self.store_output: @@ -3948,11 +3961,18 @@ def get_output(self, out=None): return self.output.copy() def check_output(self, out): - data = self.get_input() + 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): diff --git a/Wrappers/Python/cil/processors/Binner.py b/Wrappers/Python/cil/processors/Binner.py index 26deb85f8f..4d6bb6f752 100644 --- a/Wrappers/Python/cil/processors/Binner.py +++ b/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: diff --git a/Wrappers/Python/cil/processors/MaskGenerator.py b/Wrappers/Python/cil/processors/MaskGenerator.py index e0939ee8ee..a253ad0556 100644 --- a/Wrappers/Python/cil/processors/MaskGenerator.py +++ b/Wrappers/Python/cil/processors/MaskGenerator.py @@ -203,6 +203,8 @@ def check_output(self, out): 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): @@ -372,7 +374,7 @@ 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) + 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) diff --git a/Wrappers/Python/cil/processors/Padder.py b/Wrappers/Python/cil/processors/Padder.py index a9e18997a8..bb6ea48d31 100644 --- a/Wrappers/Python/cil/processors/Padder.py +++ b/Wrappers/Python/cil/processors/Padder.py @@ -379,6 +379,7 @@ def __init__(self, '_geometry': None, '_shape_in':None, '_shape_out':None, + '_shape_out_full':None, '_labels_in':None, '_processed_dims':None, '_pad_width_param':None, @@ -408,6 +409,7 @@ def set_input(self, dataset): raise TypeError("Input type mismatch: got {0} expecting {1}"\ .format(type(dataset), DataContainer)) + self._set_up() def check_input(self, data): @@ -439,8 +441,6 @@ def check_input(self, data): if self.pad_width is None: raise ValueError('Please, specify pad_width') - self._parse_input(data) - return True def _create_tuple(self, value, dtype): @@ -465,8 +465,9 @@ def _get_dimensions_from_dict(self, dict): return dimensions - def _parse_input(self, data): - + def _set_up(self): + + data = self.get_input() offset = 4-data.ndim #set defaults @@ -476,7 +477,7 @@ def _parse_input(self, data): self._shape_in = [1]*4 self._shape_in[offset::] = data.shape - self._shape_out = self._shape_in.copy() + self._shape_out_full = self._shape_in.copy() self._processed_dims = [0,0,0,0] @@ -539,7 +540,9 @@ def _parse_input(self, data): for i in range(4): if self._pad_width_param[i] != (0,0): self._processed_dims[i] = 1 - self._shape_out[i] += self._pad_width_param[i][0] + self._pad_width_param[i][1] + self._shape_out_full[i] += self._pad_width_param[i][0] + self._pad_width_param[i][1] + + self._shape_out = tuple([i for i in self._shape_out_full if i > 1]) def _process_acquisition_geometry(self): @@ -657,9 +660,9 @@ def process(self, out=None): else: # check size and shape if passed out try: - out.array = out.array.reshape(self._shape_out) + out.array = out.array.reshape(self._shape_out_full) except: - raise ValueError("Array of `out` not compatible. Expected shape: {0}, data type: {1} Got shape: {2}, data type: {3}".format(self._shape_out, np.float32, out.array.shape, out.array.dtype)) + raise ValueError("Array of `out` not compatible. Expected shape: {0}, data type: {1} Got shape: {2}, data type: {3}".format(self._shape_out_full, np.float32, out.array.shape, out.array.dtype)) if new_geometry is not None: if out.geometry != new_geometry: diff --git a/Wrappers/Python/cil/processors/Slicer.py b/Wrappers/Python/cil/processors/Slicer.py index b3ab836823..3d37bfe0ef 100644 --- a/Wrappers/Python/cil/processors/Slicer.py +++ b/Wrappers/Python/cil/processors/Slicer.py @@ -95,9 +95,9 @@ def __init__(self, '_geometry': None, '_processed_dims':None, '_shape_in':None, + '_shape_out_full':None, '_shape_out':None, - 'shape_out':None, - 'labels_out':None, + '_labels_out':None, '_labels_in':None, '_pixel_indices':None, '_accelerated': True @@ -126,6 +126,7 @@ def set_input(self, dataset): raise TypeError("Input type mismatch: got {0} expecting {1}"\ .format(type(dataset), DataContainer)) + self._set_up() def check_input(self, data): @@ -156,23 +157,22 @@ def check_input(self, data): if key not in data.dimension_labels: raise ValueError('Wrong label is specified for roi, expected one of {}.'.format(data.dimension_labels)) - self._set_up_processor(data) - return True - def _set_up_processor(self, data): + def _set_up(self): """ This parses the input roi generically and then configures the processor according to its class. """ #read input + data = self.get_input() self._parse_roi(data.ndim, data.shape, data.dimension_labels) #processor specific configurations self._configure() # set boolean of dimensions to process - self._processed_dims = [0 if self._shape_out[i] == self._shape_in[i] else 1 for i in range(4)] - self.shape_out = tuple([i for i in self._shape_out if i > 1]) - self.labels_out = [self._labels_in[i] for i,x in enumerate(self._shape_out) if x > 1] + self._processed_dims = [0 if self._shape_out_full[i] == self._shape_in[i] else 1 for i in range(4)] + self._shape_out = tuple([i for i in self._shape_out_full if i > 1]) + self._labels_out = [self._labels_in[i] for i,x in enumerate(self._shape_out_full) if x > 1] def _parse_roi(self, ndim, shape, dimension_labels): ''' @@ -252,7 +252,7 @@ def _configure(self): """ Once the ROI has been parsed this configure the input specifically for use with Slicer """ - self._shape_out = [len(x) for x in self._roi_ordered] + self._shape_out_full = [len(x) for x in self._roi_ordered] self._pixel_indices = [(x[0],x[-1]) for x in self._roi_ordered] @@ -331,10 +331,10 @@ def _process_image_geometry(self): Creates the new image geometry """ - if len(self.shape_out) == 0: + if len(self._shape_out) == 0: return None - elif len(self.shape_out) ==1: - return VectorGeometry(self.shape_out[0], dimension_labels=self.labels_out[0]) + elif len(self._shape_out) ==1: + return VectorGeometry(self._shape_out[0], dimension_labels=self._labels_out[0]) geometry_new = self._geometry.copy() for i, axis in enumerate(self._labels_in): @@ -413,13 +413,13 @@ def process(self, out=None): if new_geometry is not None: data_out = new_geometry.allocate(None) else: - processed_array = np.empty(self.shape_out,dtype=np.float32) - data_out = DataContainer(processed_array,False, self.labels_out) + processed_array = np.empty(self._shape_out,dtype=np.float32) + data_out = DataContainer(processed_array,False, self._labels_out) else: try: - out.array = np.asarray(out.array, dtype=np.float32, order='C').reshape(self.shape_out) + out.array = np.asarray(out.array, dtype=np.float32, order='C').reshape(self._shape_out) except: - raise ValueError("Array of `out` not compatible. Expected shape: {0}, data type: {1} Got shape: {2}, data type: {3}".format(self.shape_out, np.float32, out.array.shape, out.array.dtype)) + raise ValueError("Array of `out` not compatible. Expected shape: {0}, data type: {1} Got shape: {2}, data type: {3}".format(self._shape_out, np.float32, out.array.shape, out.array.dtype)) if new_geometry is not None: if out.geometry != new_geometry: diff --git a/Wrappers/Python/test/test_DataProcessor.py b/Wrappers/Python/test/test_DataProcessor.py index f9dd76142c..5a695626f7 100644 --- a/Wrappers/Python/test/test_DataProcessor.py +++ b/Wrappers/Python/test/test_DataProcessor.py @@ -180,7 +180,8 @@ def test_set_up_processor(self): roi = {'horizontal_y':horizontal_y,'horizontal_x':horizontal_x,'vertical':vertical,'channel':channel} proc = Binner(roi,accelerated=True) - proc._set_up_processor(data) + proc.set_input(data) + proc._set_up() # check set values self.assertTrue(proc._shape_in == list(data.shape)) @@ -191,7 +192,7 @@ def test_set_up_processor(self): (horizontal_x.stop - horizontal_x.start)//horizontal_x.step ] - self.assertTrue(proc._shape_out == shape_out) + self.assertTrue(proc._shape_out_full == shape_out) self.assertTrue(proc._labels_in == ['channel','vertical','horizontal_y','horizontal_x']) numpy.testing.assert_array_equal(proc._processed_dims,[True,True,False,True]) @@ -807,7 +808,8 @@ def test_set_up_processor(self): roi = {'horizontal_y':horizontal_y,'horizontal_x':horizontal_x,'vertical':vertical,'channel':channel} proc = Slicer(roi) - proc._set_up_processor(data) + proc.set_input(data) + proc._set_up() # check set values self.assertTrue(proc._shape_in == list(data.shape)) @@ -819,7 +821,7 @@ def test_set_up_processor(self): len(horizontal_x), ] - self.assertTrue(proc._shape_out == shape_out) + self.assertTrue(proc._shape_out_full == shape_out) self.assertTrue(proc._labels_in == ['channel','vertical','horizontal_y','horizontal_x']) numpy.testing.assert_array_equal(proc._processed_dims,[True,True,False,True]) @@ -1672,7 +1674,7 @@ def setUp(self): self.data_test = ImageData(arr_in, True, ig) - def test_parse_input(self): + def test_set_up(self): ig = ImageGeometry(20,22,23,0.1,0.2,0.3,0.4,0.5,0.6,channels=24) data = ig.allocate('random') @@ -1684,7 +1686,8 @@ def test_parse_input(self): # check inputs proc = Padder('constant', pad_width=2, pad_values=0.1) - proc._parse_input(data) + proc.set_input(data) + proc._set_up() self.assertListEqual(list(data.dimension_labels), proc._labels_in) self.assertListEqual(list(data.shape), proc._shape_in) @@ -1705,21 +1708,24 @@ def test_parse_input(self): # check pad_width set-up proc = Padder('constant', pad_width=2, pad_values=0.1) - proc._parse_input(data) + proc.set_input(data) + proc._set_up() self.assertListEqual(gold_width_default, proc._pad_width_param) self.assertListEqual(gold_value_default, proc._pad_values_param) self.assertListEqual(gold_processed_dims_default, proc._processed_dims) numpy.testing.assert_array_equal(gold_shape_out_default, proc._shape_out) proc = Padder('constant', pad_width=(1,2), pad_values=0.1) - proc._parse_input(data) + proc.set_input(data) + proc._set_up() self.assertListEqual(gold_width_tuple, proc._pad_width_param) self.assertListEqual(gold_value_default, proc._pad_values_param) self.assertListEqual(gold_processed_dims_default, proc._processed_dims) numpy.testing.assert_array_equal(gold_shape_out_tuple, proc._shape_out) proc = Padder('constant', pad_width=pad_width, pad_values=0.1) - proc._parse_input(data) + proc.set_input(data) + proc._set_up() gold_value_dict_custom = [(0,0) if x == (0,0) else (0.1,0.1) for x in gold_width_dict] self.assertListEqual(gold_width_dict, proc._pad_width_param) self.assertListEqual(gold_value_dict_custom, proc._pad_values_param) @@ -1728,14 +1734,16 @@ def test_parse_input(self): # check pad_value set-up proc = Padder('constant', pad_width=2, pad_values=(0.1,0.2)) - proc._parse_input(data) + proc.set_input(data) + proc._set_up() self.assertListEqual(gold_width_default, proc._pad_width_param) self.assertListEqual(gold_value_tuple, proc._pad_values_param) self.assertListEqual(gold_processed_dims_default, proc._processed_dims) numpy.testing.assert_array_equal(gold_shape_out_default, proc._shape_out) proc = Padder('constant', pad_width=2, pad_values=pad_values) - proc._parse_input(data) + proc.set_input(data) + proc._set_up() gold_width_dict_custom = [(0,0) if x == (0,0) else (2,2) for x in gold_value_dict] gold_shape_out_dict_custom = numpy.array(data.shape) + [4,4,0,4] self.assertListEqual(gold_width_dict_custom, proc._pad_width_param) @@ -1744,7 +1752,8 @@ def test_parse_input(self): numpy.testing.assert_array_equal(gold_shape_out_dict_custom, proc._shape_out) proc = Padder('constant', pad_width=pad_width, pad_values=(0.1,0.2)) - proc._parse_input(data) + proc.set_input(data) + proc._set_up() gold_value_dictionary_custom = [(0,0) if x == (0,0) else (0.1,0.2) for x in gold_width_dict] self.assertListEqual(gold_width_dict, proc._pad_width_param) self.assertListEqual(gold_value_dictionary_custom, proc._pad_values_param) @@ -1752,7 +1761,8 @@ def test_parse_input(self): numpy.testing.assert_array_equal(gold_shape_out_dict, proc._shape_out) proc = Padder('constant', pad_width=pad_width, pad_values=pad_values) - proc._parse_input(data) + proc.set_input(data) + proc._set_up() self.assertListEqual(gold_width_dict, proc._pad_width_param) self.assertListEqual(gold_value_dict, proc._pad_values_param) self.assertListEqual(gold_processed_dims_dict, proc._processed_dims) @@ -1761,7 +1771,8 @@ def test_parse_input(self): proc = Padder('constant', pad_width=pad_width, pad_values={'horizontal_x':(0.5,0.6)}) # raise an error as not all axes values defined with self.assertRaises(ValueError): - proc._parse_input(data) + proc.set_input(data) + proc._set_up() def test_process_acquisition_geometry(self): diff --git a/Wrappers/Python/test/test_out_in_place.py b/Wrappers/Python/test/test_out_in_place.py index f0d0d1fd2a..6fd1d12695 100644 --- a/Wrappers/Python/test/test_out_in_place.py +++ b/Wrappers/Python/test/test_out_in_place.py @@ -374,13 +374,9 @@ def out_check(self, processor, data, data_array_index, *args): self.assertEqual(id(out2), id(out3)) # check the processor returns an error if the dtype of out is different to expected out_wrong_type = out1.copy() - out_wrong_type.array = numpy.array(out_wrong_type.array, dtype=bool) - # if the processor does return a different out type, put an exception here - if isinstance(processor, MaskGenerator): + out_wrong_type.array = numpy.array(out_wrong_type.array, dtype=np.int64) + with self.assertRaises(TypeError): processor.get_output(out=out_wrong_type) - else: - with self.assertRaises(TypeError): - processor.get_output(out=out_wrong_type) except NotImplementedError: self.fail("out_test test not implemented for " + processor.__class__.__name__) diff --git a/Wrappers/Python/test/testclass.py b/Wrappers/Python/test/testclass.py index 91211c62a2..2239c38b23 100644 --- a/Wrappers/Python/test/testclass.py +++ b/Wrappers/Python/test/testclass.py @@ -78,7 +78,33 @@ def assertDataArraysInContainerAllClose(self, container1, container2, rtol=1e-07 np.testing.assert_allclose(container1.as_array(), container2.as_array(), rtol=rtol, err_msg=msg) def assertDataContainerAllClose(self, container1, container2, rtol=1e-07, msg=None, strict=False): - np.testing.assert_allclose(container1.as_array(), container2.as_array(), rtol=rtol, err_msg=msg) - np.testing.assert_equal(container1.geometry, container2.geometry, err_msg=msg) + # Test to check if two DataContainers are close, by checking + # - they are the same class + # - they have arrays that are all close + # - if they have geometry, the geometries are equal + # - and if strict=True, their data type is the same + # + # Parameters + # ---------- + # container1 : DataContainer + # The first DataContainer to compare. + # container2 : DataContainer + # The second DataContainer to compare. + # rtol : float, optional + # The relative tolerance for the array comparison + # msg : string, optional + # The error message to be printed in case of failure + # strict : bool, optional + # If True, raises an error if the data type in the DataContainers + # is not equal + + if not isinstance(container1, container2.__class__): + raise TypeError("container2 is not the same class as container1") + + np.testing.assert_allclose(container1.array, container2.array, rtol=rtol, err_msg=msg) + + if hasattr(container1, "geometry"): + np.testing.assert_equal(container1.geometry, container2.geometry, err_msg=msg) + if strict: np.testing.assert_equal(container1.dtype, container2.dtype, err_msg=msg) \ No newline at end of file From ea3f3f9f516558588c70d1936e8854941a772e0b Mon Sep 17 00:00:00 2001 From: hrobarts Date: Tue, 13 Aug 2024 10:39:31 +0000 Subject: [PATCH 23/27] Make check_output return False for projection operators --- .../cil/plugins/astra/processors/AstraBackProjector2D.py | 3 +++ .../cil/plugins/astra/processors/AstraBackProjector3D.py | 3 +++ .../cil/plugins/astra/processors/AstraForwardProjector2D.py | 3 +++ .../cil/plugins/astra/processors/AstraForwardProjector3D.py | 3 +++ Wrappers/Python/cil/plugins/astra/processors/FBP.py | 3 +++ Wrappers/Python/cil/plugins/astra/processors/FBP_Flexible.py | 3 +++ Wrappers/Python/cil/plugins/astra/processors/FDK_Flexible.py | 3 +++ Wrappers/Python/cil/plugins/tigre/FBP.py | 3 +++ 8 files changed, 24 insertions(+) diff --git a/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector2D.py b/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector2D.py index 1dabeb1925..6a8e4d3f82 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector2D.py +++ b/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector2D.py @@ -85,6 +85,9 @@ def check_input(self, dataset): raise ValueError("Expected input dimensions is 1 or 2, got {0}"\ .format(dataset.number_of_dimensions)) + def check_output(self, out): + return True + def set_projector(self, proj_id): self.proj_id = proj_id diff --git a/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector3D.py b/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector3D.py index b9f28846af..9bba37749d 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector3D.py +++ b/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector3D.py @@ -63,6 +63,9 @@ def check_input(self, dataset): raise ValueError("Dataset not compatible with geometry used to create the projector") return True + + def check_output(self, out): + return True def set_ImageGeometry(self, volume_geometry): diff --git a/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector2D.py b/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector2D.py index 3738bedf6f..8646c6d25c 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector2D.py +++ b/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector2D.py @@ -86,6 +86,9 @@ def check_input(self, dataset): raise ValueError("Expected input dimensions is 1 or 2, got {0}"\ .format(dataset.number_of_dimensions)) + def check_output(self, out): + return True + def set_projector(self, proj_id): self.proj_id = proj_id diff --git a/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector3D.py b/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector3D.py index 252afdf5d6..1dca6af902 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector3D.py +++ b/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector3D.py @@ -61,6 +61,9 @@ def check_input(self, dataset): raise ValueError("Dataset not compatible with geometry used to create the projector") return True + + def check_output(self, out): + return True def set_ImageGeometry(self, volume_geometry): diff --git a/Wrappers/Python/cil/plugins/astra/processors/FBP.py b/Wrappers/Python/cil/plugins/astra/processors/FBP.py index 5d46a2b456..dd10a721ce 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/FBP.py +++ b/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) diff --git a/Wrappers/Python/cil/plugins/astra/processors/FBP_Flexible.py b/Wrappers/Python/cil/plugins/astra/processors/FBP_Flexible.py index 138caa1d05..fe117f840e 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/FBP_Flexible.py +++ b/Wrappers/Python/cil/plugins/astra/processors/FBP_Flexible.py @@ -149,6 +149,9 @@ 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 check_output(self, out): + return True def process(self, out=None): diff --git a/Wrappers/Python/cil/plugins/astra/processors/FDK_Flexible.py b/Wrappers/Python/cil/plugins/astra/processors/FDK_Flexible.py index b4d902fd1e..99ed9a99cf 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/FDK_Flexible.py +++ b/Wrappers/Python/cil/plugins/astra/processors/FDK_Flexible.py @@ -73,6 +73,9 @@ def check_input(self, dataset): .format(self.sinogram_geometry.geom_type)) return True + + def check_output(self, out): + return True def process(self, out=None): diff --git a/Wrappers/Python/cil/plugins/tigre/FBP.py b/Wrappers/Python/cil/plugins/tigre/FBP.py index 07aa33158a..5d693ee4fc 100644 --- a/Wrappers/Python/cil/plugins/tigre/FBP.py +++ b/Wrappers/Python/cil/plugins/tigre/FBP.py @@ -82,6 +82,9 @@ def check_input(self, dataset): DataOrder.check_order_for_engine('tigre', dataset.geometry) return True + + def check_output(self, out): + return True def process(self, out=None): From 54cd8c113235966d131441687814cf8e57383ffe Mon Sep 17 00:00:00 2001 From: hrobarts Date: Tue, 13 Aug 2024 15:10:32 +0000 Subject: [PATCH 24/27] Add type and shape checks for plugin projectors --- .../plugins/astra/processors/AstraBackProjector2D.py | 10 ++++++++++ .../plugins/astra/processors/AstraBackProjector3D.py | 10 ++++++++++ .../astra/processors/AstraForwardProjector2D.py | 10 ++++++++++ .../astra/processors/AstraForwardProjector3D.py | 10 ++++++++++ .../cil/plugins/astra/processors/FBP_Flexible.py | 10 ++++++++++ .../cil/plugins/astra/processors/FDK_Flexible.py | 9 +++++++++ 6 files changed, 59 insertions(+) diff --git a/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector2D.py b/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector2D.py index 6a8e4d3f82..cdae6ae707 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector2D.py +++ b/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector2D.py @@ -86,6 +86,16 @@ def check_input(self, dataset): .format(dataset.number_of_dimensions)) def check_output(self, out): + if out is not None: + data = self.get_input() + if data.array.dtype != out.array.dtype: + raise TypeError("out type mismatch: got {0} expecting {1}"\ + .format(out.array.dtype, data.array.dtype)) + + if self.volume_geometry.shape != out.shape: + raise ValueError("out size mismatch: got {0} expecting {1}"\ + .format(out.shape, self._shape_out)) + return True def set_projector(self, proj_id): diff --git a/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector3D.py b/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector3D.py index 9bba37749d..e33af5f1a9 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector3D.py +++ b/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector3D.py @@ -65,6 +65,16 @@ def check_input(self, dataset): return True def check_output(self, out): + if out is not None: + data = self.get_input() + if data.array.dtype != out.array.dtype: + raise TypeError("out type mismatch: got {0} expecting {1}"\ + .format(out.array.dtype, data.array.dtype)) + + if self.volume_geometry.shape != out.shape: + raise ValueError("out size mismatch: got {0} expecting {1}"\ + .format(out.shape, self._shape_out)) + return True def set_ImageGeometry(self, volume_geometry): diff --git a/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector2D.py b/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector2D.py index 8646c6d25c..9c486f1a63 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector2D.py +++ b/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector2D.py @@ -87,6 +87,16 @@ def check_input(self, dataset): .format(dataset.number_of_dimensions)) def check_output(self, out): + if out is not None: + data = self.get_input() + if data.array.dtype != out.array.dtype: + raise TypeError("out type mismatch: got {0} expecting {1}"\ + .format(out.array.dtype, data.array.dtype)) + + if self.volume_geometry.shape != out.shape: + raise ValueError("out size mismatch: got {0} expecting {1}"\ + .format(out.shape, self._shape_out)) + return True def set_projector(self, proj_id): diff --git a/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector3D.py b/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector3D.py index 1dca6af902..a086006a3b 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector3D.py +++ b/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector3D.py @@ -63,6 +63,16 @@ def check_input(self, dataset): return True def check_output(self, out): + if out is not None: + data = self.get_input() + if data.array.dtype != out.array.dtype: + raise TypeError("out type mismatch: got {0} expecting {1}"\ + .format(out.array.dtype, data.array.dtype)) + + if self.volume_geometry.shape != out.shape: + raise ValueError("out size mismatch: got {0} expecting {1}"\ + .format(out.shape, self._shape_out)) + return True def set_ImageGeometry(self, volume_geometry): diff --git a/Wrappers/Python/cil/plugins/astra/processors/FBP_Flexible.py b/Wrappers/Python/cil/plugins/astra/processors/FBP_Flexible.py index fe117f840e..05ed54ac0b 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/FBP_Flexible.py +++ b/Wrappers/Python/cil/plugins/astra/processors/FBP_Flexible.py @@ -151,6 +151,16 @@ def check_input(self, dataset): return True def check_output(self, out): + if out is not None: + data = self.get_input() + if data.array.dtype != out.array.dtype: + raise TypeError("out type mismatch: got {0} expecting {1}"\ + .format(out.array.dtype, data.array.dtype)) + + if self.volume_geometry.shape != out.shape: + raise ValueError("out size mismatch: got {0} expecting {1}"\ + .format(out.shape, self._shape_out)) + return True diff --git a/Wrappers/Python/cil/plugins/astra/processors/FDK_Flexible.py b/Wrappers/Python/cil/plugins/astra/processors/FDK_Flexible.py index 99ed9a99cf..affa4a92e1 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/FDK_Flexible.py +++ b/Wrappers/Python/cil/plugins/astra/processors/FDK_Flexible.py @@ -75,6 +75,15 @@ def check_input(self, dataset): return True def check_output(self, out): + if out is not None: + data = self.get_input() + if data.array.dtype != out.array.dtype: + raise TypeError("out type mismatch: got {0} expecting {1}"\ + .format(out.array.dtype, data.array.dtype)) + + if self.volume_geometry.shape != out.shape: + raise ValueError("out size mismatch: got {0} expecting {1}"\ + .format(out.shape, self._shape_out)) return True From 22ebef6c3611a9367ff8c8a6dd9a049810a821a0 Mon Sep 17 00:00:00 2001 From: hrobarts Date: Tue, 13 Aug 2024 15:28:34 +0000 Subject: [PATCH 25/27] Check forward projector size matches sinogram size --- .../cil/plugins/astra/processors/AstraForwardProjector2D.py | 2 +- .../cil/plugins/astra/processors/AstraForwardProjector3D.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector2D.py b/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector2D.py index 9c486f1a63..de38fb3446 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector2D.py +++ b/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector2D.py @@ -93,7 +93,7 @@ def check_output(self, out): raise TypeError("out type mismatch: got {0} expecting {1}"\ .format(out.array.dtype, data.array.dtype)) - if self.volume_geometry.shape != out.shape: + if self.sinogram_geometry.shape != out.shape: raise ValueError("out size mismatch: got {0} expecting {1}"\ .format(out.shape, self._shape_out)) diff --git a/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector3D.py b/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector3D.py index a086006a3b..a105503bf2 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector3D.py +++ b/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector3D.py @@ -69,7 +69,7 @@ def check_output(self, out): raise TypeError("out type mismatch: got {0} expecting {1}"\ .format(out.array.dtype, data.array.dtype)) - if self.volume_geometry.shape != out.shape: + if self.sinogram_geometry.shape != out.shape: raise ValueError("out size mismatch: got {0} expecting {1}"\ .format(out.shape, self._shape_out)) From 15a970285a2d9d3a51542dec9e688fac7e7c9aeb Mon Sep 17 00:00:00 2001 From: hrobarts Date: Tue, 20 Aug 2024 17:10:12 +0000 Subject: [PATCH 26/27] Review changes --- CHANGELOG.md | 2 + .../astra/processors/AstraBackProjector2D.py | 20 ++++----- .../astra/processors/AstraBackProjector3D.py | 19 +++------ .../processors/AstraForwardProjector2D.py | 18 +++----- .../processors/AstraForwardProjector3D.py | 18 +++----- .../plugins/astra/processors/FBP_Flexible.py | 19 +++------ .../plugins/astra/processors/FDK_Flexible.py | 20 ++++----- Wrappers/Python/cil/plugins/tigre/FBP.py | 8 +++- Wrappers/Python/test/testclass.py | 42 ++++++++++--------- .../create_local_env_for_cil_development.sh | 6 +-- 10 files changed, 71 insertions(+), 101 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 485ea685ac..d6ebe982eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ - 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) * 24.1.0 diff --git a/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector2D.py b/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector2D.py index cdae6ae707..6e187e6691 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector2D.py +++ b/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector2D.py @@ -84,20 +84,14 @@ def check_input(self, dataset): else: raise ValueError("Expected input dimensions is 1 or 2, got {0}"\ .format(dataset.number_of_dimensions)) - - def check_output(self, out): - if out is not None: - data = self.get_input() - if data.array.dtype != out.array.dtype: - raise TypeError("out type mismatch: got {0} expecting {1}"\ - .format(out.array.dtype, data.array.dtype)) - - if self.volume_geometry.shape != out.shape: - raise ValueError("out size mismatch: got {0} expecting {1}"\ - .format(out.shape, self._shape_out)) - 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_projector(self, proj_id): self.proj_id = proj_id diff --git a/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector3D.py b/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector3D.py index e33af5f1a9..a43d226124 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector3D.py +++ b/Wrappers/Python/cil/plugins/astra/processors/AstraBackProjector3D.py @@ -56,7 +56,6 @@ 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: @@ -64,18 +63,12 @@ def check_input(self, dataset): return True - def check_output(self, out): - if out is not None: - data = self.get_input() - if data.array.dtype != out.array.dtype: - raise TypeError("out type mismatch: got {0} expecting {1}"\ - .format(out.array.dtype, data.array.dtype)) - - if self.volume_geometry.shape != out.shape: - raise ValueError("out size mismatch: got {0} expecting {1}"\ - .format(out.shape, self._shape_out)) - - 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): diff --git a/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector2D.py b/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector2D.py index de38fb3446..9543d97b32 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector2D.py +++ b/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector2D.py @@ -86,18 +86,12 @@ def check_input(self, dataset): raise ValueError("Expected input dimensions is 1 or 2, got {0}"\ .format(dataset.number_of_dimensions)) - def check_output(self, out): - if out is not None: - data = self.get_input() - if data.array.dtype != out.array.dtype: - raise TypeError("out type mismatch: got {0} expecting {1}"\ - .format(out.array.dtype, data.array.dtype)) - - if self.sinogram_geometry.shape != out.shape: - raise ValueError("out size mismatch: got {0} expecting {1}"\ - .format(out.shape, self._shape_out)) - - 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_projector(self, proj_id): self.proj_id = proj_id diff --git a/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector3D.py b/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector3D.py index a105503bf2..a9de858110 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector3D.py +++ b/Wrappers/Python/cil/plugins/astra/processors/AstraForwardProjector3D.py @@ -62,18 +62,12 @@ def check_input(self, dataset): return True - def check_output(self, out): - if out is not None: - data = self.get_input() - if data.array.dtype != out.array.dtype: - raise TypeError("out type mismatch: got {0} expecting {1}"\ - .format(out.array.dtype, data.array.dtype)) - - if self.sinogram_geometry.shape != out.shape: - raise ValueError("out size mismatch: got {0} expecting {1}"\ - .format(out.shape, self._shape_out)) - - 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): diff --git a/Wrappers/Python/cil/plugins/astra/processors/FBP_Flexible.py b/Wrappers/Python/cil/plugins/astra/processors/FBP_Flexible.py index 05ed54ac0b..4c5c3d604b 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/FBP_Flexible.py +++ b/Wrappers/Python/cil/plugins/astra/processors/FBP_Flexible.py @@ -150,19 +150,12 @@ def check_input(self, dataset): return True - def check_output(self, out): - if out is not None: - data = self.get_input() - if data.array.dtype != out.array.dtype: - raise TypeError("out type mismatch: got {0} expecting {1}"\ - .format(out.array.dtype, data.array.dtype)) - - if self.volume_geometry.shape != out.shape: - raise ValueError("out size mismatch: got {0} expecting {1}"\ - .format(out.shape, self._shape_out)) - - 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): diff --git a/Wrappers/Python/cil/plugins/astra/processors/FDK_Flexible.py b/Wrappers/Python/cil/plugins/astra/processors/FDK_Flexible.py index affa4a92e1..f0446b4df3 100644 --- a/Wrappers/Python/cil/plugins/astra/processors/FDK_Flexible.py +++ b/Wrappers/Python/cil/plugins/astra/processors/FDK_Flexible.py @@ -74,19 +74,13 @@ def check_input(self, dataset): return True - def check_output(self, out): - if out is not None: - data = self.get_input() - if data.array.dtype != out.array.dtype: - raise TypeError("out type mismatch: got {0} expecting {1}"\ - .format(out.array.dtype, data.array.dtype)) - - if self.volume_geometry.shape != out.shape: - raise ValueError("out size mismatch: got {0} expecting {1}"\ - .format(out.shape, self._shape_out)) - 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 diff --git a/Wrappers/Python/cil/plugins/tigre/FBP.py b/Wrappers/Python/cil/plugins/tigre/FBP.py index 5d693ee4fc..e00591446c 100644 --- a/Wrappers/Python/cil/plugins/tigre/FBP.py +++ b/Wrappers/Python/cil/plugins/tigre/FBP.py @@ -83,8 +83,12 @@ def check_input(self, dataset): DataOrder.check_order_for_engine('tigre', dataset.geometry) return True - def check_output(self, out): - 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): diff --git a/Wrappers/Python/test/testclass.py b/Wrappers/Python/test/testclass.py index 2239c38b23..f550b125ea 100644 --- a/Wrappers/Python/test/testclass.py +++ b/Wrappers/Python/test/testclass.py @@ -78,26 +78,28 @@ def assertDataArraysInContainerAllClose(self, container1, container2, rtol=1e-07 np.testing.assert_allclose(container1.as_array(), container2.as_array(), rtol=rtol, err_msg=msg) def assertDataContainerAllClose(self, container1, container2, rtol=1e-07, msg=None, strict=False): - # Test to check if two DataContainers are close, by checking - # - they are the same class - # - they have arrays that are all close - # - if they have geometry, the geometries are equal - # - and if strict=True, their data type is the same - # - # Parameters - # ---------- - # container1 : DataContainer - # The first DataContainer to compare. - # container2 : DataContainer - # The second DataContainer to compare. - # rtol : float, optional - # The relative tolerance for the array comparison - # msg : string, optional - # The error message to be printed in case of failure - # strict : bool, optional - # If True, raises an error if the data type in the DataContainers - # is not equal - + ''' + Test to check if two DataContainers are close, by checking + - they are the same class + - they have arrays that are all close + - if they have geometry, the geometries are equal + - and if strict=True, their data type is the same + + Parameters + ---------- + container1 : DataContainer + The first DataContainer to compare. + container2 : DataContainer + The second DataContainer to compare. + rtol : float, optional + The relative tolerance for the array comparison + msg : string, optional + The error message to be printed in case of failure + strict : bool, optional + If True, raises an error if the data type in the DataContainers + is not equal + ''' + if not isinstance(container1, container2.__class__): raise TypeError("container2 is not the same class as container1") diff --git a/scripts/create_local_env_for_cil_development.sh b/scripts/create_local_env_for_cil_development.sh index 8e65ca665f..b4625e4282 100755 --- a/scripts/create_local_env_for_cil_development.sh +++ b/scripts/create_local_env_for_cil_development.sh @@ -15,9 +15,9 @@ # CIL Developers, listed at: https://github.com/TomographicImaging/CIL/blob/master/NOTICE.txt set -euxo pipefail -numpy='1.24' -python='3.10' -name=cil +numpy='1.25' +python='3.11' +name=cil_tests_311_125 test_deps=0 cil_ver='' while getopts hn:p:e:tv: option ; do From 3ef1cc6649e360291ba3c13fa20f6c54ef845372 Mon Sep 17 00:00:00 2001 From: hrobarts Date: Wed, 21 Aug 2024 08:29:30 +0000 Subject: [PATCH 27/27] Undo change to scripts --- scripts/create_local_env_for_cil_development.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/create_local_env_for_cil_development.sh b/scripts/create_local_env_for_cil_development.sh index b4625e4282..8e65ca665f 100755 --- a/scripts/create_local_env_for_cil_development.sh +++ b/scripts/create_local_env_for_cil_development.sh @@ -15,9 +15,9 @@ # CIL Developers, listed at: https://github.com/TomographicImaging/CIL/blob/master/NOTICE.txt set -euxo pipefail -numpy='1.25' -python='3.11' -name=cil_tests_311_125 +numpy='1.24' +python='3.10' +name=cil test_deps=0 cil_ver='' while getopts hn:p:e:tv: option ; do