Skip to content

Commit

Permalink
Fix bug in verdi process show where not all called processes are shown
Browse files Browse the repository at this point in the history
The utility `get_node_info` was called by `verdi process show` to format
a multiline string of the given process node, including all the inputs
and called processes. The function used the `LinkManager.nested` to
reconstruct a nested representation of the relevant set of neighboring
nodes. This is useful for the inputs and outputs that can in fact be
nested and have unique labels, however, for called links, the link
labels are not necessarily unique and so the dictionary returned by the
nested method could only represent a single called process with the same
link label. To fix this we use a different formatter that simply takes
the list of neighboring nodes, which then does not care about label
uniqueness.

Since the `nested` method was silently overwriting duplicate keys, which
caused this bug, its behavior is adapted to raise a `KeyError` in this
situation.
  • Loading branch information
sphuber committed Jun 4, 2019
1 parent 4cf8fa9 commit 5ba688a
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 7 deletions.
31 changes: 26 additions & 5 deletions aiida/backends/tests/cmdline/utils/test_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@
from __future__ import print_function
from __future__ import absolute_import

from aiida import orm
from aiida.backends.testbase import AiidaTestCase
from aiida.common.links import LinkType
from aiida.orm import Code
from aiida.orm import CalculationNode, CalcFunctionNode


class TestCommonUtilities(AiidaTestCase):
Expand All @@ -27,13 +26,13 @@ def test_get_node_summary(self):

computer_label = self.computer.name # pylint: disable=no-member

code = Code(
code = orm.Code(
input_plugin_name='arithmetic.add',
remote_computer_exec=[self.computer, '/remote/abs/path'],
)
code.store()

node = CalculationNode()
node = orm.CalculationNode()
node.computer = self.computer
node.add_incoming(code, link_type=LinkType.INPUT_CALC, link_label='code')
node.store()
Expand All @@ -42,13 +41,35 @@ def test_get_node_summary(self):
self.assertIn(node.uuid, summary)
self.assertIn(computer_label, summary)

def test_get_node_info_multiple_call_links(self):
"""Test the `get_node_info` utility.
Regression test for #2868:
Verify that all `CALL` links are included in the formatted string even if link labels are identical.
"""
from aiida.cmdline.utils.common import get_node_info

workflow = orm.WorkflowNode().store()
node_one = orm.CalculationNode()
node_two = orm.CalculationNode()

node_one.add_incoming(workflow, link_type=LinkType.CALL_CALC, link_label='CALL_IDENTICAL')
node_two.add_incoming(workflow, link_type=LinkType.CALL_CALC, link_label='CALL_IDENTICAL')
node_one.store()
node_two.store()

node_info = get_node_info(workflow)
self.assertTrue('CALL_IDENTICAL' in node_info)
self.assertTrue(str(node_one.pk) in node_info)
self.assertTrue(str(node_two.pk) in node_info)

def test_get_process_function_report(self):
"""Test the `get_process_function_report` utility."""
from aiida.cmdline.utils.common import get_process_function_report

warning = 'You have been warned'

node = CalcFunctionNode()
node = orm.CalcFunctionNode()
node.store()

# Add a log message through the logger
Expand Down
19 changes: 18 additions & 1 deletion aiida/cmdline/utils/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ def get_node_info(node, include_summary=True):
result += '\n' + format_nested_links(nodes_output.nested(), headers=['Outputs', 'PK', 'Type'])

if nodes_called:
result += '\n' + format_nested_links(nodes_called.nested(), headers=['Called', 'PK', 'Type'])
result += '\n' + format_flat_links(nodes_called.all(), headers=['Called', 'PK', 'Type'])

log_messages = orm.Log.objects.get_logs_for(node)

Expand All @@ -169,6 +169,23 @@ def get_node_info(node, include_summary=True):
return result


def format_flat_links(links, headers):
"""Given a flat list of LinkTriples, return a flat string representation.
:param links: a list of LinkTriples
:param headers: headers to use
:return: formatted string
"""
table = []

for link_triple in links:
table.append([link_triple.link_label, link_triple.node.pk, link_triple.node.__class__.__name__])

result = '\n{}'.format(tabulate(table, headers=headers))

return result


def format_nested_links(links, headers):
"""Given a nested dictionary of nodes, return a nested string representation.
Expand Down
10 changes: 9 additions & 1 deletion aiida/orm/utils/links.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,9 +278,14 @@ def get_node_by_label(self, label):
return matching_entry

def nested(self, sort=True):
"""Return the matched nodes in the original nested form by expanding the collapsed link labels.
"""Construct (nested) dictionary of matched nodes that mirrors the original nesting of link namespaces.
Process input and output namespaces can be nested, however the link labels that represent them in the database
have a flat hierarchy, and so the link labels are flattened representations of the nested namespaces.
This function reconstructs the original node nesting based on the flattened links.
:return: dictionary of nested namespaces
:raises KeyError: if there are duplicate link labels in a namespace
"""
from aiida.engine.processes.ports import PORT_NAMESPACE_SEPARATOR

Expand All @@ -300,6 +305,9 @@ def nested(self, sort=True):
current_namespace = current_namespace.setdefault(subspace, {})

# Insert the node at the given port name
if port_name in current_namespace:
raise KeyError("duplicate label '{}' in namespace '{}'".format(port_name, '.'.join(port_namespaces)))

current_namespace[port_name] = entry.node

if sort:
Expand Down

0 comments on commit 5ba688a

Please sign in to comment.