Verizon / quiver

A reasonable library for modeling multi-graphs in Scala
http://verizon.github.io/quiver/
Apache License 2.0
200 stars 39 forks source link

Embedding does not create nodes #13

Closed dozed closed 7 years ago

dozed commented 8 years ago

I would expect that embedding a context, embeds all the nodes from that context to the new graph. Currently in the new graph the nodes appear in the node's context, and also in predecessors and successors, but not in nodes. decomp does not give the node labels in the context, which prevents adding nodes when embedding.

Is this a known issue? Are there any plans to address this?

val ctx = Context(Vector(((), 1)), 2, (), Vector(((), 2), ((), 3)))

val g1 = empty[Int, Unit, Unit]
  .addNode(LNode(1, ()))
  .addNode(LNode(2, ()))
  .addNode(LNode(3, ()))
  .embed(ctx)

val g2 = empty[Int, Unit, Unit]
  .embed(ctx)

println(g1)
//  1:()->[((),2)]
//  3:()->[]
//  2:()->[((),2),((),3)]

println(g2)
//  2:()->[((),2),((),3)]
// node 3 does appear in an edge, but not as a node

println(g1.decomp(2).ctx)
println(g2.decomp(2).ctx)
//  Some(Context(Vector(((),1)),2,(),Vector(((),2), ((),3))))
//  Some(Context(Vector(((),1)),2,(),Vector(((),2), ((),3))))

println(g2.successors(2))
println(g2.predecessors(2))
println(g2.nodes)
//  Vector(2, 3)
//  Vector(1, 2)
//  Vector(2)
// successors/predecessors return nodes which are not in the graph
timperrett commented 8 years ago

@stew or @kaiserpelagic might have some insights here. Sorry I didnt notice your request earlier, @dozed :-)

kaiserpelagic commented 8 years ago

@dozed I think this is at the very least a documentation bug (I could be convinced otherwise though), as a Context in this situation should only refer to Nodes already in the Graph. I see a couple of options here 1) throw an error in this situation (undesirable), 2) return an Option 3) document the behavior and provide some functionality to create a Graph from a Context. What do you think?

runarorama commented 8 years ago

Yeah, this is a documentation issue, but is working as designed. The Context describes the context of a node within a given graph. In this case ctx assumes a graph that contains nodes 2 and 3.

The reason to not throw an error in the presence of node 3 is that the intent is to allow adding node 3 to this graph later, in which case it would already have an edge from node 2.

Most importantly, we always want the following to be true:

g & c decomp c.vertex == Decomp(Some(c), g)

embed should only ever add a single node: the node under focus in the Context.

runarorama commented 8 years ago

Related: https://github.com/Verizon/quiver/issues/6