[KED-1650] New graphing algorithm & feature flags #185
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There's no onClick event, so the button element is unnecessary
This to distinguish clicked (i.e. selected) from active (i.e. hovered/focused)
Check that it sets nodeClicked to null if the selected node is being disabled
stichbury
reviewed
Jun 19, 2020
README.md
Outdated
|
|
||
| ### Viewing flags | ||
|
|
||
| See the message in your browser's console for info regarding the available flags and their values as currently set on your machine. |
There was a problem hiding this comment.
I'm not sure I follow this. What message? What console?
There was a problem hiding this comment.
I've reworded with some extra information including a link to a Mozilla article explaining the developer console, does that help?
There was a problem hiding this comment.
It does. So much better 🚀
stichbury
reviewed
Jun 19, 2020
stichbury
reviewed
Jun 19, 2020
stichbury
reviewed
Jun 19, 2020
stichbury
approved these changes
Jun 19, 2020
There was a problem hiding this comment.
Nice! Just one major issue, for me, which was that statement about browser console listings was a bit too vague. I'm no expert in this area, but if you can be more explicit in how you find the information, it would probably help every user at some level, by reducing ambiguity.
yetudada
approved these changes
Jun 19, 2020
# Conflicts: # src/selectors/layers.js # src/selectors/layout.js
|
Legal headers LGTM :) |
richardwestenra
approved these changes
Jun 23, 2020
yetudada
reviewed
Jun 23, 2020
|
Looks good to me -- the changes in ReadMe are spot on. |
Merged
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Improves pipeline visualisation using a new DAG graphing algorithm approach.
The algorithm is focused on improving the quality and clarity of output for both node layout and edge routing while maintaining good performance.
Goals include balancing the regularity, legibility, symmetry, spacing and compactness of layout and edges while scaling to graphs with a large number of nodes and edges, for which the existing solution does not handle well enough for this purpose.
These changes are delivered alongside a minimal feature flag system to support toggling of experimental new features such as this one until their general release.
The feature flag must be enabled in your browser before use and testing (see below).
Development notes
As there is currently no GUI for flags, they must be enabled and disabled using parameters in the URL. The status of all flags will persist in the user's local storage config and the console shows a message regarding the status of all flags on load.
The flag for the new graphing algorithm is
newgraph, it is disabled by default, you can use the URL parametersnewgraph=trueornewgraph=false.e.g.
http://localhost:4141/?data=demo&newgraph=true.The implementation itself is self contained and includes a selector that connects it to the rest of the system in the same way as the existing solution, such that it works as a drop-in replacement.
It currently depends on the
kiwi.jssolver, though there is potential that this may not eventually be required given some upgrades to the main solver.QA notes
The feature must be enabled by the user before testing, as described above. Try the following test data sets. Side-by-side comparison is possible by enabling or disabling the flag:
Demo
http://localhost:4141/?data=demo&newgraph=true
http://localhost:4141/?data=demo&newgraph=false
Random (includes layers)
http://localhost:4141/?data=random&newgraph=true
http://localhost:4141/?data=random&newgraph=false
(see console to get a reusable seed for direct comparison)
Animals
http://localhost:4141/?data=animals&newgraph=true
http://localhost:4141/?data=animals&newgraph=false
Nodes.json (as generated by Kedro)
http://localhost:4141/?newgraph=true
http://localhost:4141/?newgraph=false
It has been tested on both real samples as well as varied pipelines generated by the existing random generator, both ranging so far from around 10 nodes to around 1000, all showing some strong improvement, both when labels are visible and when not.
It has support for and has been tested with pipelines that include layers.
There are also unit tests covering the majority of the changes, though they are not sufficient to test the actual quality of graphing output which must be done manually. For this I have been considering some metrics for more objective evaluation but more exploration is needed.
While the algorithm appears to work well on most pipelines seen so far, wider beta testing is encouraged before the flag is removed for general release.
Checklist
RELEASE.mdfileLegal notice
I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":
I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorised to submit this contribution on behalf of the original creator(s) or their licensees.
I certify that the use of this contribution as authorised by the Apache 2.0 license does not violate the intellectual property rights of anyone else.