[Feature]: Flat Converter from ewoks execution graph to ELK layout graph #17#18
Conversation
|
Running the example looks like that : pixi run python examples/convert_ewoks_to_elk_graph.pyEwoks Graph: {'task1': {'default_inputs': [{'name': 'a', 'value': 1}],
'task_identifier': 'ewokscore.tests.examples.tasks.condsumtask.CondSumTask',
'task_type': 'class'},
'task2': {'default_inputs': [{'name': 'b', 'value': 1}],
'task_identifier': 'ewokscore.tests.examples.tasks.condsumtask.CondSumTask',
'task_type': 'class'},
'task3': {'default_inputs': [{'name': 'b', 'value': 1}],
'task_identifier': 'ewokscore.tests.examples.tasks.condsumtask.CondSumTask',
'task_type': 'class'}}
[('task1',
'task2',
{'conditions': [{'source_output': 'too_small', 'value': True}],
'data_mapping': [{'source_output': 'result', 'target_input': 'a'}]}),
('task2',
'task3',
{'conditions': [{'source_output': 'too_small', 'value': True}],
'data_mapping': [{'source_output': 'result', 'target_input': 'a'}]}),
('task3',
'task1',
{'conditions': [{'source_output': 'too_small', 'value': True}],
'data_mapping': [{'source_output': 'result', 'target_input': 'a'}]})]TaskSizes data structure: {'task1': (54.232, 63.0), 'task2': (54.232, 63.0), 'task3': (54.232, 63.0)}ELK Graph: {'children': [{'height': 63.0, 'id': 'task1', 'width': 54.232},
{'height': 63.0, 'id': 'task2', 'width': 54.232},
{'height': 63.0, 'id': 'task3', 'width': 54.232}],
'edges': [{'id': 'edge_task1_task2_0',
'sources': ['task1'],
'targets': ['task2']},
{'id': 'edge_task2_task3_0',
'sources': ['task2'],
'targets': ['task3']},
{'id': 'edge_task3_task1_0',
'sources': ['task3'],
'targets': ['task1']}],
'id': 'root',
'layoutOptions': {'elk.layered.edgeRouting.splines.mode': 'CONSERVATIVE_SOFT',
'org.eclipse.elk.algorithm': 'layered',
'org.eclipse.elk.direction': 'RIGHT',
'org.eclipse.elk.edgeRouting': 'SPLINES',
'org.eclipse.elk.layered.spacing.nodeNodeBetweenLayers': 80,
'org.eclipse.elk.spacing.edgeEdge': 20,
'org.eclipse.elk.spacing.nodeNode': 60}} |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
loichuder
left a comment
There was a problem hiding this comment.
First review round.
I don't understand how the introduction of SvgTaskGroup fits in the PR. Is it needed for the Elk layouting?
| ELK_SPACING_TASKS = 60 | ||
| ELK_SPACING_TASK_LAYERS = 80 | ||
| ELK_SPACING_LINK = 20 | ||
| ELK_ROUTING_MODE = "CONSERVATIVE_SOFT" |
There was a problem hiding this comment.
Is there a usecase to access each constant individually?
If not, we could only have ELK_LAYOUT_OPTIONS as a constant.
| @@ -0,0 +1 @@ | |||
| from .elk_converter import convert_ewoks_to_elk_graph # noqa: F401 | |||
| :returns: an ELK graph, i.e. | ||
| ``{"id": str, "layoutOptions": dict, | ||
| "children": [{"id", "width", "height"}, ...], | ||
| "edges": [{"id", "sources": [str], "targets": [str]}, ...]}``. |
There was a problem hiding this comment.
This would be way better if this was encoded in the type ElkGraph
|
|
||
| edges = [] | ||
| for source, target, link_attrs in ewoks_graph.graph.edges(data=True): | ||
| n_mappings = len(link_attrs.get("data_mapping") or []) or 1 |
There was a problem hiding this comment.
| n_mappings = len(link_attrs.get("data_mapping") or []) or 1 | |
| n_mappings = len(link_attrs.get("data_mapping", [])) or 1 |
| from .svg_group import SvgGroup | ||
| from .svg_task import SvgTask | ||
|
|
||
| TaskSizes = dict[str, tuple[float, float]] |
There was a problem hiding this comment.
It would be best to have a namedtuple instead of tuple[float, float] since it is very easy to confuse width and height if they are only addressed by their index.
Yes for the routing with ELK I need to have the sizes of eachs tasks box [{task_id:sizes},...]. |
Tip
Description
Implements
convert_ewoks_to_elk_graph, converting a flatewoksTaskGraphinto anELKJSON layout graph (childrenwithwidth/height,edgeswithsources/targets, andlayoutOptionssourced fromconstants.py).Closes #17
Note
Changes Made
layout/elk_converter.py:convert_ewoks_to_elk_graph(ewoks_graph, task_sizes) -> ElkGraph.svg/svg_task_group.py: newSvgTaskGroup(SvgGroup)— SvgClass to group tasks together will allow to perform various operation on all the tasks. For example in this context extract all the tasks sizes.ewoksdraw/__init__.py: addedbuild_svg_task_group(ewoks_graph) -> SvgTaskGroup, hiding the_get_all_node_inputs/_get_all_task_output_namesnode-to-SvgTaskplumbing behind a single call.examples/convert_ewoks_to_elk_graph.py: runnable usage example.tests/test_elk_converter.py: unit tests covering top-level structure, children/task_sizes correspondence, single vs. multidata_mappingedges, empty graphs, counts across allewokscoreexample graphs, and thetask_sizesmismatch validation.Warning
Note to Reviewer(s)
portswere deliberately left out of scope — links are node-to-node, not port-to-port. This converter doesn't need it for layout purposes.task_sizesis a required, caller-supplied input rather than computed internally, so the ELK layout logic stays independent of thesvgpackage.