Repository: scikit-rf/scikit-rf

Issue:
Problem with Network.is_symmetric()
There seems to be an issue with the `is_symmetric()` function in the `Network` class. Instead of a return value, I'm getting an error: `ValueError: shape too large to be a matrix.`

Apart from this error, the use of NumPy's `Matrix` class is [no longer recommended](https://numpy.org/doc/stable/reference/generated/numpy.matrix.html). Maybe it's time to upgrade `is_symmetric()`?

Minimum example:
```
import skrf
nw1 = skrf.data.ring_slot
nw1.is_symmetric()
```
Error output:
```
  File "/home/vinc/PycharmProjects/scikit-rf/skrf/network.py", line 1600, in is_symmetric
    mat = npy.matrix(test_network.s)
  File "/home/vinc/miniconda3/envs/skrf-dev/lib/python3.8/site-packages/numpy/matrixlib/defmatrix.py", line 135, in __new__
    new = data.view(subtype)
  File "/home/vinc/miniconda3/envs/skrf-dev/lib/python3.8/site-packages/numpy/matrixlib/defmatrix.py", line 180, in __array_finalize__
    raise ValueError("shape too large to be a matrix.")
ValueError: shape too large to be a matrix.
```




Discussion:

Patch:
diff --git a/skrf/network.py b/skrf/network.py
index 22df7f0b..106dd2a4 100644
--- a/skrf/network.py
+++ b/skrf/network.py
@@ -1554,11 +1554,10 @@ class Network(object):
         '''
         return npy.allclose(reciprocity(self.s), npy.zeros_like(self.s), atol=tol)
 
-    def is_symmetric(self, n: int = 1, port_order: Dict[int, int] = {},
-        tol: float = mf.ALMOST_ZERO) -> bool:
-        '''
-        Returns whether the 2N-port network has n-th order reflection symmetry
-        by checking s_ii == s_jj for appropriate pair(s) of i and j.
+    def is_symmetric(self, n: int = 1, port_order: Dict[int, int] = {}, tol: float = mf.ALMOST_ZERO) -> bool:
+        """
+        Returns whether the 2N-port network has n-th order reflection symmetry by checking
+        :math:`S_{i,i} == S_{j,j}` for appropriate pair(s) of :math:`i` and :math:`j`.
 
         https://en.wikipedia.org/wiki/Two-port_network#Scattering_parameters_(S-parameters)
 
@@ -1568,9 +1567,13 @@ class Network(object):
             Order of line symmetry to test for
         port_order : dict[int, int]
             Renumbering of zero-indexed ports before testing
-        tol: float
+        tol : float
             Tolerance in numeric comparisons
 
+        Returns
+        -------
+        bool
+
         Raises
         ------
         ValueError
@@ -1580,17 +1583,18 @@ class Network(object):
             (4) If port_order is not a valid reindexing of ports
             e.g. specifying x->y but not y->z, specifying x->y twice,
             or using an index outside the range 0 to 2N-1
-        '''
-        z, y, x = self.s.shape  # z is number of frequencies, and x is number of ports (2N)
-        if x % 2 != 0 or x != y:
-            raise ValueError('test of symmetric is only valid for a 2N-port network')
-        N = x // 2
-        if n <= 0 or n > N:
+        """
+
+        nfreqs, ny, nx = self.s.shape  # nfreqs is number of frequencies, and nx, ny both are number of ports (2N)
+        if nx % 2 != 0 or nx != ny:
+            raise ValueError('Using is_symmetric() is only valid for a 2N-port network (N=2,4,6,8,...)')
+        n_ports = nx // 2
+        if n <= 0 or n > n_ports:
             raise ValueError('specified order n = ' + str(n) + ' must be ' +
-                             'between 1 and N = ' + str(N) + ', inclusive')
-        if x % n != 0:
+                             'between 1 and N = ' + str(n_ports) + ', inclusive')
+        if nx % n != 0:
             raise ValueError('specified order n = ' + str(n) + ' must evenly divide ' +
-                             'N = ' + str(N))
+                             'N = ' + str(n_ports))
 
         from_ports = list(map(lambda key: int(key), port_order.keys()))
         to_ports = list(map(lambda val: int(val), port_order.values()))
@@ -1598,12 +1602,12 @@ class Network(object):
         if len(from_ports) > 0 and len(to_ports) > 0:
             test_network.renumber(from_ports, to_ports)
 
-        mat = npy.matrix(test_network.s)
-        offs = npy.array(range(0, N))  # port index offsets from each mirror line
-        for k in range(0, N, N // n):  # iterate through n mirror lines
-            mirror = k*npy.ones_like(offs)
-            i, j = mirror-1 - offs, mirror + offs
-            if not npy.allclose(mat[i, i], mat[j, j], atol=tol):
+        offs = npy.array(range(0, n_ports))  # port index offsets from each mirror line
+        for k in range(0, n_ports, n_ports // n):  # iterate through n mirror lines
+            mirror = k * npy.ones_like(offs)
+            i = mirror - 1 - offs
+            j = mirror + offs
+            if not npy.allclose(test_network.s[:, i, i], test_network.s[:, j, j], atol=tol):
                 return False
         return True
 


Test patch:
diff --git a/skrf/tests/test_network.py b/skrf/tests/test_network.py
index 5831b933..4656acaf 100644
--- a/skrf/tests/test_network.py
+++ b/skrf/tests/test_network.py
@@ -610,31 +610,40 @@ class NetworkTestCase(unittest.TestCase):
 
     def test_is_symmetric(self):
         # 2-port
-        a = rf.Network(f=[1],
-                       s=[[-1, 0],
-                          [0, -1]],
+        a = rf.Network(f=[1, 2, 3],
+                       s=[[[-1, 0], [0, -1]],
+                          [[-2, 0], [0, -2]],
+                          [[1, 0], [0, 1]]],
                        z0=50)
         self.assertTrue(a.is_symmetric(), 'A short is symmetric.')
         self.assertRaises(ValueError, a.is_symmetric, port_order={1: 2})  # error raised by renumber()
         a.s[0, 0, 0] = 1
         self.assertFalse(a.is_symmetric(), 'non-symmetrical')
 
+        # another 2-port (transmission line with 201 frequency samples)
+        nw_line = rf.data.line
+        self.assertTrue(nw_line.is_symmetric())
+
+        # another 2-port (ring slot with 201 frequency samples)
+        nw_ringslot = rf.data.ring_slot
+        self.assertFalse(nw_ringslot.is_symmetric())
+
         # 3-port
-        b = rf.Network(f=[1],
-                       s=[[0, 0.5, 0.5],
-                          [0.5, 0, 0.5],
-                          [0.5, 0.5, 0]],
+        b = rf.Network(f=[1, 3],
+                       s=[[[0, 0.5, 0.5], [0.5, 0, 0.5], [0.5, 0.5, 0]],
+                          [[0, 0.2, 0.2], [0.2, 0, 0.2], [0.2, 0.2, 0]]],
                        z0=50)
         with self.assertRaises(ValueError) as context:
             b.is_symmetric()
-        self.assertEqual(str(context.exception), 'test of symmetric is only valid for a 2N-port network')
+        self.assertEqual(str(context.exception),
+                         'Using is_symmetric() is only valid for a 2N-port network (N=2,4,6,8,...)')
 
         # 4-port
-        c = rf.Network(f=[1],
-                       s=[[0, 1j, 1, 0],
-                          [1j, 0, 0, 1],
-                          [1, 0, 0, 1j],
-                          [0, 1, 1j, 0]],
+        c = rf.Network(f=[1, 3, 5, 6],
+                       s=[[[0, 1j, 1, 0], [1j, 0, 0, 1], [1, 0, 0, 1j], [0, 1, 1j, 0]],
+                          [[0, 0.8j, 0.7, 0], [0.8j, 0, 0, 0.7], [0.7, 0, 0, 0.8j], [0, 0.7, 0.8j, 0]],
+                          [[0, 0.3j, 1, 0], [0.3j, 0, 0, 1], [1, 0, 0, 0.3j], [0, 1, 0.3j, 0]],
+                          [[0, -1j, -1, 0], [-1j, 0, 0, -1], [-1, 0, 0, -1j], [0, -1, -1j, 0]]],
                        z0=50)
         self.assertTrue(c.is_symmetric(n=2), 'This quadrature hybrid coupler is symmetric.')
         self.assertTrue(c.is_symmetric(n=2, port_order={0: 1, 1: 2, 2: 3, 3: 0}),