Improve Validation and Error Handling in QCNN Sequence Method (#147)

* changed the error handling logic in qcnn.py

* added tests to test_qcnn.py and deleted previous one. updated qcnn.py fixing linting issues.

* fixed regex matching

* suggested changes

---------

Co-authored-by: Saasha Joshi <32019413+SaashaJoshi@users.noreply.github.com>

piqture/neural_networks/qcnn.py

@@ -12,70 +12,91 @@
 
 from __future__ import annotations
 
-from typing import Callable
+from inspect import isclass
+from typing import Type
 
 from qiskit.circuit import QuantumCircuit
 
+from piqture.neural_networks.layers.base_layer import BaseLayer
 from piqture.neural_networks.quantum_neural_network import QuantumNeuralNetwork
 
-# pylint: disable=too-few-public-methods
-
 
 class QCNN(QuantumNeuralNetwork):
     """
-    Builds a Quantum Neural Network circuit with the help of
-    convolutional, pooling or fully-connected layers.
-
-    References:
-        [1] I. Cong, S. Choi, and M. D. Lukin, “Quantum
-        convolutional neural networks,” Nature Physics,
-        vol. 15, no. 12, pp. 1273–1278, Aug. 2019,
-        doi: https://doi.org/10.1038/s41567-019-0648-8.
+    A Quantum Convolutional Neural Network implementation.
+
+    This class implements a quantum convolutional neural network by extending
+    the base QuantumNeuralNetwork class. It provides functionality to build
+    quantum circuits with convolutional-style quantum operations.
     """
 
     def __init__(self, num_qubits: int):
         """
-        Initializes a Quantum Neural Network circuit with the given
-        number of qubits.
+        Initialize a Quantum Neural Network circuit with the given number of qubits.
 
         Args:
-            num_qubits (int): builds a quantum convolutional neural
-            network circuit with the given number of qubits or image
-            dimensions.
+            num_qubits (int): Number of qubits to use in the quantum convolutional
+                             neural network circuit. This determines the dimensions
+                             of the input that can be processed.
         """
         QuantumNeuralNetwork.__init__(self, num_qubits)
 
-    def sequence(self, operations: list[tuple[Callable, dict]]) -> QuantumCircuit:
+    def sequence(
+        self, operations: list[tuple[Type[BaseLayer], dict]]
+    ) -> QuantumCircuit:
         """
-        Builds a QNN circuit by composing the circuit with given
-        sequence of list of operations.
+        Build a QNN circuit by composing the circuit with given sequence of operations.
 
         Args:
-            operations (list[tuple[Callable, dict]]: a tuple
-            of a Layer object and a dictionary of its arguments.
+            operations (list[tuple[Type[BaseLayer], dict]]): A list of tuples where
+                      each tuple contains a Layer class that inherits from BaseLayer
+                      and a dictionary of its arguments.
 
         Returns:
-            circuit (QuantumCircuit): final QNN circuit with all the
-            layers.
+            QuantumCircuit: Final QNN circuit with all the layers applied.
+
+        Raises:
+            TypeError: If operations format is invalid or if any operation doesn't
+                      inherit from BaseLayer.
+            ValueError: If operations list is empty.
         """
+        # Validate operations list
         if not isinstance(operations, list):
             raise TypeError("The input operations must be of the type list.")
 
+        if not operations:
+            raise ValueError("The operations list cannot be empty.")
+
         if not all(isinstance(operation, tuple) for operation in operations):
             raise TypeError(
                 "The input operations list must contain tuple[operation, params]."
             )
 
-        if not callable(operations[0][0]):
-            raise TypeError(
-                "Operation in input operations list must be Callable functions/classes."
-            )
-
-        if not isinstance(operations[0][1], dict):
-            raise TypeError(
-                "Parameters of operation in input operations list must be in a dictionary."
-            )
-
+        # Validate each operation
+        for idx, (layer, params) in enumerate(operations):
+            # Check if it's a class and inherits from BaseLayer
+            if not isclass(layer):
+                raise TypeError(
+                    f"Operation at index {idx} must be a class, got {type(layer).__name__}"
+                )
+
+            if not issubclass(layer, BaseLayer):
+                raise TypeError(
+                    f"Operation at index {idx} must inherit from BaseLayer, got {layer.__name__}"
+                )
+
+            # Prevent using BaseLayer itself
+            if layer is BaseLayer:
+                raise TypeError(f"Operation at index {idx} cannot be BaseLayer itself.")
+
+            # Validate parameters
+            if not isinstance(params, dict):
+                raise TypeError(
+                    f"Parameters of operation at index {idx} must be in a dictionary, "
+                    f"got {type(params).__name__}"
+                )
+
+        # Build the circuit
         unmeasured_bits = list(range(self.num_qubits))
         for layer, params in operations:
             layer_instance = layer(
@@ -84,8 +105,6 @@ def sequence(self, operations: list[tuple[Callable, dict]]) -> QuantumCircuit:
                 unmeasured_bits=unmeasured_bits,
                 **params,
             )
-            # Optionally collect circuit since it is
-            # composed in place.
             _, unmeasured_bits = layer_instance.build_layer()
 
         return self.circuit

tests/neural_networks/test_qcnn.py

@@ -21,11 +21,13 @@
 
 from piqture.neural_networks import QCNN
 from piqture.neural_networks.layers import (
+    BaseLayer,
     FullyConnectedLayer,
     QuantumConvolutionalLayer,
     QuantumPoolingLayer2,
     QuantumPoolingLayer3,
 )
+from piqture.tensor_networks import MERA
 
 
 class TestQCNN:
@@ -78,13 +80,20 @@ def test_operation_tuple(self, num_qubits, operations):
 
     @pytest.mark.parametrize(
         "num_qubits, operations",
-        [(2, [([], {})]), (3, [(pytest.mark, {})]), (4, [(1, {})]), (5, [("abc", {})])],
+        [
+            (2, [([], {})]),
+            (3, [(pytest.mark, {})]),
+            (4, [(1, {})]),
+            (5, [("abc", {})]),
+            (2, [(print, {})]),
+            (3, [(sum, {})]),
+            (4, [(lambda x: x, {})]),
+        ],
     )
-    def test_operation(self, num_qubits, operations):
+    def test_operation_isclass(self, num_qubits, operations):
         """Tests the type of operation in operation tuple."""
         with raises(
-            TypeError,
-            match="Operation in input operations list must be Callable functions/classes.",
+            TypeError, match=r"Operation at index \d+\ must be a class, got .*?"
         ):
             _ = QCNN(num_qubits).sequence(operations)
 
@@ -101,7 +110,7 @@ def test_params(self, num_qubits, operations):
         """Tests the type of params in operation tuple."""
         with raises(
             TypeError,
-            match="Parameters of operation in input operations list must be in a dictionary.",
+            match=r"Parameters of operation at index \d+ must be in a dictionary, got .*?",
         ):
             _ = QCNN(num_qubits).sequence(operations)
 
@@ -140,3 +149,33 @@ def test_sequence(self, num_qubits, operations):
                 or mock_quantum_pooling_layer3.call_count > 0
                 or mock_fully_connected_layer.call_count > 0
             )
+
+    @pytest.mark.parametrize(
+        "num_qubits, operations",
+        [
+            (2, [(dict, {})]),
+            (3, [(str, {})]),
+            (4, [(object, {})]),
+            (2, [(MERA, {})]),
+            (3, [(QCNN, {})]),
+        ],
+    )
+    def test_non_baselayer_operation(self, num_qubits, operations):
+        """Tests if the layer operation inherits from the abstract BaseLayer class."""
+        with raises(
+            TypeError,
+            match=r"Operation at index \d+\ must inherit from BaseLayer, got \*?",
+        ):
+            _ = QCNN(num_qubits).sequence(operations)
+
+    @pytest.mark.parametrize(
+        "num_qubits, operations",
+        [(2, [(BaseLayer, {})])],
+    )
+    def test_baselayer(self, num_qubits, operations):
+        """Tests if the layer operation is the abstract BaseLayer class."""
+        with raises(
+            TypeError,
+            match=r"Operation at index \d+\ cannot be BaseLayer itself.",
+        ):
+            _ = QCNN(num_qubits).sequence(operations)