Skip to content
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.

Sign up for GitHub

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

Separate path optimization of disconnected components in the network graph #53

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Separate path optimization of disconnected components in the network graph #53

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
474427d
Implement function to find path disconnected components
Todorbsc Feb 9, 2024
4484f5d
Fix einexpr function to respect Optimizers abstract class method
Todorbsc Feb 9, 2024
0700b48
Separate optimization for each disconnected components in path
Todorbsc Feb 9, 2024
ba187ec
Merge branch 'master' into fix/issue51_separe_disconn_graphs_opt
Todorbsc Feb 9, 2024
95af947
Fix code style
Todorbsc Feb 13, 2024
4a1993c
Fix code style
Todorbsc Feb 13, 2024
2fc3371
Fix code style
Todorbsc Feb 13, 2024
48f18fc
Replace dfs function by connected_components from Graphs package
Todorbsc Feb 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions src/EinExpr.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,3 +300,43 @@ function sumtraces(path::EinExpr)

EinExpr(path.head, _args)
end

function dfs(graph, v, visited, subgraph)
visited[v] = true
push!(subgraph, v)
for u in 1:size(graph, 1)
if graph[v, u] && !visited[u] # Check adjacency and visited status
dfs(graph, u, visited, subgraph)
end
end
end

function components(path::EinExpr{L}) where {L}
network = [args.head for args in path.args]
# create adjacency graph of the network
tengraph = zeros(Bool, length(network), length(network))
for i in 1:length(network)
for j in 1:length(network)
if !isempty(network[i] ∩ network[j]) && i ≠ j
tengraph[i,j] = true
end
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! But let's rewrite it like this:

Suggested change
network = [args.head for args in path.args]
# create adjacency graph of the network
tengraph = zeros(Bool, length(network), length(network))
for i in 1:length(network)
for j in 1:length(network)
if !isempty(network[i] ∩ network[j]) && i ≠ j
tengraph[i,j] = true
end
end
end
network = map(head, path.args)
# create adjacency matrix of the network
n = nargs(path)
adjmat = falses(n,n)
for (i,j) in combinations(1:n, 2)
if !isdisjoint(network[i], network[j])
adjmat[i,j] = true
adjmat[j,i] = true
end
end

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!


# find disconneceted subgraphs
subgraphs = Vector{Vector{Int}}()
n = size(tengraph, 1)
visited = falses(n)
for v in 1:n
if !visited[v]
subgraph = Int[]
dfs(tengraph, v, visited, subgraph)
push!(subgraphs, subgraph)
end
end

# create subnetworks
indeppaths = [sum(EinExpr.([network[tns] for tns in subnet])) for subnet in subgraphs]

return indeppaths
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Continuing with above

Suggested change
# find disconneceted subgraphs
subgraphs = Vector{Vector{Int}}()
n = size(tengraph, 1)
visited = falses(n)
for v in 1:n
if !visited[v]
subgraph = Int[]
dfs(tengraph, v, visited, subgraph)
push!(subgraphs, subgraph)
end
end
# create subnetworks
indeppaths = [sum(EinExpr.([network[tns] for tns in subnet])) for subnet in subgraphs]
return indeppaths
# find disconneceted subgraphs
subgraphs = Vector{Vector{Int}}()
visited = falses(n)
for vertex in Iterators.filter(v -> !visited[v], 1:n)
subgraph = Int[]
dfs(adjmat, vertex, visited, subgraph)
push!(subgraphs, subgraph)
end
# create subnetworks
indeppaths = [sum(EinExpr.([network[tns] for tns in subnet])) for subnet in subgraphs]
return indeppaths

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

end
4 changes: 2 additions & 2 deletions src/Optimizers/Greedy.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ The implementation uses a binary heaptree to sort candidate pairwise tensor cont
outer::Bool = false
end

function einexpr(config::Greedy, path::EinExpr{L}, sizedict::Dict{L}) where {L}
function einexpr(config::Greedy, sizedict::Dict{L}, path::EinExpr{L}) where {L}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change the order?

Copy link
Author

@Todorbsc Todorbsc Feb 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of the multiple dispatching: this way, all optimizers' einexpr function are called through Optimizers.einexpr when the function call is like the one suggested in the issue #51:

network = [[:a, :b], [:b, :c], [:A, :B], [:B, :C]]
expr = sum(EinExpr.(network))
optdata = Dict(key => 2 for key in unique(vcat(network...)))
einexpr(Exhaustive(), expr, optdata)

Although I am not really convinced about this fix, it makes possible to call the optimizers with each of the non-interconnected subgraphs at the level of abstraction of the class Optimizers avoiding the need to modify each specific optimizer.

Let me know what are your thoughts on this and whether there's another more-suitable way to implement this.

# remove self-loops
path = sumtraces(path)
metric = config.metric(sizedict)
Expand Down Expand Up @@ -70,4 +70,4 @@ function einexpr(config::Greedy, path::EinExpr{L}, sizedict::Dict{L}) where {L}
return path
end

einexpr(config::Greedy, path::SizedEinExpr) = SizedEinExpr(einexpr(config, path.path, path.size), path.size)
einexpr(config::Greedy, path::SizedEinExpr) = SizedEinExpr(einexpr(config, path.size, path.path), path.size)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above: why change the order?

2 changes: 0 additions & 2 deletions src/Optimizers/Naive.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ using AbstractTrees

struct Naive <: Optimizer end

einexpr(::Naive, path, _) = einexpr(Naive(), path)

function einexpr(::Naive, path)
# remove self-loops
path = sumtraces(path)
Expand Down
2 changes: 1 addition & 1 deletion src/Optimizers/Optimizers.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ abstract type Optimizer end
function einexpr end

einexpr(T::Type{<:Optimizer}, args...; kwargs...) = einexpr(T(; kwargs...), args...)
einexpr(config::Optimizer, expr, sizedict) = einexpr(config, SizedEinExpr(expr, sizedict))
einexpr(config::Optimizer, expr, sizedict) = sum(einexpr.((config,), [SizedEinExpr(exp, sizedict) for exp in [comp for comp in components(expr)]]))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
einexpr(config::Optimizer, expr, sizedict) = sum(einexpr.((config,), [SizedEinExpr(exp, sizedict) for exp in [comp for comp in components(expr)]]))
einexpr(config::Optimizer, expr, sizedict) = sum(einexpr.((config,), map(exp -> SizedEinExpr(exp, sizedict), components(expr))))

Maybe it could be rewritten like this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I completely agree. I guess I am still getting used to Julia haha
Made the change! Thanks


include("Naive.jl")
include("Exhaustive.jl")
Expand Down
Loading