ProjectionOperator Bug Fix (Issue #1990)

NOTICE.txt

@@ -71,6 +71,7 @@ Joshua Hellier (2024) - 3
 Nicholas Whyatt (2024) - 1
 Rasmia Kulan (2024) - 1
 Emmanuel Ferdman (2025) - 14
+Mariam Demir (2025) - 1
 
 CIL Advisory Board:
 Llion Evans - 9

Wrappers/Python/cil/plugins/astra/operators/ProjectionOperator.py

@@ -124,15 +124,19 @@ def __init__(self,
         sinogram_geometry_sc = acquisition_geometry.get_slice(channel=0)
         volume_geometry_sc = image_geometry.get_slice(channel=0)
 
+        device = device.lower()
         if device == 'gpu':
             operator = AstraProjector3D(volume_geometry_sc,
                                         sinogram_geometry_sc)
-        elif AcquisitionType.DIM2 & self.sinogram_geometry.dimension:
-            operator = AstraProjector2D(volume_geometry_sc,
-                                        sinogram_geometry_sc,
-                                        device=device)
+        elif device == 'cpu':
+            if AcquisitionType.DIM2 & self.sinogram_geometry.dimension:
+                operator = AstraProjector2D(volume_geometry_sc,
+                                            sinogram_geometry_sc,
+                                            device=device)
+            else:
+                raise NotImplementedError("Cannot use requested CPU to process 3D data. Please use GPU.")
         else:
-            raise NotImplementedError("Cannot process 3D data without a GPU")
+            raise ValueError("Please provide a valid device name: 'gpu' or 'cpu'")
 
         if acquisition_geometry.channels > 1:
             operator_full = ChannelwiseOperator(

Wrappers/Python/test/test_PluginsAstra_Projectors.py

@@ -22,14 +22,15 @@
 from utils import has_astra, has_nvidia, initialise_tests
 from utils_projectors import TestCommon_ProjectionOperatorBlockOperator
 from cil.utilities import dataexample
+from unittest_parametrize import param, parametrize, ParametrizedTestCase
 
 initialise_tests()
 
 if has_astra:
     from cil.plugins.astra.operators import AstraProjector2D, AstraProjector3D
     from cil.plugins.astra.operators import ProjectionOperator
 
-class TestAstraProjectors(unittest.TestCase):
+class TestAstraProjectors(ParametrizedTestCase, unittest.TestCase):
     def setUp(self):
 
         N = 128
@@ -70,6 +71,46 @@ def setUp(self):
 
         self.norm = 14.85
 
+
+    def test_ProjectionOperator_img_geom_default(self):
+        K = ProjectionOperator(image_geometry=None, acquisition_geometry=self.ag, device='gpu')
+        assert(K.volume_geometry == self.ag.get_ImageGeometry())
+
+    def test_ProjectionOperator_acq_geom_default(self):
+        with self.assertRaises(TypeError):
+            ProjectionOperator(image_geometry=self.ig, acquisition_geometry=None, device='gpu')
+
+    def test_ProjectionOperator_all_default(self):
+        with self.assertRaises(TypeError):
+            ProjectionOperator(image_geometry=None, acquisition_geometry=None, device='gpu')
+
+    @parametrize("device, raise_error, err_type", 
+        [param('cpu', False, None, id="cpu_NoError"), 
+         param('CPU', False, None, id="CPU_NoError"), 
+         param('InvalidInput', True, ValueError, id="InvalidInput_ValueError")])
+    @unittest.skipUnless(has_astra, "Requires ASTRA")
+    def test_ProjectionOperator_2Ddata(self, device, raise_error: bool, err_type):
+        if raise_error:
+            with self.assertRaises(err_type):
+                ProjectionOperator(self.ig, self.ag, device)
+        else:
+            assert isinstance(ProjectionOperator(self.ig, self.ag, device), object)
+
+    @parametrize("device, raise_error, err_type", 
+        [param('gpu', False, None, id="gpu_NoError"), 
+         param('GPU', False, None, id="GPU_NoError"),
+         param('cpu', True, NotImplementedError, id="cpu_NotImplementedError"), 
+         param('CPU', True, NotImplementedError, id="CPU_NotImplementedError"),
+         param('InvalidInput', True, ValueError, id="InvalidInput_ValueError")]) 
+    @unittest.skipUnless(has_astra and has_nvidia, "Requires ASTRA GPU")
+    def test_ProjectionOperator_3Ddata(self, device, raise_error: bool, err_type):
+        if raise_error:
+            with self.assertRaises(err_type):
+                ProjectionOperator(self.ig3, self.ag3, device)
+        else:
+            assert isinstance(ProjectionOperator(self.ig3, self.ag3, device), object)
+
+
     def foward_projection(self, A, ig, ag):
         image_data = ig.allocate(None)
         image_data.fill(1)

docs/docs_environment.yml

@@ -37,3 +37,4 @@ dependencies:
   - pip
   - pip:
     - git+https://github.com/pydata/pydata-sphinx-theme
+    - unittest-parametrize # Needs to be here?

docs/source/developer_guide.rst

@@ -16,7 +16,7 @@
 Developers' Guide
 *****************
 
-CIL is an Object Orientated software. It has evolved during the years and it currently does not fully adheres to the following conventions. New additions must comply with
+CIL is an Object Orientated software. It has evolved during the years and it currently does not fully adhere to the following conventions. New additions must comply with
 the following.
 
 Conventions on new CIL objects
@@ -126,14 +126,13 @@ a HTTP server to view the documentation.
 #. Follow the instructions `here <https://github.com/TomographicImaging/CIL/tree/master#building-cil-from-source-code>`_ to create a conda environment and build ``cil`` from source
 #. Go to ``docs`` folder
 #. Install packages from ``docs_environment.yml``
-#. [Install Ruby version 3.2](https://www.ruby-lang.org/en/documentation/installation/#installers)
+#. `Install Ruby version 3.2 <https://www.ruby-lang.org/en/documentation/installation/#installers>`_
 #. Install the web dependencies with ``make web-deps``
 #. Build the documentation with ``make dirhtml web``
 #. Start an HTTP server with ``make serve`` to access the docs via `localhost:8000 <http://localhost:8000>`_.
 
 Example:
 ::
-
   git clone --recurse-submodule git@github.com:TomographicImaging/CIL
   cd CIL
   sh scripts/create_local_env_for_cil_development_tests.sh -n NUMPY_VERSION -p PYTHON_VERSION -e ENVIRONMENT_NAME
@@ -156,6 +155,58 @@ The ``mkdemos.py`` script (called by ``make dirhtml``):
 
 The ``nbsphinx`` extension will convert the ``*.ipynb`` files to HTML.
 
+
+Testing
+=============
+
+Parametrized Tests
+----------
+Methods are tested using Python's `unittest <https://docs.python.org/3/library/unittest.html>`_ library. Please see the documentation `here <https://docs.python.org/3/library/unittest.html#basic-example>`_  for information and usage examples.
+The `unittest-parametrize <https://github.com/adamchainz/unittest-parametrize>`_ library is used to generate parametrized test cases.
+
+The ``@parametrize`` decorator allows you to define multiple sets of input parameters for a single test method, treating each combination as a separate test case. 
+This helps to test various scenarios without duplicating code.
+
+Example: Parameterized Test for ``ProjectionOperator``
+::    
+  @parametrize("device, raise_error, err_type", 
+    [param('cpu', False, None, id="cpu_NoError"), 
+     param('CPU', False, None, id="CPU_NoError"), 
+     param('InvalidInput', True, ValueError, id="InvalidInput_ValueError")])
+
+  def test_ProjectionOperator_2Ddata(self, device, raise_error: bool, err_type):
+      if raise_error:
+          with self.assertRaises(err_type):
+              ProjectionOperator(self.ig, self.ag, device)
+      else:
+          assert isinstance(ProjectionOperator(self.ig, self.ag, device), object)
+
+
+The parameters passed to the test are: The ``device`` (string), which is the device name passed to the ``ProjectionOperator``, 
+``raise_error`` (bool), which specifies if an error is expected during initialisation, and the expected ``err_type``.
+
+In this example, the test instantiates a ``ProjectionOperator`` with a ``device`` name, checks if any errors should be raised, and ensures they are the expected type.
+There are 3 sets of parameters: 
+
+- ``param('cpu', False, None, id="cpu_NoError")`` - Test using the device name 'cpu' (lowercase), and expects no error.  
+- ``param('CPU', False, None, id="CPU_NoError")`` - Test using the device name 'CPU' (uppercase), and expects no error.  
+- ``param('InvalidInput', True, ValueError, id="InvalidInput_ValueError")`` - Test using an invalid string, and expects the ``ValueError`` to be raised.  
+
+Each parameter set has a unique id which can also be customised for easier identification in test outputs (e.g., ``cpu_NoError``, ``InvalidInput_ValueError``)
+
+When running the test, each parameterized case is shown as a distinct result:
+::
+  test_ProjectionOperator_2Ddata[cpu_NoError] ... ok
+  test_ProjectionOperator_2Ddata[CPU_NoError] ... ok
+  test_ProjectionOperator_2Ddata[InvalidInput_ValueError] ... ok
+
+  ----------------------------------------------------------------------
+  Ran 3 tests in 0.001s
+
+  OK
+
+
+
 Contributions guidelines
 ========================
 

scripts/create_local_env_for_cil_development.sh

@@ -20,6 +20,8 @@ python='3.10'
 name=cil
 test_deps=0
 cil_ver=''
+pip_install_pkgs=()
+
 while getopts hn:p:e:tv: option ; do
   case "${option}" in
   n) numpy="${OPTARG}" ;;
@@ -92,6 +94,12 @@ else
     -c ccpi
     --override-channels
   )
+  pip_install_pkgs+=(
+    unittest-parametrize
+  )
 fi
 
-conda "${conda_args[@]}"
+conda "${conda_args[@]}" 
+if [[ -n "${pip_install_pkgs[@]}" ]]; then
+  python -m pip install "${pip_install_pkgs[@]}"
+fi

scripts/requirements-test.yml

@@ -49,3 +49,6 @@ dependencies:
   - numba
   - tqdm
   - zenodo_get >=1.6
+  - pip
+  - pip:
+    - unittest-parametrize