Skip to content
This repository has been archived by the owner on Oct 8, 2021. It is now read-only.

Subgraph #18

Merged
merged 3 commits into from Mar 3, 2015
Merged

Subgraph #18

merged 3 commits into from Mar 3, 2015

Conversation

jpfairbanks
Copy link
Contributor

easy to use function function inducedsubgraph(g::AbstractGraph, iter) which returns the new graph and the vertex name map as a Dict.

worker function for filtering that does not allocate any temporaries.
@doc "inplace filtering for preallocated output, edge iterable, vertex mapping" -> function inducedsubgraph!(h::AbstractGraph, edges, newvid)

with test
updated export and runtests.jl
add the test for subgraphs.jl
@jpfairbanks jpfairbanks mentioned this pull request Mar 3, 2015
@sbromberger
Copy link
Owner

This is great - thanks very much. Could we put this in operators.jl, though, and just call the functions "subgraph"?

@jpfairbanks
Copy link
Contributor Author

Yes it can go in operators.

From wolfram mathworld: A vertex-induced subgraph (sometimes simply called an "induced subgraph") is a subset of the vertices of a graph G together with any edges whose endpoints are both in this subset.
http://mathworld.wolfram.com/Vertex-InducedSubgraph.html

In the future we can support "graph minor" where the vertices are collapsed into each other by "edge contraction" http://mathworld.wolfram.com/GraphMinor.html although I don't have time to implement graph minor like i just did with induced subgraph.

@coveralls
Copy link

Coverage Status

Coverage increased (+2.53%) to 95.06% when pulling 6488842 on jpfairbanks:subgraph into 13bb7b2 on sbromberger:master.

@sbromberger
Copy link
Owner

OK - move to operators, keep the name :) But let's figure out why Travis is failing first.

@sbromberger
Copy link
Owner

ah, the docstring. Perhaps just leave it as a comment for now?

@jpfairbanks
Copy link
Contributor Author

I used @doc so it didn't work on 0.3. I only use 0.4 locally. Either a version check or using Docile.

@jpfairbanks
Copy link
Contributor Author

ok that's fine too but then we need to add it to doc/somewhere to get it in read the docs

@sbromberger
Copy link
Owner

I'll put it in RTD in a new commit. No worries.

@jpfairbanks
Copy link
Contributor Author

i think i beat you to it?

@sbromberger
Copy link
Owner

Hah. ok. One other thing - does it make sense to test iter against g? that is, error if any element of iter > nv(g)?

@jpfairbanks
Copy link
Contributor Author

you won't get an error if you don't. when iterating over the edges you will never try that vertex.

I'm inclined to follow the philosophy of handling argument diversity in the caller. We don't know how the iterable will be generated.

@sbromberger
Copy link
Owner

Yup, I see it (and agree with your philosophy). Will merge once travis comes back green - thanks :)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.11%) to 92.64% when pulling df710a5 on jpfairbanks:subgraph into 13bb7b2 on sbromberger:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+2.63%) to 95.16% when pulling df710a5 on jpfairbanks:subgraph into 13bb7b2 on sbromberger:master.

sbromberger added a commit that referenced this pull request Mar 3, 2015
@sbromberger sbromberger merged commit 03f0972 into sbromberger:master Mar 3, 2015
@sbromberger
Copy link
Owner

(disclaimer: agree with your philosophy except for add_edge!, where I error if the edge already exists.)

@sbromberger
Copy link
Owner

BTW: thanks for this. I can't wait to try it out!

@sbromberger
Copy link
Owner

Oh - one other thing (post merge): is this intended to work for directed graphs as well?

If so, it needs a change:

julia> g = DiGraph(100,200)
{100, 200} directed graph

julia> h = inducedsubgraph(g,[1,2,3,4])
({4, 0} undirected graph,Dict{Any,Any}(4=>4,2=>2,3=>3,1=>1))

If not, it needs to a change to only accept Graph (instead of AbstractGraph).

@jpfairbanks
Copy link
Contributor Author

jpfairbanks commented Mar 3, 2015 via email

@sbromberger
Copy link
Owner

Better way to do that is as follows:

function inducedsubgraph{T<:AbstractGraph}(g::T, iter)
    n = length(iter)
    h = T(n)
...

Let me code something up. I also think we can get more efficiency by using finclist of the original graph. I'll do some testing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants