Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DM-34811 Fix component linkages in pipeline dot render #185

Merged
merged 4 commits into from
May 25, 2022
Merged

DM-34811 Fix component linkages in pipeline dot render #185

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8f327a2
Fix component linkages in pipeline dot render
natelust May 18, 2022
bb192e7
Add support for metadata connections
natelust May 19, 2022
d7efc10
Sort inputs to dot output
natelust May 20, 2022
475179c
Minor fixes
natelust May 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/changes/DM-34811.bugfix.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed a bug where dot graphs of pipelines did not correctly render edges between composite and component dataset types.
8 changes: 7 additions & 1 deletion python/lsst/ctrl/mpexec/dotTools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
# -----------------------------
# Imports for other modules --
# -----------------------------
from lsst.daf.butler import DimensionUniverse
from lsst.daf.butler import DatasetType, DimensionUniverse
from lsst.pipe.base import Pipeline, iterConnections

# ----------------------------------
Expand Down Expand Up @@ -244,7 +244,13 @@ def expand_dimensions(dimensions):
dimensions = expand_dimensions(attr.dimensions)
_renderDSTypeNode(attr.name, dimensions, file)
allDatasets.add(attr.name)
nodeName, component = DatasetType.splitDatasetTypeName(attr.name)
_renderEdge(attr.name, taskNodeName, file)
# connect component dataset types to the composite type that
# produced it
if component is not None and (nodeName, attr.name) not in allDatasets:
_renderEdge(nodeName, attr.name, file)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this also need:

if nodeName not in allDatasets:
    _renderDSTypeNode(nodeName, ...)

(in case a component is a pre-existing input and not an intermediate)?

It may be a nicer representation if a component could be rendered as a box inside a dataset type node (e.g. using record-based graphviz nodes), but that is probably out of scope for this ticket.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what you mean with your first point, the rendering of the DSType node is handled above, and prerequisites are handled in a separate loop.

I thought about your latter point, but decided it was not worth re-working so much for a quicker fix that seems to do its job well enough.

Copy link
Collaborator

@andy-slac andy-slac May 18, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the code above renders a node for attr.name, but here you add an edge from nodeName to attr.name. If there is no node rendered for nodeName it will be shown using default node representation without any dataset-related info (simple box with a name inside).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yeah, I see what you are saying, good point, ill fix that up

allDatasets.add((nodeName, attr.name))

for attr in iterConnections(taskDef.connections, "prerequisiteInputs"):
if attr.name not in allDatasets:
Expand Down
17 changes: 13 additions & 4 deletions tests/test_dotTools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,17 +114,23 @@ class DotToolsTestCase(unittest.TestCase):
def testPipeline2dot(self):
"""Tests for dotTools.pipeline2dot method"""
pipeline = _makePipeline(
[("A", ("B", "C"), "task1"), ("C", "E", "task2"), ("B", "D", "task3"), (("D", "E"), "F", "task4")]
[
("A", ("B", "C"), "task1"),
("C", "E", "task2"),
("B", "D", "task3"),
(("D", "E"), "F", "task4"),
("D.C", "G", "task5"),
]
)
file = io.StringIO()
pipeline2dot(pipeline, file)

# It's hard to validate complete output, just checking few basic
# things, even that is not terribly stable.
lines = file.getvalue().strip().split("\n")
ndatasets = 6
ntasks = 4
nedges = 10
ndatasets = 8
ntasks = 5
nedges = 13
nextra = 2 # graph header and closing
self.assertEqual(len(lines), ndatasets + ntasks + nedges + nextra)

Expand All @@ -144,6 +150,9 @@ def testPipeline2dot(self):
self.assertEqual(node[0] + node[-1], '""')
continue

# make sure components are connected appropriately
self.assertIn('"D" -> "D.C";', file.getvalue())


class MyMemoryTestCase(lsst.utils.tests.MemoryTestCase):
pass
Expand Down