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.

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

Replace the PERT Chart layour alorithm #8

Merged
merged 5 commits into from Oct 20, 2020
Merged

Conversation

oxinabox
Copy link
Owner

@oxinabox oxinabox commented Jun 29, 2020

I am not happy with out the existing graph layout algorithms handle PERT charts.
Or Directed Acyclic Graphs more generally.

This is what the current (spring) layout comes up with:
spring

Here is a quick plot of the output of the new algorithm in this PR
dag

I haven't hooked it up properly to plotting but here is the new layout using this algorithm,
but here is a quick plot of the output.
It is basically the same as what I have drawn by hand for this chart (c and d are swapped though, I wonder if I can and a small term to favour having earlier nodes at the top).

The algorithm is the formulate the layout as a QP, which gets solved with JuMP / Ipopt.
Which is pretty fast.
The constraints are:

  • the column is set by the longest distance from the root node*.
  • The vertical placements must 1 unit apart
  • Minimize total squared vertical distance between lined nodes

(* method allows multiple root nodes because it works for all DAG but PERT charts just have 1)

@codecov-commenter
Copy link

codecov-commenter commented Jun 29, 2020

Codecov Report

Merging #8 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master       #8   +/-   ##
=======================================
  Coverage   91.78%   91.78%           
=======================================
  Files           5        5           
  Lines          73       73           
=======================================
  Hits           67       67           
  Misses          6        6           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce11a3b...8cd4965. Read the comment docs.

@oxinabox
Copy link
Owner Author

Can probably relax the upper and lower bound constraints a bunch more,
Just need to put some constraint on something so veritcal placement doesn't go off to a random point i think.
Maybe just constrain the first root to be at the origin.

@oxinabox
Copy link
Owner Author

This is basically a simplified version of Sugiyama layered graph drawing.
Especially the assignment to layers using longest path from root, and with how PERT charts are constrained to a single door and leaf.
https://en.m.wikipedia.org/wiki/Layered_graph_drawing

@rschwarz
Copy link

I've followed the discussion on Gitter and was curious how tools such as dot are implemented, so I tried to locate the relevant papers.

From what I've seen do far, it seems that the implementation is often done in multiple steps. These steps are presented as optimization problems, but then actually solved with heuristics. In particular, I found the steps:

  1. reverse arcs to make graph acyclic (does not apply in your application?)

  2. assign layers to nodes (you already have a heuristic based on longest paths?)

  3. find order of nodes within each layer, in order to minimize edge crossings

  4. find actual positions of nodes within layer (given order) to make edges short.

It seems to be that you're trying to do 3 and 4 in one go. If the order is given, I guess that 4) can actually be done well with a (convex) QP. But for 3) it might be better to deal with the discrete structure directly and formulate a MIP, or use some iterative improvement heuristic.

Further, with edges that span multiple layers, there's the trick of introducing artificial nodes on each layer to represent the edge going through. With this modification, the edge crossing only needs to consider adjacent layers.

@oxinabox
Copy link
Owner Author

oxinabox commented Jun 30, 2020

yeah, pretty much my thoughts also.
I think combining 3 and 4 works decently, but I think is worth looking into seperating them.
I have decided to move this code into its own package: https://github.com/oxinabox/LayeredLayouts.jl

Then can implement multiple algorithms there.

@oxinabox
Copy link
Owner Author

Redid it using Compose.jl + LayeredLayouts.jl
image

Will merge once LayeredLayouts is registered

move vertial constraint to be directly on variables and relax it a bit

tried some other options commented to say they don't work
@codecov-io
Copy link

codecov-io commented Oct 18, 2020

Codecov Report

Merging #8 into master will decrease coverage by 2.71%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #8      +/-   ##
==========================================
- Coverage   95.94%   93.23%   -2.72%     
==========================================
  Files           6        6              
  Lines         148      133      -15     
==========================================
- Hits          142      124      -18     
- Misses          6        9       +3     
Impacted Files Coverage Δ
src/ProjectManagement.jl 100.00% <ø> (ø)
src/graph_viz.jl 86.66% <50.00%> (-10.48%) ⬇️
src/timing_distributions.jl 86.36% <0.00%> (-1.14%) ⬇️
src/timing_graphs.jl 90.00% <0.00%> (-0.48%) ⬇️
src/paths.jl 100.00% <0.00%> (ø)
src/project.jl 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 91e66f8...f7f02e5. Read the comment docs.

@oxinabox oxinabox merged commit c350b9d into master Oct 20, 2020
@oxinabox oxinabox deleted the ox/graphplotbetter branch October 20, 2020 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants