kedro-org / kedro-viz

Visualise your Kedro data and machine-learning pipelines and track your experiments.
https://demo.kedro.org
Apache License 2.0
680 stars 113 forks source link

Ensure deterministic graph calculation with consistent layer, node, and edge ordering in Kedro-Viz #2185

Closed rashidakanchwala closed 4 days ago

rashidakanchwala commented 1 week ago

Description

Resolves #2057.

Based on @astrojuanlu's comment, I realized that Kedro-Viz might be causing the randomness. To investigate, I checked if the inputs to graph calculations—nodes, edges, and layout—were consistent. While the order of nodes and layout remained stable, the order of edges varied with each backend data request. To address this, we now sort the layers, the edges and nodes to ensure consistency.

Update - As @ravi-kumar-pilla noted, there was an issue with layer ordering: changing a node name could alter the layer order, especially for layers with identical dependencies, like model_input and feature in the example he shared. This issue stemmed from the layer_dependencies dictionary in services/layers.py, where layers with the same dependencies weren’t consistently ordered. To fix this, I added alphabetical sorting for layers with identical dependencies to ensure stability in toposort.

For nodes and edges, I now sort them immediately upon loading from the backend API, ensuring they are consistently ordered in the Redux state.

For testing, I initially considered using Cypress/backend e2e testing with screenshot comparison of the flowchart, but it proved too complex. Instead, I created a new mock dataset, reordered_spaceflights, with reordered nodes and edges. I added tests in normalise-data-test and actions/graph-test. The first test verifies that nodes and edges are consistently sorted by their ids, regardless of backend order. The second test compares the x and y coordinates in the flowchart, confirming that the graph layout is the same between the two mocks.

QA notes

Make changes to the files, doc changes, and see the layout is consistent.

Checklist

jitu5 commented 1 week ago

@rashidakanchwala I tested with both your branch and main, I made the changes roughly 5 to 7 times and took screenshot every time and compared, it look exactly same on your branch and on main its different every time.

Its working for me.

ravi-kumar-pilla commented 1 week ago

Hi @rashidakanchwala , The fix looks promising. I tried without expanding modular pipelines and the nodes seem to position similarly. However, upon expanding modular pipelines, there seems to be some random positioning like below -

Run 1 -

image

Run 2 -

image

Observation: create_feature_importance node has changed its position, so are the layers model_input and feature

Steps to reproduce:

  1. Follow QA steps
  2. Expand all modular pipelines, observe layout of feature_engineering
  3. Change node name of apply_types_to_companies to apply_types_to_companies_test inside data_ingestion
  4. Observe the layout with the change

Some questions:

  1. Can we try sorting nodes and layers as well for consistency ?
  2. Is there any overhead or lag you observed due to sorting as graphNew is an expensive operation ?
  3. Do we think of adding any tests to be sure of the positioning ?

Thank you