Repository: siliconcompiler/siliconcompiler

Issue:
cli dashboard should not access chip object in render loop
### Feature Description

https://github.com/siliconcompiler/siliconcompiler/blob/e1fc955d9f5bc8168d762f7c452c527c18df28ce/siliconcompiler/report/dashboard/cli/__init__.py#L473

This should be handled in the get_job data along with generating a priority list of nodes to display

Discussion:

Solution patch:
diff --git a/siliconcompiler/report/dashboard/cli/__init__.py b/siliconcompiler/report/dashboard/cli/__init__.py
index 3352d3405..30a696379 100644
--- a/siliconcompiler/report/dashboard/cli/__init__.py
+++ b/siliconcompiler/report/dashboard/cli/__init__.py
@@ -397,133 +397,55 @@ class CliDashboard(AbstractDashboard):
         if layout.job_board_show_log:
             table.add_column("Log")
 
-        table_data = []
+        # jobname, node index, priority, node order
+        table_data_select = []
+        for chip, job in job_data.items():
+            for n, node in enumerate(job.nodes):
+                table_data_select.append(
+                    (job.jobname, n, node["print"]["priority"], node["print"]["order"], chip)
+                )
 
-        for jobname, job in job_data.items():
-            if not job.nodes:
-                continue
+        # sort for priority
+        table_data_select = sorted(table_data_select, key=lambda d: (d[2], *d[3], d[0]))
 
-            for node in job.nodes:
-                if (
-                    layout.job_board_show_log
-                    and os.path.exists(node["log"])
-                ):
-                    log_file = "[bright_black]{}[/]".format(node['log'])
-                else:
-                    log_file = None
+        # trim to size
+        table_data_select = table_data_select[0:layout.job_board_height]
 
-                if node["time"]["duration"] is not None:
-                    duration = f'{node["time"]["duration"]:.1f}s'
-                elif node["time"]["start"] is not None:
-                    duration = f'{time.time() - node["time"]["start"]:.1f}s'
-                else:
-                    duration = ""
+        # sort for printing order
+        table_data_select = sorted(table_data_select, key=lambda d: (*d[3], d[2], d[0]))
 
-                table_data.append((node["status"], node["step"], node["index"], (
-                    CliDashboard.format_status(node["status"]),
-                    CliDashboard.format_node(
-                        job.design, job.jobname, node["step"], node["index"]
-                    ),
-                    duration,
-                    *node["metrics"],
-                    log_file
-                )))
-
-        def job_data_fits(table_data):
-            return len(table_data) <= layout.job_board_height
-
-        def remove_pending(table_data, keep_nodes):
-            if not keep_nodes:
-                keep_nodes = set()
-            for n in range(len(table_data)):
-                if NodeStatus.is_waiting(table_data[-n][0]):
-                    if (table_data[-n][1], table_data[-n][2]) not in keep_nodes:
-                        table_data.pop(-n)
-                        return table_data
-            return table_data
-
-        def remove_success(table_data, keep_nodes):
-            if not keep_nodes:
-                keep_nodes = set()
-            for n in range(len(table_data)):
-                if NodeStatus.is_success(table_data[n][0]):
-                    if (table_data[n][1], table_data[n][2]) not in keep_nodes:
-                        table_data.pop(n)
-                        return table_data
-            return table_data
-
-        def count_table_data(table_data):
-            pending = 0
-            done = 0
-            for status, _, _, _ in table_data:
-                if NodeStatus.is_done(status):
-                    done += 1
-                elif NodeStatus.is_waiting(status):
-                    pending += 1
-            return pending, done
-
-        running_nodes = set([
-            (step, index) for status, step, index, _ in table_data
-            if NodeStatus.is_running(status)])
-
-        done_nodes = set([
-            (step, index) for status, step, index, _ in table_data
-            if NodeStatus.is_done(status)])
-
-        flow = self._chip.get('option', 'flow')
-        entry_nodes = set([node for node in _get_flowgraph_entry_nodes(self._chip, flow)
-                           if node not in running_nodes and node not in done_nodes])
-
-        input_nodes = set()
-        for step, index in running_nodes:
-            input_nodes.update(_get_flowgraph_node_inputs(self._chip, flow, (step, index)))
-        input_nodes.update(entry_nodes)
-
-        output_nodes = set()
-        for step, index in nodes_to_execute(self._chip):
-            for in_node in self._chip.get('flowgraph', flow, step, index, 'input'):
-                if in_node in running_nodes:
-                    output_nodes.add((step, index))
-        output_nodes.update(entry_nodes)
-
-        # order:
-        # loop:
-        #     remove pending, 1+ removed from running
-        #     remove success, 1+ removed from running
-        # loop:
-        #     remove pending, any
-        #     remove success, any
-        # trim running to fit
-
-        while True:
-            if job_data_fits(table_data):
-                break
-            start_len = len(table_data)
-            pending, done = count_table_data(table_data)
-            if pending >= done:
-                table_data = remove_pending(table_data, output_nodes)
-            else:
-                table_data = remove_success(table_data, input_nodes)
-            if len(table_data) == start_len:
-                break
-
-        while True:
-            if job_data_fits(table_data):
-                break
-            start_len = len(table_data)
-            pending, done = count_table_data(table_data)
-            if pending >= done:
-                table_data = remove_pending(table_data, None)
-            else:
-                table_data = remove_success(table_data, None)
-            if len(table_data) == start_len:
-                break
+        table_data = []
+
+        for _, node_idx, _, _, chip in table_data_select:
+            job = job_data[chip]
+            node = job.nodes[node_idx]
 
-        if not job_data_fits(table_data):
-            # trim to fit
-            table_data = table_data[0:layout.job_board_height]
+            if (
+                layout.job_board_show_log
+                and os.path.exists(node["log"])
+            ):
+                log_file = "[bright_black]{}[/]".format(node['log'])
+            else:
+                log_file = None
 
-        for _, _, _, row_data in table_data:
+            if node["time"]["duration"] is not None:
+                duration = f'{node["time"]["duration"]:.1f}s'
+            elif node["time"]["start"] is not None:
+                duration = f'{time.time() - node["time"]["start"]:.1f}s'
+            else:
+                duration = ""
+
+            table_data.append((
+                CliDashboard.format_status(node["status"]),
+                CliDashboard.format_node(
+                    job.design, job.jobname, node["step"], node["index"]
+                ),
+                duration,
+                *node["metrics"],
+                log_file
+            ))
+
+        for row_data in table_data:
             table.add_row(*row_data)
 
         if table.row_count == 0:
@@ -675,7 +597,7 @@ class CliDashboard(AbstractDashboard):
         This data is used to populate the dashboard.
         """
 
-        job_data = self._get_job(starttimes=starttimes)
+        job_data = self._get_job(chip=self._chip, starttimes=starttimes)
 
         with self._render_data_lock:
             self._render_data.jobs[self._chip] = job_data
@@ -705,16 +627,80 @@ class CliDashboard(AbstractDashboard):
             starttimes = {}
 
         nodes = []
+        nodestatus = {}
+        nodeorder = {}
+        node_priority = {}
         try:
+            node_inputs = {}
+            node_outputs = {}
             flow = chip.get("option", "flow")
             if not flow:
                 raise SiliconCompilerError("dummy error")
             execnodes = nodes_to_execute(chip)
-            for nodeset in _get_flowgraph_execution_order(chip, flow):
-                for node in nodeset:
+            for n, nodeset in enumerate(_get_flowgraph_execution_order(chip, flow)):
+                for m, node in enumerate(nodeset):
                     if node not in execnodes:
                         continue
                     nodes.append(node)
+
+                    node_priority[node] = 0
+
+                    status = chip.get("record", "status", step=node[0], index=node[1])
+                    if status is None:
+                        status = NodeStatus.PENDING
+                    nodestatus[node] = status
+                    nodeorder[node] = (n, m)
+
+                    node_inputs[node] = _get_flowgraph_node_inputs(chip, flow, node)
+                    for in_node in chip.get('flowgraph', flow, node[0], node[1], 'input'):
+                        node_outputs.setdefault(in_node, set()).add(node)
+
+            flow_entry_nodes = set(_get_flowgraph_entry_nodes(chip, flow))
+
+            running_nodes = set([node for node in nodes if NodeStatus.is_running(nodestatus[node])])
+            done_nodes = set([node for node in nodes if NodeStatus.is_done(nodestatus[node])])
+            error_nodes = set([node for node in nodes if NodeStatus.is_error(nodestatus[node])])
+
+            def get_node_distance(node, search, level=1):
+                dists = {}
+
+                if node not in search:
+                    return dists
+
+                for snode in search[node]:
+                    dists[snode] = level
+                    dists.update(get_node_distance(snode, search, level=level+1))
+
+                return dists
+
+            # Compute relative node distances
+            node_dists = {}
+            for cnode in nodes:
+                # use 2x + 1 to give completed nodes sorting priority
+                node_dists[cnode] = {
+                    node: 2*level+1 for node, level in get_node_distance(cnode, node_inputs).items()
+                }
+                node_dists[cnode].update({
+                    node: 2*level for node, level in get_node_distance(cnode, node_outputs).items()
+                })
+
+            # Compute printing priority of nodes
+            remaining_entry_nodes = flow_entry_nodes - done_nodes
+            startnodes = running_nodes.union(remaining_entry_nodes)
+            priority_node = {0: startnodes.union(error_nodes)}
+            for node in nodes:
+                dists = node_dists[node]
+                levels = []
+                for snode in startnodes:
+                    if snode not in dists:
+                        continue
+                    levels.append(dists[snode])
+                if not levels:
+                    continue
+                priority_node.setdefault(min(levels), set()).add(node)
+            for level, level_nodes in priority_node.items():
+                for node in level_nodes:
+                    node_priority[node] = level
         except SiliconCompilerError:
             pass
 
@@ -725,7 +711,7 @@ class CliDashboard(AbstractDashboard):
         job_data.jobname = jobname
         job_data.design = design
         totaltimes = [
-            self._chip.get("metric", "totaltime", step=step, index=index) or 0
+            chip.get("metric", "totaltime", step=step, index=index) or 0
             for step, index in nodes
         ]
         if not totaltimes:
@@ -733,9 +719,7 @@ class CliDashboard(AbstractDashboard):
         job_data.runtime = max(totaltimes)
 
         for step, index in nodes:
-            status = self._chip.get("record", "status", step=step, index=index)
-            if not status:
-                status = NodeStatus.PENDING
+            status = nodestatus[(step, index)]
 
             job_data.total += 1
             if NodeStatus.is_error(status):
@@ -752,14 +736,13 @@ class CliDashboard(AbstractDashboard):
             starttime = None
             duration = None
             if NodeStatus.is_done(status):
-                duration = self._chip.get("metric", "tasktime", step=step, index=index)
+                duration = chip.get("metric", "tasktime", step=step, index=index)
             if (step, index) in starttimes:
                 starttime = starttimes[(step, index)]
 
             node_metrics = []
             for metric in self._metrics:
-                value = self._chip.get('metric', metric,
-                                       step=step, index=index)
+                value = chip.get('metric', metric, step=step, index=index)
                 if value is None:
                     node_metrics.append("")
                 else:
@@ -777,11 +760,15 @@ class CliDashboard(AbstractDashboard):
                     "metrics": node_metrics,
                     "log": os.path.join(
                         os.path.relpath(
-                            self._chip.getworkdir(step=step, index=index),
-                            self._chip.cwd,
+                            chip.getworkdir(step=step, index=index),
+                            chip.cwd,
                         ),
                         f"{step}.log",
                     ),
+                    "print": {
+                        "order": nodeorder[(step, index)],
+                        "priority": node_priority[(step, index)]
+                    }
                 }
             )
 


Test patch:
diff --git a/tests/core/test_cli_dashboard.py b/tests/core/test_cli_dashboard.py
index d4004edfa..3a04cc228 100644
--- a/tests/core/test_cli_dashboard.py
+++ b/tests/core/test_cli_dashboard.py
@@ -44,6 +44,10 @@ def mock_running_job_lg():
             "time": {
                 "duration": None,
                 "start": None
+            },
+            "print": {
+                "order": (index, index),
+                "priority": 0 if statuses[index % len(statuses)] == NodeStatus.ERROR else index
             }
         }
         for index in range(mock_job_data.total)
@@ -70,6 +74,10 @@ def mock_running_job():
             "status": random.choice(statuses),
             "metrics": ["", ""],
             "log": f"node{index + 1}.log",
+            "print": {
+                "order": (index, index),
+                "priority": 0 if statuses[index % len(statuses)] == NodeStatus.ERROR else index
+            }
         }
         for index in range(mock_job_data.total)
     ]
@@ -98,6 +106,10 @@ def mock_finished_job_fail():
             "time": {
                 "duration": 5.0,
                 "start": None
+            },
+            "print": {
+                "order": (index, index),
+                "priority": 0 if statuses[index % len(statuses)] == NodeStatus.ERROR else index
             }
         }
         for index in range(mock_job_data.total)
@@ -126,6 +138,10 @@ def mock_finished_job_passed():
             "time": {
                 "duration": 5.0,
                 "start": None
+            },
+            "print": {
+                "order": (index, index),
+                "priority": index
             }
         }
         for index in range(mock_job_data.total)
@@ -501,24 +517,24 @@ def test_render_job_dashboard(mock_running_job_lg, dashboard_medium):
         assert len(actual_lines) == 19
 
         expected_lines = [
+            expected_lines_all[0],
             expected_lines_all[1],
             expected_lines_all[2],
+            expected_lines_all[3],
             expected_lines_all[4],
             expected_lines_all[5],
+            expected_lines_all[6],
             expected_lines_all[7],
             expected_lines_all[8],
+            expected_lines_all[9],
             expected_lines_all[10],
             expected_lines_all[11],
+            expected_lines_all[12],
             expected_lines_all[13],
-            expected_lines_all[14],
             expected_lines_all[16],
-            expected_lines_all[17],
             expected_lines_all[19],
-            expected_lines_all[20],
             expected_lines_all[22],
-            expected_lines_all[23],
             expected_lines_all[25],
-            expected_lines_all[26],
             expected_lines_all[28]
         ]
         assert len(actual_lines) == len(expected_lines)