Modified isSubGraph in Labelled.AdjacencyMap, minor error rectified

[adithyaov]

Changed Labelled.AdjacencyMap.isSubGraph

Please note that I am unable to run the tests (QuickCheck) on my system due to a problem with WSL. So the only option for me to verify if the tests are passing is Travis CI.

[snowleopard]

@adithyaov The following properties should be satisfied by isSubgraphOf:

isSubgraphOf x (overlay   x y) == True
isSubgraphOf x (connect e x y) == True
isSubgraphOf y (connect e x y) == True

I don't think your current implementation satisfies these requirements.

[snowleopard]

Also, if you are fixing a bug, please add a test that currently fails but passes with yours fix.

[adithyaov]

@snowleopard

isSubgraphOf x (overlay   x y) == True
isSubgraphOf x (connect e x y) == True
isSubgraphOf y (connect e x y) == True

Satisfying the properties above becomes really complex. Please note the following, say,

x = edge (Sum 2) 0 1
y = edge (Sum -2) 0 1
z = overlay x y = vertices [0, 1]

Justifying that x is a subgraph of z is quite dufficult. We would probably need to check different conditions depending on the Monoid (edge).
In case of Sum, isSubgraphOf x y = isSubMapOfBy (isSubMapOfBy (\_ _ -> True)) x y as Sum can result in anything.
I'll get back to this PR after some amount of thought.

[snowleopard]

@adithyaov We typically want edge labels to come from a dioid:

http://hackage.haskell.org/package/algebraic-graphs-0.4/docs/Algebra-Graph-Label.html#t:Dioid

Sum is not a dioid, which is why you have issues here. Combining algebraic graphs and non-dioids is problematic, because many basic properties no longer hold, e.g. x + x = x.

Both Distance and Capacity are dioids, so there should be no issues with them.

Dioids are idempotent, which allows us to keep the monotonicity of algebraic graphs: by overlaying a graph with another graph, we can only add structure, not remove it.

[adithyaov]

@snowleopard
Ah, I see, In that case, I believe the previous implementation is correct. I think one should probably change isSubgraphOf to work only with dioids.
I think I got an error as I was checking it for the monoid Sum.

[snowleopard]

I think I got an error as I was checking it for a Sum Monoid.

Right, I see. Indeed, with Sum there will be a few issues. I added a note about this: #175 (comment).

If the current implementation works for dioids, then I think there is no need to change anything for now, but we'll definitely need to improve documentation for edge-lebelled graphs, as discussed in #175.

[adithyaov]

Closing this PR for now. Please feel free to reopen this if required.
Please refer to #175