Added -a option to alias the largest Node in the graph. #8
Conversation
…lias to rewrite the pangraph's largest degree vertex into 4 equivalently connected Vertices.
src/fantasi/Tuura/Fantasi/Main.hs
Outdated
| let graphVHDL = VHDL.writeGraph pangraph | ||
| let simEnvVHDL = VHDL.writeEnvironment pangraph | ||
|
|
||
| -- output vhdl graph | ||
| writeFile graphVHDLPath graphVHDL | ||
| -- output vhdl simulation environment | ||
| writeFile simulationEnvVhdlPath simEnvVHDL | ||
|
|
||
| getPangraph :: ByteString -> Options -> Maybe Pangraph |
There was a problem hiding this comment.
It feels strange to intertwine parsing and aliasing logic. I suggest you move this to the main function: first parse a graph, reporting parse errors, and then perform your transformation.
|
@thisiswhereitype Thanks for the first try! I've left a few minor comments, but my major concern is complexity: the current implementation seems unnecessary complex to me. Below I give an alternative using the Alga library: -- Given a hub vertex, and four new vertices, the function replaces the hub with the vertices
-- reducing the peak connectivity but preserving all distances.
-- Note that our assumption is that in the original graph we have edges going in both direction
-- between every pair of connected vertices. In the result, however, this is no longer true.
splitHub :: Ord a => a -> (a, a, a, a) -> Graph a -> Graph a
splitHub x (aa, ab, ba, bb) g = overlays
[ removeVertex x g
, connect (vertices a) (vertices [aa, ab])
, connect (vertices b) (vertices [ba, bb])
, connect (vertices [aa, ba]) (vertices a)
, connect (vertices [ab, bb]) (vertices b) ]
where
neighbours = toList $ preSet x g
n = length neighbours
half = n `div` 2
(a, b) = (take half neighbours, drop half neighbours)I haven't compiled it, but this is much easier to understand and maintain. Would you like to give it a try? |
|
Most of the code was in place, had to update pangraph for compatibility with alga 0.2 |
|
Finding a hub can also be easily implemented using Alga, so let's do that to avoid using two different graph libraries. You can just look at |
|
Are we not interested in |
|
No, because we assume that the given graph is undirected (see the comment), that is |
|
@thisiswhereitype I think we could generalise this to not necessarily undirected graphs. We will then need to split both preset and postset of the hub, as follows: splitHub :: Ord a => a -> (a, a, a, a) -> Graph a -> Graph a
splitHub x (aa, ab, ba, bb) g = overlays
[ removeVertex x g
, biclique ai [aa, ab]
, biclique bi [ba, bb]
, biclique [aa, ba] ao
, biclique [ab, bb] bo ]
where
is = toList $ preSet x g
os = toList $ postSet x g
hi = length is `div` 2
ho = length os `div` 2
(ai, bi) = (take hi is, drop hi is)
(ao, bo) = (take ho os, drop ho os) Doesn't seem a lot more complicated. Shall we go for this solution instead of the above? |
…Alga, move all aliasing code to HubRewrite.hs
|
I have addressed all your comments @snowleopard |
| else Just | ||
| maybePangraph = P.parse bs >>= applyAlias | ||
| in fromMaybe (error errMsg) maybePangraph | ||
| where errMsg = "Pangraph is nothing! Does the graph construct?" |
There was a problem hiding this comment.
This is not a helpful message: the user of FANTASI has no idea what Pangraph is, and what it means for it to be "nothing". Please make this more informative.
Furthermore, you still seem to be mixing up parsing and aliasing. Why not parse first, report parse errors, and then alias? It makes the code easier to read.
There was a problem hiding this comment.
You are right, I was thinking as a Pangraph user. I think it I might try to catch Maybe exceptions here.
| (out0, out1) = breakInHalf . toList $ postSet x g | ||
|
|
||
| -- Useful alias. | ||
| type Endpoints = (VertexID, VertexID) |
There was a problem hiding this comment.
This seems to have been used only once, which I guess doesn't make it very useful. I'd just inline it.
|
@thisiswhereitype Thanks, much simpler now! See some further comments above. Also, I think in practice we'll need to split multiple hubs, not just one. Perhaps, we should accept a number It would also be interesting if, when splitting is requested, you report how this improved the graph. More specifically, report the maximum degree, the total number of vertices and the total number of (one-directional) edges. |
Intuitively this seems parallisable when the hubs are not neighbours; so maybe the algorithm can work in on different subgraphs and then apply recursively until a desired metric is met?
I wonder what stats are useful heuristics for the success of the routing algorithm. hubImprovementStats :: (Stats, Stats) -> StatDiff
graphStats :: Graph a -> Stat
data Stats = Stats
{ maxDeg :: Int
, degVariance :: Float
, edgeCount :: Int
, edgeNeighboursBy2Hops :: Int
} ...
|
|
@thisiswhereitype Hi there! Are you planning to finish this PR? It's not urgent, but I'd like to merge it within the next month. |
Additions:
src/Tuura/Fantasi/HubRewrite.hscalledaliasHubcan be explored withdriven1.-ato apply this on thefantasiexecutable.Could verify connectivity remains the same with a new graph library?
Could apply recursively to reduce hub outliers? Perhaps we should measure the https://en.wikipedia.org/wiki/Conductance_(graph) ?