Merge pull request #186 from april-tools/clean_todo

Clean some TODOs

cirkit/new/layers/inner/product/product.py

@@ -9,7 +9,8 @@ class ProductLayer(InnerLayer):
 
     # We still accept any Reparameterization instance for reparam, but it will be ignored.
     # TODO: this disable should be a pylint bug, because reparam is fixed to None
-    def __init__(  # pylint: disable=useless-parent-delegation
+    # pylint: disable-next=useless-parent-delegation
+    def __init__(
         self,
         *,
         num_input_units: int,

cirkit/new/layers/input/constant.py

@@ -14,7 +14,8 @@ class ConstantLayer(InputLayer):
 
     # We still accept any Reparameterization instance for reparam, but it will be ignored.
     # DISABLE: It's designed to have these arguments.
-    def __init__(  # pylint: disable=too-many-arguments
+    # pylint: disable-next=too-many-arguments
+    def __init__(
         self,
         *,
         num_input_units: int,

cirkit/new/layers/input/exp_family/categorical.py

@@ -1,5 +1,4 @@
-from typing import NoReturn
-from typing_extensions import Self  # FUTURE: in typing from 3.11
+from typing_extensions import Never, Self  # FUTURE: in typing from 3.11
 
 import torch
 from torch import Tensor
@@ -17,7 +16,8 @@ class CategoricalLayer(ExpFamilyLayer):
     """
 
     # DISABLE: It's designed to have these arguments.
-    def __init__(  # pylint: disable=too-many-arguments
+    # pylint: disable-next=too-many-arguments
+    def __init__(
         self,
         *,
         num_input_units: int,
@@ -95,7 +95,7 @@ def log_partition(self, eta: Tensor) -> Tensor:
     @classmethod
     def get_partial(  # type: ignore[misc]
         cls, symb_cfg: SymbLayerCfg[Self], *, order: int = 1, var_idx: int = 0, ch_idx: int = 0
-    ) -> NoReturn:
+    ) -> Never:
         """Get the symbolic config to construct the partial differential w.r.t. the given channel \
         of the given variable in the scope of this layer.
 

cirkit/new/layers/input/exp_family/diff_ef.py

@@ -24,7 +24,8 @@ class DiffEFLayer(InputLayer):
 
     # DISABLE: It's designed to have these arguments.
     # IGNORE: Unavoidable for kwargs.
-    def __init__(  # type: ignore[misc]  # pylint: disable=too-many-arguments
+    # pylint: disable-next=too-many-arguments
+    def __init__(  # type: ignore[misc]
         self,
         *,
         num_input_units: int,

cirkit/new/layers/input/exp_family/exp_family.py

@@ -16,12 +16,12 @@ class ExpFamilyLayer(InputLayer):
     """The abstract base class for Exponential Family distribution layers.
 
     Exponential Family dist:
-        f(x|theta) = exp(η(theta) · T(x) + log_h(x) - A(η)).
+        f(x|θ) = exp(η(θ) · T(x) + log_h(x) - A(η)).
     Ref: https://en.wikipedia.org/wiki/Exponential_family#Table_of_distributions.
 
-    However here we directly parameterize η instead of theta:
+    However here we directly parameterize η instead of θ:
         f(x|η) = exp(η · T(x) + log_h(x) - A(η)).
-    Implemtations provide inverse mapping from η to theta.
+    Implemtations provide inverse mapping from η to θ.
 
     This implementation is fully factorized over the variables if used as multivariate, i.e., \
     equivalent to num_vars (arity) univariate EF distributions followed by a Hadamard product of \

cirkit/new/layers/input/exp_family/prod_ef.py

@@ -31,7 +31,8 @@ class ProdEFLayer(ExpFamilyLayer):
 
     # DISABLE: It's designed to have these arguments.
     # IGNORE: Unavoidable for kwargs.
-    def __init__(  # type: ignore[misc]  # pylint: disable=too-many-arguments
+    # pylint: disable-next=too-many-arguments
+    def __init__(  # type: ignore[misc]
         self,
         *,
         num_input_units: int,

cirkit/new/layers/input/input.py

@@ -10,7 +10,7 @@
 class InputLayer(Layer):
     """The abstract base class for input layers."""
 
-    # NOTE: We use exactly the same interface (H, *B, K)->(*B, K) for __call__ of input layers:
+    # NOTE: We use exactly the same interface (H, *B, K) -> (*B, K) for __call__ of input layers:
     #           1. Define arity(H)=num_vars(D), reusing the H dimension.
     #           2. Define num_input_units(K)=num_channels(C), which reuses the K dimension.
     #       For dimension D (variables), we should parse the input in circuit according to the

cirkit/new/model/functional.py

@@ -17,7 +17,7 @@ def integrate(
     Returns:
         TensorizedCircuit: The circuit giving the definite integral.
     """
-    return self.__class__(self.symb_circuit.integrate(scope=scope))
+    return type(self)(self.symb_circuit.integrate(scope=scope))
 
 
 def differentiate(self: "TensorizedCircuit", *, order: int = 1) -> "TensorizedCircuit":
@@ -37,7 +37,7 @@ def differentiate(self: "TensorizedCircuit", *, order: int = 1) -> "TensorizedCi
     Returns:
         TensorizedCircuit: The circuit giving the (total) differential.
     """
-    return self.__class__(self.symb_circuit.differentiate(order=order))
+    return type(self)(self.symb_circuit.differentiate(order=order))
 
 
 def product(self: "TensorizedCircuit", other: "TensorizedCircuit") -> "TensorizedCircuit":
@@ -50,4 +50,4 @@ def product(self: "TensorizedCircuit", other: "TensorizedCircuit") -> "Tensorize
     Returns:
         SymbolicTensorizedCircuit: The circuit product.
     """
-    return self.__class__(self.symb_circuit.product(other.symb_circuit))
+    return type(self)(self.symb_circuit.product(other.symb_circuit))

cirkit/new/model/tensorized_circuit.py

@@ -15,8 +15,8 @@
 )
 
 
-# TODO: this final may not be wanted for user customization, but use of __class__ in TCF require it.
-# Mark this class final so that __class__ of a TensC is always TensorizedCircuit.
+# TODO: this final may not be wanted for user customization, but use of type() in TCF requires it.
+# Mark this class final so that type(TensC) is always TensorizedCircuit.
 @final
 class TensorizedCircuit(nn.Module):
     """The tensorized circuit with concrete computational graph in PyTorch.

cirkit/new/region_graph/algorithms/poon_domingos.py

@@ -113,7 +113,8 @@ def _parse_delta(
 
 # TODO: too-complex,too-many-locals. how to solve?
 # DISABLE: We use function name with upper case to mimic a class constructor.
-def PoonDomingos(  # pylint: disable=invalid-name,too-complex,too-many-locals
+# pylint: disable-next=invalid-name,too-complex,too-many-locals
+def PoonDomingos(
     shape: Sequence[int],
     *,
     delta: Union[float, List[float], List[List[float]]],
@@ -159,9 +160,21 @@ def PoonDomingos(  # pylint: disable=invalid-name,too-complex,too-many-locals
     queue.append(cur_hypercube)
     depth_dict[cur_hypercube] = 0
 
-    # DISABLE: It's intended to use while loop.
-    while queue:  # pylint: disable=while-used
-        cur_hypercube = queue.popleft()
+    # DISABLE: This is considered a constant.
+    SENTINEL = ((-1,) * len(shape), (-1,) * len(shape))  # pylint: disable=invalid-name
+
+    def queue_popleft() -> HyperCube:
+        """Wrap queue.popleft() with sentinel value.
+
+        Returns:
+            HyperCube: The result of queue.popleft(), or SENTINEL when queue is exhausted.
+        """
+        try:
+            return queue.popleft()
+        except IndexError:
+            return SENTINEL
+
+    for cur_hypercube in iter(queue_popleft, SENTINEL):
         if depth_dict[cur_hypercube] > max_depth:
             continue
 

cirkit/new/region_graph/algorithms/quad_tree.py

@@ -4,7 +4,6 @@
 from cirkit.new.region_graph.algorithms.utils import HypercubeToScope
 from cirkit.new.region_graph.region_graph import RegionGraph
 from cirkit.new.region_graph.rg_node import RegionNode
-from cirkit.new.utils import Scope
 
 # TODO: now should work with H!=W but need tests
 
@@ -102,26 +101,22 @@ def QuadTree(shape: Tuple[int, int], *, struct_decomp: bool = False) -> RegionGr
     hypercube_to_scope = HypercubeToScope(shape)
 
     # Padding using Scope({num_var}) which is one larger than range(num_var).
-    pad_scope = Scope({height * width})
-    # The regions of the current layer, in shape (H, W).
-    layer = [[RegionNode(pad_scope)] * (width + 1) for _ in range(height + 1)]
+    # DISABLE: This is considered a constant here, although RegionNode is mutable.
+    PADDING = RegionNode({height * width})  # pylint: disable=invalid-name
+    # The regions of the current layer, in shape (H, W). The same PADDING object is reused.
+    layer = [[PADDING] * (width + 1) for _ in range(height + 1)]
 
     # Add univariate input nodes.
     for i, j in itertools.product(range(height), range(width)):
         node = RegionNode(hypercube_to_scope[((i, j), (i + 1, j + 1))])
         layer[i][j] = node
         graph.add_node(node)
 
-    # Merge layer by layer.
-    # TODO: or not using while loop?
-    while height > 1 or width > 1:  # pylint: disable=while-used
-        prev_height = height
-        prev_width = width
-        prev_layer = layer
-
-        height = (prev_height + 1) // 2
-        width = (prev_width + 1) // 2
-        layer = [[RegionNode(pad_scope)] * (width + 1) for _ in range(height + 1)]
+    # Merge layer by layer, loop until (H, W)==(1, 1).
+    for _ in range((max(height, width) - 1).bit_length()):
+        height = (height + 1) // 2
+        width = (width + 1) // 2
+        prev_layer, layer = layer, [[PADDING] * (width + 1) for _ in range(height + 1)]
 
         for i, j in itertools.product(range(height), range(width)):
             regions = tuple(  # Filter valid regions in the 4 possible sub-regions.
@@ -132,7 +127,7 @@ def QuadTree(shape: Tuple[int, int], *, struct_decomp: bool = False) -> RegionGr
                     prev_layer[i * 2 + 1][j * 2],
                     prev_layer[i * 2 + 1][j * 2 + 1],
                 )
-                if node.scope != pad_scope
+                if node.scope != PADDING
             )
             if len(regions) == 1:
                 node = regions[0]

cirkit/new/region_graph/algorithms/utils.py

@@ -1,3 +1,4 @@
+import math
 from typing import Dict, Sequence, Tuple
 
 import numpy as np
@@ -30,7 +31,9 @@ def __init__(self, shape: Sequence[int]) -> None:
         self.shape = tuple(shape)
         # We assume it's feasible to save the whole hypercube, since it should be the whole region.
         # ANNOTATE: Numpy has typing issues.
-        self.hypercube: NDArray[np.int64] = np.arange(np.prod(shape), dtype=np.int64).reshape(shape)
+        self.hypercube: NDArray[np.int64] = np.arange(math.prod(shape), dtype=np.int64).reshape(
+            shape
+        )
 
     def __missing__(self, key: HyperCube) -> Scope:
         """Construct the item when not exist in the dict.