-
Notifications
You must be signed in to change notification settings - Fork 13
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
Support MPO-(MPS with dangling site indices) contraction #46
Comments
Yes, I think we should remove any automated index replacement and leave it up to the user to provide a proper
EDIT: Sorry, I lost track and thought we were talking about |
Finally, in the signature of the function I don't think we should specialize Instead I think it should more generally be defined as: function ITensors.contract(
::ITensors.Algorithm"fit",
A::IsTree,
B::IsTree;
init_guess=[...], # Determine an initial guess by analyzing the external indices of `A * B`
kwargs...,
)
[...]
end where: const IsTree = Union{MPS,MPO,TTNS,TTNO} to make it as general as possible. Going forward, we might make this into a trait, and also just get ride of EDIT: In #47 I went ahead and removed the |
Agreed with your comments & suggestions. Should we set a default value to Candidate 1 is backward compatible. Candidate 2 is explicit. In addition, we could write a separate function for generating a random initial guess with a given global bond dimension from Candidate 1:
Candidate 2:
|
@shinaoka I don't think using I don't think there is any particular challenge to analyzing I think like for DMRG we should have the users specify an initial guess unless we come up with something good that generally works. I know this algorithm has a tendency to get stuck in local minima so the initial guess is particular important. Something to consider would be to use a cheaper algorithm for contracting the TTNs approximately, such as the zip-up algorithm. But we would have to test that out to decide if it is a good default in general. |
Also, we've decided that for now we won't explicitly support accepting function ITensors.contract(
::ITensors.Algorithm"fit",
A::AbstractTTN,
B::AbstractTTN;
init_guess,
kwargs...,
)
# [...]
end Users can convert their |
Thank you for letting me know about the decision! I will test the code once you make changes throughout the package. |
@shinaoka in #50 I changed this function contract(
::Algorithm"fit",
tn1::AbstractTTN,
tn2::AbstractTTN;
init,
kwargs...,
)
# [...]
end which helps us move towards not having a distinction between states and operators. Unfortunately that function now seems to be broken, which I think happened when we originally generalized the solver functions from MPO/MPS to TTNs in #44. If you have time, it would be very helpful if you could investigate why it isn't working! |
Thank you! It seems to be working on my side. Do you have any example code? nbit = 3
sites = siteinds("Qubit", nbit)
M1 = replaceprime(randomMPO(sites) + randomMPO(sites), 1=>2, 0=>1)
M2 = randomMPO(sites) + randomMPO(sites)
M12_ref = contract(M1, M2; alg="naive")
t12_ref = TreeTensorNetwork([M12_ref[v] for v in eachindex(M12_ref)])
t1 = TreeTensorNetwork([M1[v] for v in eachindex(M1)])
t2 = TreeTensorNetwork([M2[v] for v in eachindex(M2)])
t12 = contract(t1, t2; alg="fit", init=t12_ref)
#t12 = contract(t1, t2; alg="fit")
function toarr(t)
arr = reduce(*, [t[v] for v in vertices(t)])
return Array(arr, vcat(sites, sites''))
end
@show t12 ≈ t12_ref
@show toarr(t12) ≈ toarr(t12_ref) |
Turns out there were some issues in the tests due to a bug in |
I see. So, shall we add the above code to the test? |
@shinaoka that seems like a reasonable test to add. Does it seem like the current |
I made a fresh start after #44 was merged into the main branch.
For QTT, we may want to contract an MPO (with two site indices at each vertex) and an MPS with dangling site indices. I found several issues, but let me sout them out one by one. The first issue is the following.
A minimum example for
naive
algoritm works:fitting
algorithm does not work:The replacement of site indices does not work with an MPS with dangling site indices. Shall we just remove this part?
https://github.com/mtfishman/ITensorNetworks.jl/blob/main/src/treetensornetworks/solvers/contract_operator_state.jl#L38-L42
If we move those lines, it will be user's responsibility to provide an appropriate
init_state
with the same site indices asH * psi0
. The following default value assumes thatinit_state
lives in the same space aspsi0
, which is the case ifH
is a time-evoluation operator. For a more general MPO, this default value is not appropriate. What do you think about removing these lines for the replacement and adding a check on the site indces oninit_state
?https://github.com/mtfishman/ITensorNetworks.jl/blob/main/src/treetensornetworks/solvers/contract_operator_state.jl#L17
The text was updated successfully, but these errors were encountered: