Refactor TensorNetwork type; add NormNetwork struct. #119
Refactor TensorNetwork type; add NormNetwork struct. #119jack-dunham wants to merge 102 commits into
TensorNetwork type; add NormNetwork struct. #119Conversation
Introduce `BeliefPropagationProblem` wrapper to hold the cache and the error `diff` field. Also simplifies some kwargs wrangling.
…be set from another cache
Also includes some fixes to the way `TensorNetwork` types are constructed based on index structure.
for more information, see https://pre-commit.ci
…instead of trying to operate on existing graphs The reason for this is: - One only cares about the edges of the input graph - A simple graph cannot be used as it "forgets" its edge names resulting in recursion - As shown with `TensorNetwork`, removing edges may not always be defined.
…s from an array.
This was caused by the change to the `cache` being backed by a directed graph.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #119 +/- ##
==========================================
- Coverage 80.35% 78.50% -1.85%
==========================================
Files 13 13
Lines 738 670 -68
==========================================
- Hits 593 526 -67
+ Misses 145 144 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Your PR requires formatting changes to meet the project's style guidelines. Click here to view the suggested changes.diff --git a/src/normnetwork.jl b/src/normnetwork.jl
index 881683c..f165e5f 100644
--- a/src/normnetwork.jl
+++ b/src/normnetwork.jl
@@ -25,7 +25,9 @@ end
Base.eltype(::Type{<:NormNetwork{T}}) where {T} = LazyITensor{eltype(T), T}
-NormNetwork(tn::ITensorNetwork) = NormNetwork(tn, map(uniquename, keys(tn.dimname_vertices)))
+function NormNetwork(tn::ITensorNetwork)
+ return NormNetwork(tn, map(uniquename, keys(tn.dimname_vertices)))
+end
# ====================================== Graphs.jl ======================================= #
diff --git a/test/test_apply_operator.jl b/test/test_apply_operator.jl
index a20d2b1..1d4d623 100644
--- a/test/test_apply_operator.jl
+++ b/test/test_apply_operator.jl
@@ -3,7 +3,7 @@ import TensorAlgebra as TA
using GradedArrays: U1, gradedrange
using Graphs: dst, edges, src, vertices
using ITensorBase: Index, name, named, operator, setname, uniquename
-using ITensorNetworksNext: NormNetwork, ITensorNetwork, apply_operator, apply_operators,
+using ITensorNetworksNext: ITensorNetwork, NormNetwork, apply_operator, apply_operators,
beliefpropagation, insertlink!, message_environment, tensornetwork
using MatrixAlgebraKit: truncrank
using NamedGraphs.NamedGraphGenerators: named_cycle_graph, named_path_graph
diff --git a/test/test_beliefpropagation.jl b/test/test_beliefpropagation.jl
index e3d6ddb..3ecf8ee 100644
--- a/test/test_beliefpropagation.jl
+++ b/test/test_beliefpropagation.jl
@@ -3,8 +3,8 @@ using DataGraphs: DataGraphs, DataGraph, edge_data, edge_data_type
using Dictionaries: Dictionary, dictionary, set!
using Graphs: AbstractGraph, dst, edges, has_edge, src, vertices
using ITensorBase: ITensor, Index, inds, name, noprime, prime
-using ITensorNetworksNext: ITensorNetworksNext, MessageCache, StopWhenConverged,
- ITensorNetwork, bethe_free_energy, edge_scalar, incoming_messages, linkinds,
+using ITensorNetworksNext: ITensorNetworksNext, ITensorNetwork, MessageCache,
+ StopWhenConverged, bethe_free_energy, edge_scalar, incoming_messages, linkinds,
messagecache, region_scalar, subgraph, tensornetwork, vertex_scalar, vertex_scalars
using LinearAlgebra: LinearAlgebra
using NamedGraphs.GraphsExtensions: all_edges, arranged_edges, incident_edges, vertextype
diff --git a/test/test_contract_network.jl b/test/test_contract_network.jl
index d8cbdf6..a6cdb30 100644
--- a/test/test_contract_network.jl
+++ b/test/test_contract_network.jl
@@ -1,8 +1,7 @@
using Graphs: edges, vertices
-using ITensorNetworksNext: contract_network, linkinds, siteinds, tensornetwork
using ITensorBase: Greedy, Index, Optimal
-using ITensorNetworksNext:
- Exact, LeftAssociative, ITensorNetwork, contract_network, linkinds, siteinds, tensornetwork
+using ITensorNetworksNext: Exact, ITensorNetwork, LeftAssociative, contract_network,
+ linkinds, siteinds, tensornetwork
using NamedGraphs.GraphsExtensions: arranged_edges, incident_edges
using NamedGraphs.NamedGraphGenerators: named_grid
using TensorOperations: TensorOperations
|
jack-dunham
force-pushed
the
jd/tensornetwork-refactor
branch
from
1310cc2 to
7c51b20
Compare
| B = conjbra(nn, vertex) | ||
| # TODO: implement and use a lazy `conj` via `LazyNamedDimsArrays` here? | ||
| return lazy(A) * lazy(conj(B)) |
There was a problem hiding this comment.
Maybe we can define bra(nn, vertex) = conj(conjbra(nn, vertex))? Agreed we should probably define a lazy conj, it is something that has come up a few places, but eager is ok for now.
| ket(nn::NormNetwork, vertex) = nn.tensornetwork[vertex] | ||
| conjbra(nn::NormNetwork, vertex) = replacedimnames(n -> namemap(nn, n), ket(nn, vertex)) | ||
|
|
||
| lazy_norm(tn::TensorNetwork) = NormNetwork(tn) |
There was a problem hiding this comment.
I think we were calling this norm_network in ITensorNetworksNext. I kind of prefer that name since it indicates what kind of lazy representation it is, i.e. one could imagine lazy_norm might output a lazy expression for the norm. I think norm_network is a bit more descriptive.
| return tn | ||
| end | ||
| # Return the vertices associated with an index. | ||
| function indsites(tn::AbstractGraph, ind) |
There was a problem hiding this comment.
I guess my motivation for this name was the existing function names for the tensor network type:
linkinds
siteindsI.e. indsites is the "inverse" of siteinds. Happy to revisit these names, but we should choose something consistent.
There was a problem hiding this comment.
One contrast to make is that this function really just using the dimension name to look up with vertices, while siteinds is indicating that it is outputting indices/named axes. And I agree that it is slightly unfortunate mixing graph terminology of vertices/edges with more physics terminology of sites/links, I went back and forth about that but I'm not sure we need to overoptimize for consistency with linkinds/siteinds naming here.
| for e in edges(graph) | ||
| show(io, mime, e) | ||
| println(io) | ||
| function has_indname(tn::AbstractGraph, name) |
There was a problem hiding this comment.
See above comment on this.
| index_map = Dictionary{I, I}() | ||
| for (name, vertices) in pairs(tn.index_locations) | ||
| if length(vertices) == 2 | ||
| insert!(index_map, name, randname(name)) |
There was a problem hiding this comment.
It might be nice to have an interface where we can specify the ket names (say if you have a set of messages and want a norm network that matches the names of the messages).
There was a problem hiding this comment.
Good idea. We can have:
### Inner constructor:
function NormNetwork(tn::TensorNetwork{T, V, I}, map::Dictionary{I, I}) where {T, V, I}
namemap = Dictionary{I, I}()
for (name, vertices) in pairs(tn.index_locations)
if length(vertices) == 2
insert!(namemap, name, map[name])
end
end
return new{T, V, I}(tn, namemap)
end
###
NormNetwork(tn::TensorNetwork) = NormNetwork(tn, map(randname, keys(tn.index_locations)))
| function TensorNetwork(f::Base.Callable, graph::AbstractGraph) | ||
| return TensorNetwork(graph, Dictionary(map(f, vertices(graph)))) | ||
| end | ||
| for ind in dimnames(tensor) |
There was a problem hiding this comment.
The terminology is definitely kind of tricky. I think if something is just a name, we should refer to it as dimname or name. In my mind, ind would refer to a named axis, though I've gone back and forth about whether we should still use the ind/inds terminology from ITensors.jl or embrace the axis/axes terminology from Base Julia...
There was a problem hiding this comment.
In this case, it should definitely be name (I was going back and forth between storing the actual index or the name, so I think this is left over from that).
There was a problem hiding this comment.
I've gone back and forth about the dimnames vs. inds question as well, and the approach if been trying for in ITensorBase/ITensorNetworksNext is actually a fairly big shift from ITensors/ITensorMPS/ITensorNetworks: I think we were overusing inds (i.e. overusing passing around both the name and the range/space information) while I think it is clearer in many places to just to use the name as an input/output. I.e. I'd like to be more precise in ITensorBase/ITensorNetworksNext about which functions really just need inputs and outputs of dimension names without the space. The issue was that in ITensors/ITensorMPS/ITensorNetworks, we used inds in places where really only the name was needed and being used, so it become unclear when the space was being ignore/implicitly dropped (such as in replaceinds).
|
|
||
| struct TensorNetwork{T, V, I} <: AbstractTensorNetwork{T, V} | ||
| tensors::Dictionary{V, T} | ||
| index_locations::Dictionary{I, Set{V}} |
There was a problem hiding this comment.
Maybe a better name would be dimname_vertices.
Defaults to one constructed via `randname`.
for more information, see https://pre-commit.ci
| if VERSION >= v"1.11.0-DEV.469" | ||
| eval( | ||
| Meta.parse( | ||
| "public apply_operator, apply_operators, beliefpropagation_normnetwork, identity_norm_message_env, normnetwork, norm_message_env, ones_norm_message_env, rand_norm_message_env, randn_norm_message_env, similar_norm_message_env" |
There was a problem hiding this comment.
I don't think we wanted to remove all of these, i.e. apply_operator[s] should remain.
4 participants
This PR refactors the
TensorNetworkdata type to include a reverse index map, and to be stricter about it's construction and how it's tensors can be set.This PR also adds
NormNetworktype, as a wrapper around aTensorNetwork, to represent the norm of a tensor network.