Repository: DARMA-tasking/LB-analysis-framework

Issue:
is reading data from json file working correctly?
Random question after analysing the code in `lbsVTDataReader.py`:

- `comm_dict` is first created at top level
https://github.com/DARMA-tasking/LB-analysis-framework/blob/f93c5aad550a283dabb8d27d55167b47ebc8b422/src/lbaf/IO/lbsVTDataReader.py#L142-L144
- then for every phase, it gets re-initialized
https://github.com/DARMA-tasking/LB-analysis-framework/blob/f93c5aad550a283dabb8d27d55167b47ebc8b422/src/lbaf/IO/lbsVTDataReader.py#L151-L156
- inside the loop the communication edges are written into it
https://github.com/DARMA-tasking/LB-analysis-framework/blob/f93c5aad550a283dabb8d27d55167b47ebc8b422/src/lbaf/IO/lbsVTDataReader.py#L188-L192
- at the end `comm_dict` is returned
https://github.com/DARMA-tasking/LB-analysis-framework/blob/f93c5aad550a283dabb8d27d55167b47ebc8b422/src/lbaf/IO/lbsVTDataReader.py#L213


As far as I understand only the data from the last phase gets returned. Is that intended / correct behavior?

Discussion:

Patch:
diff --git a/src/lbaf/IO/lbsVTDataReader.py b/src/lbaf/IO/lbsVTDataReader.py
index 9fd56b1..a977197 100644
--- a/src/lbaf/IO/lbsVTDataReader.py
+++ b/src/lbaf/IO/lbsVTDataReader.py
@@ -90,15 +90,16 @@ class LoadReader:
                 sys.exit(1)
 
             # Merge rank communication with existing ones
-            for k, v in rank_comm.items():
-                if k in communications:
-                    c = communications[k]
-                    c.get("sent").extend(v.get("sent"))
-                    c.get("received").extend(v.get("received"))
-                else:
-                    communications[k] = v
-
-        # Build dictionnary of rank objects
+            if rank_comm.get(phase_id) is not None:
+                for k, v in rank_comm[phase_id].items():
+                    if k in communications:
+                        c = communications[k]
+                        c.get("sent").extend(v.get("sent"))
+                        c.get("received").extend(v.get("received"))
+                    else:
+                        communications[k] = v
+
+        # Build dictionary of rank objects
         rank_objects_set = set()
         for rank in rank_list:
             rank_objects_set.update(rank.get_objects())
@@ -153,15 +154,11 @@ class LoadReader:
             phase_id = phase["id"]
 
             # Create communicator dictionary
-            comm_dict = {}
-
-            # Temporary communication list to avoid duplicates
-            temp_comm = []
+            comm_dict[phase_id] = {}
 
             # Add communications to the object
             communications = phase.get("communications")
-            if communications and communications not in temp_comm:
-                temp_comm.append(communications)
+            if communications:
                 for num, comm in enumerate(communications):
                     # Retrieve communication attributes
                     c_type = comm.get("type")
@@ -175,21 +172,16 @@ class LoadReader:
                         if c_to.get("type") == "object" and c_from.get("type") == "object":
                             # Create receiver if it does not exist
                             receiver_obj_id = c_to.get("id")
-                            comm_dict.setdefault(
-                                receiver_obj_id,
-                                {"sent": [], "received": []})
+                            comm_dict[phase_id].setdefault(receiver_obj_id, {"sent": [], "received": []})
 
                             # Create sender if it does not exist
                             sender_obj_id = c_from.get("id")
-                            comm_dict.setdefault(
-                                sender_obj_id,
-                                {"sent": [], "received": []})
+                            comm_dict[phase_id].setdefault(sender_obj_id, {"sent": [], "received": []})
 
                             # Create communication edges
-                            comm_dict[receiver_obj_id]["received"].append(
-                                {"from": c_from.get("id"), "bytes": c_bytes})
-                            comm_dict[sender_obj_id]["sent"].append(
-                                {"to": c_to.get("id"), "bytes": c_bytes})
+                            comm_dict[phase_id][receiver_obj_id]["received"].append({"from": c_from.get("id"),
+                                                                                     "bytes": c_bytes})
+                            comm_dict[phase_id][sender_obj_id]["sent"].append({"to": c_to.get("id"), "bytes": c_bytes})
                             self.__logger.debug(f"Added communication {num} to phase {phase_id}")
                             for k, v in comm.items():
                                 self.__logger.debug(f"{k}: {v}")


Test patch:
diff --git a/tests/test_lbs_vt_statistics_reader.py b/tests/test_lbs_vt_data_reader.py
similarity index 91%
rename from tests/test_lbs_vt_statistics_reader.py
rename to tests/test_lbs_vt_data_reader.py
index de9d81d..190c2cd 100644
--- a/tests/test_lbs_vt_statistics_reader.py
+++ b/tests/test_lbs_vt_data_reader.py
@@ -29,24 +29,24 @@ class TestConfig(unittest.TestCase):
         self.logger = logging.getLogger()
         self.lr = LoadReader(file_prefix=self.file_prefix, logger=self.logger, file_suffix='json')
         self.ranks_comm = [
-            {
+            {0: {
                 5: {'sent': [], 'received': [{'from': 0, 'bytes': 2.0}]},
                 0: {'sent': [{'to': 5, 'bytes': 2.0}], 'received': []},
                 4: {'sent': [], 'received': [{'from': 1, 'bytes': 1.0}]},
                 1: {'sent': [{'to': 4, 'bytes': 1.0}], 'received': []},
                 2: {'sent': [], 'received': [{'from': 3, 'bytes': 1.0}]},
                 3: {'sent': [{'to': 2, 'bytes': 1.0}, {'to': 8, 'bytes': 0.5}], 'received': []},
-                8: {'sent': [], 'received': [{'from': 3, 'bytes': 0.5}]}},
-            {
+                8: {'sent': [], 'received': [{'from': 3, 'bytes': 0.5}]}}},
+            {0: {
                 1: {'sent': [], 'received': [{'from': 4, 'bytes': 2.0}]},
                 4: {'sent': [{'to': 1, 'bytes': 2.0}], 'received': []},
                 8: {'sent': [], 'received': [{'from': 5, 'bytes': 2.0}]},
                 5: {'sent': [{'to': 8, 'bytes': 2.0}], 'received': []},
                 6: {'sent': [], 'received': [{'from': 7, 'bytes': 1.0}]},
-                7: {'sent': [{'to': 6, 'bytes': 1.0}], 'received': []}},
-            {
+                7: {'sent': [{'to': 6, 'bytes': 1.0}], 'received': []}}},
+            {0: {
                 6: {'sent': [], 'received': [{'from': 8, 'bytes': 1.5}]},
-                8: {'sent': [{'to': 6, 'bytes': 1.5}], 'received': []}},
+                8: {'sent': [{'to': 6, 'bytes': 1.5}], 'received': []}}},
             {}
         ]
         self.ranks_iter_map = [{0: Rank(i=0, mo={Object(i=3, t=0.5), Object(i=2, t=0.5), Object(i=0, t=1.0),
@@ -56,24 +56,24 @@ class TestConfig(unittest.TestCase):
                                {0: Rank(i=2, mo={Object(i=8, t=1.5)}, logger=self.logger)},
                                {0: Rank(i=3, logger=self.logger)}]
 
-    def test_lbs_vt_statistics_reader_initialization(self):
+    def test_lbs_vt_data_reader_initialization(self):
         self.assertEqual(self.lr._LoadReader__file_prefix, self.file_prefix)
         self.assertEqual(self.lr._LoadReader__file_suffix, 'json')
 
-    def test_lbs_vt_statistics_reader_get_node_trace_file_name_001(self):
+    def test_lbs_vt_data_reader_get_node_trace_file_name_001(self):
         file_name = f"{self.lr._LoadReader__file_prefix}.0.{self.lr._LoadReader__file_suffix}"
         self.assertEqual(file_name, self.lr.get_node_trace_file_name(node_id=0))
 
-    def test_lbs_vt_statistics_reader_get_node_trace_file_name_002(self):
+    def test_lbs_vt_data_reader_get_node_trace_file_name_002(self):
         file_name = f"{self.lr._LoadReader__file_prefix}.100.{self.lr._LoadReader__file_suffix}"
         self.assertEqual(file_name, self.lr.get_node_trace_file_name(node_id=100))
 
-    def test_lbs_vt_statistics_reader_get_node_trace_file_name_003(self):
+    def test_lbs_vt_data_reader_get_node_trace_file_name_003(self):
         # Node_id is an in 000 is converted to 0
         file_name = f"{self.lr._LoadReader__file_prefix}.000.{self.lr._LoadReader__file_suffix}"
         self.assertNotEqual(file_name, self.lr.get_node_trace_file_name(node_id=000))
 
-    def test_lbs_vt_statistics_reader_read(self):
+    def test_lbs_vt_data_reader_read(self):
         for phase in range(4):
             rank_iter_map, rank_comm = self.lr.read(phase, 0)
             self.assertEqual(self.ranks_comm[phase], rank_comm)
@@ -87,7 +87,7 @@ class TestConfig(unittest.TestCase):
             self.assertEqual(prep_time_list, gen_time_list)
             self.assertEqual(prep_id_list, gen_id_list)
 
-    def test_lbs_vt_statistics_reader_read_compressed(self):
+    def test_lbs_vt_data_reader_read_compressed(self):
         file_prefix = os.path.join(self.data_dir, 'synthetic_lb_stats_compressed', 'data')
         lr = LoadReader(file_prefix=file_prefix, logger=self.logger, file_suffix='json')
         for phase in range(4):
@@ -103,12 +103,12 @@ class TestConfig(unittest.TestCase):
             self.assertEqual(prep_time_list, gen_time_list)
             self.assertEqual(prep_id_list, gen_id_list)
 
-    def test_lbs_vt_statistics_reader_read_file_not_found(self):
+    def test_lbs_vt_data_reader_read_file_not_found(self):
         with self.assertRaises(FileNotFoundError) as err:
             LoadReader(file_prefix=f"{self.file_prefix}xd", logger=self.logger, file_suffix='json').read(0, 0)
         self.assertEqual(err.exception.args[0], f"File {self.file_prefix}xd.0.json not found!")
 
-    def test_lbs_vt_statistics_reader_read_wrong_schema(self):
+    def test_lbs_vt_data_reader_read_wrong_schema(self):
         file_prefix = os.path.join(self.data_dir, 'synthetic_lb_stats_wrong_schema', 'data')
         with self.assertRaises(SchemaError) as err:
             LoadReader(file_prefix=file_prefix, logger=self.logger, file_suffix='json').read(0, 0)
@@ -116,7 +116,7 @@ class TestConfig(unittest.TestCase):
             err_msg = se.read()
         self.assertEqual(err.exception.args[0], err_msg)
 
-    def test_lbs_vt_statistics_reader_json_reader(self):
+    def test_lbs_vt_data_reader_json_reader(self):
         for phase in range(4):
             file_name = self.lr.get_node_trace_file_name(phase)
             rank_iter_map, rank_comm = self.lr.json_reader(returned_dict={}, file_name=file_name, phase_ids=0,