Skip to content

Commit

Permalink
Improve status panel behavior (#1166)
Browse files Browse the repository at this point in the history
This PR updates the status panel as follows:
- The root node of the process tree is force selected to avoid inconsistent selection on process node loading
- `metadata_inputs` keys not found in root workflow properties are ignored
- Dynamic marking (*) propagates upwards (legend updated to better reflect meaning)
  • Loading branch information
edan-bainglass authored Feb 18, 2025
1 parent b2a7210 commit 511b3a0
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 75 deletions.
10 changes: 7 additions & 3 deletions src/aiidalab_qe/app/result/components/status/status.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,11 @@ def _render(self):

self.children = [self.accordion]

def _post_render(self):
self._select_tree_root()

def _on_monitor_counter_change(self, _):
self._update_process_tree()
self.process_tree.update()

def _on_accordion_change(self, change):
if change["new"] == 0:
Expand All @@ -125,9 +128,10 @@ def _on_calculation_link_click(self, change):
def _switch_to_advanced_view(self, _):
self.accordion.selected_index = 1

def _update_process_tree(self):
def _select_tree_root(self):
if self.rendered:
self.process_tree.update()
self.process_tree.value = None
self.process_tree.value = self._model.process_uuid

def _reset_process_tree(self, _):
if not self.rendered:
Expand Down
54 changes: 29 additions & 25 deletions src/aiidalab_qe/app/result/components/status/tree.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import typing as t
from copy import deepcopy

import ipywidgets as ipw
import traitlets as tl
Expand Down Expand Up @@ -51,7 +52,7 @@ def _on_process_change(self, _):
def _on_monitor_counter_change(self, _):
self._update()

def _on_inspect(self, uuid):
def _on_inspect(self, uuid: str):
self._model.clicked = None # ensure event is triggered when label is reclicked
self._model.clicked = uuid

Expand Down Expand Up @@ -79,28 +80,19 @@ def _render(self):
self.trunk,
ipw.HBox(
children=[
ipw.HTML(
value="*",
layout=ipw.Layout(margin="0"),
),
ipw.HTML(
value="""
<div style="font-style: italic;">
workflow will automatically re-submit failed calculations
<br>
<b>(advanced users)</b> click
<a
href="https://aiida.readthedocs.io/projects/aiida-core/en/stable/howto/workchains_restart.html"
target="_blank"
>here</a> to learn how AiiDA handles errors
</div>
""",
layout=ipw.Layout(margin="0"),
<div style="font-style: italic;">
*Actual number of jobs may exceed estimated total due to
error handling, dynamic sub-workflows, and/or other runtime
adjustments
</div>
""",
),
],
layout=ipw.Layout(
align_items="flex-start",
margin="5px 0 0 0",
margin="5px 10px 0 0",
),
),
]
Expand All @@ -122,7 +114,7 @@ class ProcessTreeNode(ipw.VBox, t.Generic[ProcessNodeType]):
"BandsWorkChain": "Electronic band structure workflow",
"PwBandsWorkChain": "Electronic band structure workflow",
"ProjwfcBandsWorkChain": "Electronic band structure workflow",
"PwRelaxWorkChain": "Structure relaxation workflow",
"PwRelaxWorkChain": "Structure relaxation workflow manager",
"PdosWorkChain": "Projected density of states workflow",
"PwBaseWorkChain": {
"scf": "SCF workflow",
Expand Down Expand Up @@ -234,7 +226,19 @@ def __iadd__(self, other: ProcessTreeNode):
class WorkChainTreeNode(ProcessTreeNode[orm.WorkChainNode]):
@property
def metadata_inputs(self):
return self.node.get_metadata_inputs()
# BACKWARDS COMPATIBILITY: originally this was added because "relax" was in the
# metadata inputs regardless if running relaxation, as it was used also for the
# summary to extract pw parameters. #1163 removes this dependency, thus allowing
# the popping of the "relax" port if not running relaxation. However, this is
# kept for backwards compatibility, for jobs ran before #1163.
inputs = deepcopy(self.node.get_metadata_inputs()) or {}
if "properties" in self.node.inputs:
inputs = {
key: value
for key, value in inputs.items()
if key in self.node.inputs.properties
}
return inputs

@property
def sub_processes(self):
Expand Down Expand Up @@ -273,7 +277,7 @@ def expand(self, recursive=False):
if isinstance(branch, WorkChainTreeNode):
branch.expand(recursive=True)

def collapse(self, recursive=False):
def collapse(self, recursive: bool = False):
if not self.collapsed:
self.toggle.click()
if recursive:
Expand All @@ -299,7 +303,7 @@ def _build_header(self):
self.tally,
]

def _add_branches(self, node=None):
def _add_branches(self, node: orm.ProcessNode | None = None):
node = node or self.node
for child in sorted(node.called, key=lambda child: child.ctime):
if child.pk in self.pks or isinstance(child, orm.CalcFunctionNode):
Expand All @@ -316,7 +320,6 @@ def _add_branches(self, node=None):
)
if child.process_label in (
"BandsWorkChain",
"PwRelaxWorkChain",
"ProjwfcBaseWorkChain",
):
self._add_branches(child)
Expand All @@ -334,7 +337,7 @@ def _get_tally(self):
tally += " job" if total == 1 else " jobs"
return tally

def _get_current_total(self, node):
def _get_current_total(self, node: orm.ProcessNode):
total = 0
for child in node.called:
if isinstance(child, orm.WorkChainNode):
Expand All @@ -343,7 +346,7 @@ def _get_current_total(self, node):
total += 1
return total

def _count_finished(self, node):
def _count_finished(self, node: orm.ProcessNode):
count = 0
for child in node.called:
if isinstance(child, orm.WorkChainNode):
Expand All @@ -361,11 +364,12 @@ def _get_expected(self, inputs: dict[str, dict]) -> dict:
if "metadata" in sub_inputs and "options" in sub_inputs["metadata"]:
# This is a calculation
count += 1
dynamic = key == "pw"
dynamic = dynamic or key == "pw"
elif key != "metadata":
# This is a workflow
nested = self._get_expected(sub_inputs)
count += nested["count"]
dynamic = dynamic or nested["dynamic"]
expected[key] = nested

expected |= {
Expand Down
119 changes: 72 additions & 47 deletions tests/test_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,18 +97,25 @@ def mock_qeapp_workchain():
yield qe_workchain


class TestSimplifiedProcessTree:
model: SimplifiedProcessTreeModel
node: orm.WorkChainNode
class TreeTestingMixin:
tree: SimplifiedProcessTree

@property
def workchain_node(self) -> WorkChainTreeNode:
def relax_workchain_node(self) -> WorkChainTreeNode:
return self.tree.trunk.branches[0] # type: ignore

@property
def calculation_node(self) -> CalcJobTreeNode:
return self.workchain_node.branches[0] # type: ignore
def base_workchain_node(self) -> WorkChainTreeNode:
return self.relax_workchain_node.branches[0] # type: ignore

@property
def calcjob_node(self) -> CalcJobTreeNode:
return self.base_workchain_node.branches[0] # type: ignore


class TestSimplifiedProcessTree(TreeTestingMixin):
model: SimplifiedProcessTreeModel
node: orm.WorkChainNode

@pytest.fixture(scope="class", autouse=True)
def setup(self, request):
Expand Down Expand Up @@ -140,58 +147,77 @@ def test_update_on_process_change(self, mock_qeapp_workchain):
assert not self.tree.trunk.collapsed
assert len(self.tree.trunk.branches) == 1

def test_workchain_node(self):
workchain_node = self.workchain_node
assert isinstance(workchain_node, WorkChainTreeNode)
assert workchain_node.level == 1
assert workchain_node.emoji.value == "✅"
assert workchain_node.state.value == "finished"
human_label = ProcessTreeNode._TITLE_MAPPING["PwBaseWorkChain"]["relax"]
assert workchain_node.label.value == human_label
assert isinstance(workchain_node.label, ipw.HTML)
assert workchain_node.collapsed
assert len(workchain_node.branches) == 0
def test_relax_workchain_manager_node(self):
assert isinstance(self.relax_workchain_node, WorkChainTreeNode)
assert self.relax_workchain_node.level == 1
assert self.relax_workchain_node.emoji.value == "✅"
assert self.relax_workchain_node.state.value == "finished"
human_label = ProcessTreeNode._TITLE_MAPPING["PwRelaxWorkChain"]
assert self.relax_workchain_node.label.value == human_label
assert isinstance(self.relax_workchain_node.label, ipw.HTML)
assert self.relax_workchain_node.collapsed
assert len(self.relax_workchain_node.branches) == 0

def test_expand(self):
workchain_node = self.workchain_node
workchain_node.expand()
assert "open" in workchain_node.branches._dom_classes
assert workchain_node.toggle.icon == "minus"
self.relax_workchain_node.expand()
assert "open" in self.relax_workchain_node.branches._dom_classes
assert self.relax_workchain_node.toggle.icon == "minus"
assert len(self.relax_workchain_node.branches) == 1

def test_collapse(self):
workchain_node = self.workchain_node
workchain_node.collapse()
assert "open" not in workchain_node.branches._dom_classes
assert workchain_node.toggle.icon == "plus"
self.relax_workchain_node.collapse()
assert "open" not in self.relax_workchain_node.branches._dom_classes
assert self.relax_workchain_node.toggle.icon == "plus"

def test_expand_recursive(self):
self.tree.trunk.expand(recursive=True)
assert not self.tree.trunk.collapsed
assert not self.workchain_node.collapsed
assert all(
not branch.collapsed
for branch in (
self.tree.trunk,
self.relax_workchain_node,
self.base_workchain_node,
)
)

def test_workchain_node(self):
assert isinstance(self.base_workchain_node, WorkChainTreeNode)
assert self.base_workchain_node.level == 2
assert self.base_workchain_node.emoji.value == "✅"
assert self.base_workchain_node.state.value == "finished"
human_label = ProcessTreeNode._TITLE_MAPPING["PwBaseWorkChain"]["relax"]
assert self.base_workchain_node.label.value == human_label
assert isinstance(self.base_workchain_node.label, ipw.HTML)
assert len(self.base_workchain_node.branches) == 1

def test_collapse_recursive(self):
self.tree.trunk.collapse(recursive=True)
assert self.tree.trunk.collapsed
assert self.workchain_node.collapsed
self.relax_workchain_node.collapse(recursive=True)
assert self.relax_workchain_node.collapsed
assert self.base_workchain_node.collapsed

def test_collapse_all_button(self):
self.tree.trunk.expand(recursive=True)
self.tree.collapse_button.click()
assert self.tree.trunk.collapsed
assert self.workchain_node.collapsed

def test_calculation_node(self):
calculation_node = self.calculation_node
assert isinstance(calculation_node, CalcJobTreeNode)
assert calculation_node.level == 2
assert calculation_node.emoji.value == "✅"
assert calculation_node.state.value == "finished"
assert all(
branch.collapsed
for branch in (
self.tree.trunk,
self.relax_workchain_node,
self.base_workchain_node,
)
)

def test_calcjob_node(self):
assert isinstance(self.calcjob_node, CalcJobTreeNode)
assert self.calcjob_node.level == 3
assert self.calcjob_node.emoji.value == "✅"
assert self.calcjob_node.state.value == "finished"
human_label = ProcessTreeNode._TITLE_MAPPING["PwCalculation"]["relax"]
assert isinstance(calculation_node.label, ipw.Button)
assert calculation_node.label.description == human_label
assert isinstance(self.calcjob_node.label, ipw.Button)
assert self.calcjob_node.label.description == human_label


class TestWorkChainStatusPanel:
class TestWorkChainStatusPanel(TreeTestingMixin):
model: WorkChainStatusModel
panel: WorkChainStatusPanel

Expand All @@ -218,13 +244,12 @@ def test_calcjob_node_link(self):

trunk = self.tree.trunk
trunk.expand(recursive=True)
calcjob_node: CalcJobTreeNode = trunk.branches[0].branches[0]
calcjob_node.label.click()
assert self.panel.process_tree.value == calcjob_node.uuid
assert self.panel.process_tree._tree.nodes == (calcjob_node.node,)
self.calcjob_node.label.click()
assert self.panel.process_tree.value == self.calcjob_node.uuid
assert self.panel.process_tree._tree.nodes == (self.calcjob_node.node,)
# TODO understand why the following does not trigger automatically as in the app
# TODO understand why the following triggers a thread
# self.panel.process_tree.set_trait("selected_nodes", [calcjob_node.node])
# self.panel.process_tree.set_trait("selected_nodes", [self.calcjob_node.node])
# assert isinstance(self.panel.node_view, CalcJobNodeViewerWidget)
# assert self.panel.node_view_container.children[0] is self.node_view # type: ignore

Expand Down

0 comments on commit 511b3a0

Please sign in to comment.