Bugfix to enable monitoring aliased vars (lava-nc#675)

* Bugfix to enable monitoring aliased vars * Update ports.py * Update ports.py * Update test_ports.py * Update test_ports.py * Address review feedback * Match expected behaviour for implicit var port naming --------- Co-authored-by: PhilippPlank <[email protected]>
monkin77 · May 2, 2023 · a042683 · a042683
1 parent 0ddfe38
commit a042683
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 7 deletions.
diff --git a/src/lava/magma/core/process/ports/ports.py b/src/lava/magma/core/process/ports/ports.py
@@ -646,9 +646,11 @@ def create_implicit_var_port(var: Var) -> "ImplicitVarPort":
             vp.process = var.process
             # VarPort name could shadow existing attribute
             if hasattr(var.process, vp.name):
-                raise AssertionError(
-                    "Name of implicit VarPort might conflict"
-                    " with existing attribute.")
+                name = str(vp.name)
+                name_suffix = 1
+                while hasattr(var.process, vp.name):
+                    vp.name = name + "_" + str(name_suffix)
+                    name_suffix += 1
             setattr(var.process, vp.name, vp)
             var.process.var_ports.add_members({vp.name: vp})
 

diff --git a/tests/lava/magma/core/process/ports/test_ports.py b/tests/lava/magma/core/process/ports/test_ports.py
@@ -290,7 +290,8 @@ class VarProcess(AbstractProcess):
 
     def test_connect_RefPort_to_Var_process_conflict(self):
         """Checks connecting RefPort implicitly to Var, with registered
-        processes and conflicting names. -> AssertionError"""
+        processes and conflicting names. -> adding _k with k=1,2,3... to
+        the name."""
 
         # Create a mock parent process
         class VarProcess(AbstractProcess):
@@ -307,9 +308,21 @@ class VarProcess(AbstractProcess):
         v.name = "existing_attr"
 
         # ... and connect it directly via connect_var(..)
-        # The naming conflict should raise an AssertionError
-        with self.assertRaises(AssertionError):
-            rp.connect_var(v)
+        rp.connect_var(v)
+
+        # This has the same effect as connecting a RefPort explicitly via a
+        # VarPort to a Var...
+        self.assertEqual(rp.get_dst_vars(), [v])
+        # ... but still creates a VarPort implicitly
+        vp = rp.get_dst_ports()[0]
+        self.assertIsInstance(vp, VarPort)
+        # ... which wraps the original Var
+        self.assertEqual(vp.var, v)
+
+        # In this case, the VarPort inherits its name and parent process from
+        # the Var it wraps + _implicit_port + _k with k=1,2,3...
+        self.assertEqual(vp.name, "_" + v.name + "_implicit_port" + "_1")
+        self.assertEqual(vp.process, v.process)
 
     @unittest.skip("Currently not supported")
     def test_connect_RefPort_to_many_Vars(self):