-
Notifications
You must be signed in to change notification settings - Fork 92
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
Induced bipartite #147
base: master
Are you sure you want to change the base?
Induced bipartite #147
Changes from all commits
3ec79c5
dc3b24c
8ed3577
21edb53
d48860a
bf8f0f6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -708,6 +708,43 @@ egonet(g::AbstractGraph{T}, v::Integer, d::Integer, distmx::AbstractMatrix{U}=we | |
g[neighborhood(g, v, d, distmx, dir=dir)] | ||
|
||
|
||
""" | ||
induced_bipartite_subgraph(G,X,Y) | ||
|
||
Return the bipartite subgraph of `g` induced by the disjoint subsets of vertices X and Y. | ||
|
||
""" | ||
function induced_bipartite_subgraph(g::T,X::AbstractVector{U},Y::AbstractVector{U}) where T <: AbstractGraph where U <: Integer | ||
|
||
X ∩ Y != [] && throw(ArgumentError("X and Y sould not intersect!")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would this ever be satisfied with an untyped array? better to test the length |
||
!(X ⊆ vertices(g) && Y ⊆ vertices(g)) && throw(ArgumentError("X and Y should be subsets of the vertices")) | ||
!(allunique(X) && allunique(Y)) && throw(ArgumentError("Vertices in subsets of vertices must be unique")) | ||
|
||
|
||
n = length(X) + length(Y) | ||
G = T(n) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no guarantee this constructor exists (I'm not sure how we solve that though) |
||
Y_set = Set(Y) | ||
X_set = Set(X) | ||
|
||
newIndex = Dict(reverse.(collect(enumerate([X;Y])))) | ||
|
||
for x in X | ||
for y in outneighbors(g,x) | ||
(y ∉ Y_set) && continue | ||
add_edge!(G,newIndex[x],newIndex[y]) | ||
end | ||
end | ||
|
||
for y in Y | ||
for x in outneighbors(g,y) | ||
(x ∉ X_set) && continue | ||
add_edge!(G,newIndex[y],newIndex[x]) | ||
end | ||
end | ||
G | ||
end | ||
|
||
|
||
|
||
""" | ||
compute_shifts(n::Int, x::AbstractArray) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -308,6 +308,7 @@ | |
@test sort(vm) == [1:5;] | ||
end | ||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. useless diff |
||
gs = star_graph(10) | ||
distgs = fill(4.0, 10, 10) | ||
@testset "Egonet: $g" for g in testgraphs(gs) | ||
|
@@ -318,6 +319,14 @@ | |
@test @inferred(ndims(g)) == 2 | ||
end | ||
|
||
g10 = complete_graph(10) | ||
@testset "Induced bipartite Subgraphs: $g" for g in testgraphs(g10) | ||
sg = @inferred(induced_bipartite_subgraph(g, [2,3],[4,5])) | ||
@test nv(sg) == 4 | ||
@test ne(sg) == 4 | ||
@test is_bipartite(sg) | ||
end | ||
Comment on lines
+322
to
+328
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While this is good, I think we need a few more test cases, e.g.
It might also be good to test more than just some properties, namely if the subgraph is actually the one that we wanted. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that more tests would be nice |
||
|
||
gx = SimpleGraph(100) | ||
@testset "Length: $g" for g in testgraphs(gx) | ||
@test length(g) == 10000 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From convention it would be better to use
G
instead ofT
for the graph type. Furthermore, as far as I understand, this method would not work on any kind ofAbstractGraph
, only onSimpleGraph
andSimpleDiGraph
. So I would restrict it to these types of graphs.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I copied the style of
induced_subgraph
that also usesT
for the graph type. If this is more consistent with the convention, I can change it. Also, we could follow what's been done forinduced_subgraph
which also returns a mapping from the subgraph to the whole graph. This solution would let people use this function for any AbstractGraph (for example, using the mapping to retrieve information in a metagraph)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think following
induced_subgraph
is a good idea for the mapping, for the types I'd go withG