-
Notifications
You must be signed in to change notification settings - Fork 43
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
Add DiagonalTensorMap constructors and converters #212
Conversation
I don't think that this is fully correct. We really need the data to be associated to the fusiontrees, because their order within the blocks changed between versions. As a result, simply loading in the blocks would yield permuted data, which is definitely not what you want. |
So, summarizing my thoughts a bit here: For the functions, I would say the best approach is:
For the best practice on saving and loading states, I'm actually a bit torn. It seems to me that JLD2 has actually come quite a long way, and is quite good at supporting arbitrary datatypes. Additionally, after reading through the documentation a bit more, there are several features that could make our lives a lot easier:
The combination of these two could mean that we simply define something like: struct TensorData_v1
# stable data format
end and store tensors simply by using JLD2's automatic serialization. Then, since the custom serialization is stable, we can load in tensors from any version by implementing the correct deserializer. Additionally, if we would ever like to go to a different data format, we could just add struct TensorData_v2 end and remap the types on load. |
Co-authored-by: Lukas Devos <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #212 +/- ##
==========================================
- Coverage 82.25% 82.22% -0.03%
==========================================
Files 43 43
Lines 5433 5447 +14
==========================================
+ Hits 4469 4479 +10
- Misses 964 968 +4 ☔ View full report in Codecov by Sentry. |
I think this is good to go from my end; the failure on nightly seems unrelated. @Yue-Zhengyuan , anything that you still like to see changed? |
I'm OK to fix in another PR about transferring tensors from old to new TensorKit. |
I seem to have missed this, but what is wrong with the description of how to transfer tensors from TensorKit prior to v0.13 to newer versions as currently in the README? I've restored the README because the updated description you wrote was incorrect, as shown by the example in #211 . Is there anything else that you would like to see in this regard? |
Example: first, run the following in environment with v0.12.7 using TensorKit
using JLD2
function save_tensor_tmp(filename::AbstractString, t::AbstractTensorMap)
t_dict = Dict(
:space => space(t),
:data => Dict((f₁, f₂) => t[f₁, f₂] for (f₁, f₂) in fusiontrees(t))
)
jldsave(filename; t_dict)
return nothing
end
V1, V2 = ℂ^2, ℂ^3
a = TensorMap(randn, Float64, V1 ← V2);
println(fusiontrees(a)) # output: ((nothing, nothing),)
save_tensor_tmp("tmp.jld2", a) Next, run the following in environment with v0.14.3 using TensorKit
using JLD2
function load_tensor_tmp(filename::AbstractString)
t_dict = JLD2.load_object(filename) # I replaced `jldload` with `load_object`
T = eltype(valtype(t_dict[:data]))
t = TensorMap{T}(undef, t_dict[:space])
for ((f₁, f₂), val) in t_dict[:data]
t[f₁, f₂] .= val
end
return t
end
a = load_tensor_tmp("tmp.jld2"); Error message:
This is because in v0.14.3, the a = randn(ℂ^2 ← ℂ^3)
fusiontrees(a)
#= output:
1-element Vector{Tuple{FusionTree{Trivial, 1, 0, 0}, FusionTree{Trivial, 1, 0, 0}}}:
(FusionTree{Trivial}((Trivial(),), Trivial(), (false,), ()), FusionTree{Trivial}((Trivial(),), Trivial(), (false,), ()))
=# There are also problems with other sector types. For example, for an SU(2) tensor, the error message is
|
Ok, I surely had missed/forgotten that part of the issue. My apologies and thanks for laying it out so clearly. It would indeed be good to fix this in a separate PR. I will merge this one and think about possible solutions. |
Changes:
the old data is now specified by.blocks
instead offusiontrees