From b355d998316301ff35b28954804cac9367aabeb8 Mon Sep 17 00:00:00 2001 From: Alessandro Pierro Date: Thu, 27 Apr 2023 15:48:11 +0200 Subject: [PATCH 1/7] Bugfix to enable monitoring aliased vars --- src/lava/magma/core/process/ports/ports.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/lava/magma/core/process/ports/ports.py b/src/lava/magma/core/process/ports/ports.py index 083394d9f..848094870 100644 --- 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.") + # TODO : Implement proper strategy to handle this case + # raise AssertionError( + # "Name of implicit VarPort might conflict" + # " with existing attribute.") + vp.name = vp.name + "_1" setattr(var.process, vp.name, vp) var.process.var_ports.add_members({vp.name: vp}) From adfec48df501a069aad57c37234de2b3f03c3da5 Mon Sep 17 00:00:00 2001 From: PhilippPlank <32519998+PhilippPlank@users.noreply.github.com> Date: Thu, 27 Apr 2023 16:56:05 +0200 Subject: [PATCH 2/7] Update ports.py --- src/lava/magma/core/process/ports/ports.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/lava/magma/core/process/ports/ports.py b/src/lava/magma/core/process/ports/ports.py index 848094870..06378a87d 100644 --- a/src/lava/magma/core/process/ports/ports.py +++ b/src/lava/magma/core/process/ports/ports.py @@ -646,11 +646,16 @@ def create_implicit_var_port(var: Var) -> "ImplicitVarPort": vp.process = var.process # VarPort name could shadow existing attribute if hasattr(var.process, vp.name): - # TODO : Implement proper strategy to handle this case - # raise AssertionError( - # "Name of implicit VarPort might conflict" - # " with existing attribute.") - vp.name = vp.name + "_1" + found_unique_name = False + name = "" + i = 0 + # Find a unique name + while not found_unique_name: + i += 1 + name = vp.name + "_" + str(i) + if not hasattr(var.process, vp.name): + found_unique_name = True + vp.name = name setattr(var.process, vp.name, vp) var.process.var_ports.add_members({vp.name: vp}) From 945fb2883e269ebfeb049598ff2388ab8e7db200 Mon Sep 17 00:00:00 2001 From: PhilippPlank <32519998+PhilippPlank@users.noreply.github.com> Date: Thu, 27 Apr 2023 18:13:45 +0200 Subject: [PATCH 3/7] Update ports.py --- src/lava/magma/core/process/ports/ports.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lava/magma/core/process/ports/ports.py b/src/lava/magma/core/process/ports/ports.py index 06378a87d..6cbbbea10 100644 --- a/src/lava/magma/core/process/ports/ports.py +++ b/src/lava/magma/core/process/ports/ports.py @@ -653,7 +653,7 @@ def create_implicit_var_port(var: Var) -> "ImplicitVarPort": while not found_unique_name: i += 1 name = vp.name + "_" + str(i) - if not hasattr(var.process, vp.name): + if not hasattr(var.process, name): found_unique_name = True vp.name = name setattr(var.process, vp.name, vp) From a113cd6db79df7f443aec6c5f773c4826c4a74b6 Mon Sep 17 00:00:00 2001 From: PhilippPlank <32519998+PhilippPlank@users.noreply.github.com> Date: Thu, 27 Apr 2023 18:26:02 +0200 Subject: [PATCH 4/7] Update test_ports.py --- tests/lava/magma/core/process/ports/test_ports.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/lava/magma/core/process/ports/test_ports.py b/tests/lava/magma/core/process/ports/test_ports.py index 33f155814..25a06c92d 100644 --- 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,12 @@ 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) + + # 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): From 23552de4ccbb85b8f213c97e4f749570b701ad0e Mon Sep 17 00:00:00 2001 From: PhilippPlank <32519998+PhilippPlank@users.noreply.github.com> Date: Thu, 27 Apr 2023 19:03:49 +0200 Subject: [PATCH 5/7] Update test_ports.py --- tests/lava/magma/core/process/ports/test_ports.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/lava/magma/core/process/ports/test_ports.py b/tests/lava/magma/core/process/ports/test_ports.py index 25a06c92d..594dcaab2 100644 --- a/tests/lava/magma/core/process/ports/test_ports.py +++ b/tests/lava/magma/core/process/ports/test_ports.py @@ -310,6 +310,15 @@ class VarProcess(AbstractProcess): # ... and connect it directly via connect_var(..) 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") From 600f0c114309f42cf0f583bf58492171baf15b67 Mon Sep 17 00:00:00 2001 From: Alessandro Pierro Date: Fri, 28 Apr 2023 14:30:30 +0200 Subject: [PATCH 6/7] Address review feedback --- src/lava/magma/core/process/ports/ports.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/lava/magma/core/process/ports/ports.py b/src/lava/magma/core/process/ports/ports.py index 6cbbbea10..3bdba4e65 100644 --- a/src/lava/magma/core/process/ports/ports.py +++ b/src/lava/magma/core/process/ports/ports.py @@ -646,16 +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): - found_unique_name = False - name = "" - i = 0 - # Find a unique name - while not found_unique_name: - i += 1 - name = vp.name + "_" + str(i) - if not hasattr(var.process, name): - found_unique_name = True - vp.name = name + name = str(vp.name) + name_suffix = 0 + 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}) From 2493607becde491d796b3ddc1ee00077102d83be Mon Sep 17 00:00:00 2001 From: Alessandro Pierro Date: Fri, 28 Apr 2023 17:49:54 +0200 Subject: [PATCH 7/7] Match expected behaviour for implicit var port naming --- src/lava/magma/core/process/ports/ports.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lava/magma/core/process/ports/ports.py b/src/lava/magma/core/process/ports/ports.py index 3bdba4e65..ad4eb66d8 100644 --- a/src/lava/magma/core/process/ports/ports.py +++ b/src/lava/magma/core/process/ports/ports.py @@ -647,7 +647,7 @@ def create_implicit_var_port(var: Var) -> "ImplicitVarPort": # VarPort name could shadow existing attribute if hasattr(var.process, vp.name): name = str(vp.name) - name_suffix = 0 + name_suffix = 1 while hasattr(var.process, vp.name): vp.name = name + "_" + str(name_suffix) name_suffix += 1