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.

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

replace(!)(::Chain, tensor => newtensor) only works for tensors of the same size #191

Closed
starsfordummies opened this issue Aug 20, 2024 · 5 comments · Fixed by #201
Closed
Labels
bug Something isn't working

Comments

@starsfordummies
Copy link
Contributor

Is this intentional? in principle in a sweep we might want to update MPS tensors on the fly with inconsistent indices.
Or am I doing it wrong?

MWE

using Tenet
psi = rand(Chain, Open, State; n=5, χ=8)
i2 = inds(tensors(psi, at=Site(2)))
t2n = Tensor(rand( size(tensors(psi,at=Site(2)))... ), i2)
replace(psi, tensors(psi,at=Site(2)) => t2n)   # works 

but

t2n_alt = Tensor(rand(2,2,10), i2)
replace(psi, tensors(psi,at=Site(2)) => t2n_alt)   # throws 

ERROR: DimensionMismatch: size(tensor,##236)=10 but should be equal to size(tn,##236)=4
Stacktrace:
 [1] push!(tn::TensorNetwork, tensor::Tensor{Float64, 3, Array{Float64, 3}})
   @ Tenet ~/.julia/dev/Tenet/src/TensorNetwork.jl:538
 [2] replace!
   @ ~/.julia/dev/Tenet/src/TensorNetwork.jl:228 [inlined]
 [3] replace!
   @ ~/.julia/dev/Tenet/src/TensorNetwork.jl:163 [inlined]
 [4] replace(tn::Chain, old_new::Tuple{Pair{Tensor{Float64, 3, Array{Float64, 3}}, Tensor{Float64, 3, Array{Float64, 3}}}})
   @ Tenet ~/.julia/dev/Tenet/src/TensorNetwork.jl:169
 [5] replace(tn::Chain, old_new::Pair{Tensor{Float64, 3, Array{Float64, 3}}, Tensor{Float64, 3, Array{Float64, 3}}})
   @ Tenet ~/.julia/dev/Tenet/src/TensorNetwork.jl:168
 [6] top-level scope
   @ REPL[10]:1
@mofeing
Copy link
Member

mofeing commented Aug 21, 2024

This is currently intentional: Indices should have the same size on replacement. But you could try using @unsafe_region macro; it makes push! skip index size check until the end of the macro region.

@unsafe_region psi begin
    # put here your replacements
end

@starsfordummies
Copy link
Contributor Author

oki, that kind of works: it does the substitution, but still prints an error - this could be misleading, I'd probably remove that..

julia> size(t2n_alt)
(2, 2, 1)

julia> inds(tensors(psi)[2]) == inds(t2n_alt)
true

julia> size.(tensors(psi))
5-element Vector{Tuple{Int64, Int64, Vararg{Int64}}}:
 (2, 2)
 (2, 2, 4)
 (4, 2, 4)
 (4, 2, 2)
 (2, 2)

julia> Tenet.@unsafe_region psi begin
       replace!(psi, tensors(psi,at=Site(2)) => t2n_alt)
       end
ERROR: DimensionMismatch: Inconsistent size of indices
Stacktrace:
 [1] top-level scope
   @ ~/.julia/dev/Tenet/src/TensorNetwork.jl:518

julia> size.(tensors(psi))
5-element Vector{Tuple{Int64, Int64, Vararg{Int64}}}:
 (2, 2)
 (2, 2, 1)  
 (4, 2, 4)
 (4, 2, 2)
 (2, 2)

By the way, there's an even more worrying behavior of replace!: in case of error without unsafe_region, it doesn't add the new tensor but it seems that it does delete the old one, seems quite dangerous...

julia> size.(tensors(psi))
5-element Vector{Tuple{Int64, Int64, Vararg{Int64}}}:
 (2, 2)
 (2, 2, 4)
 (4, 2, 4)
 (4, 2, 2)
 (2, 2)

julia> replace!(psi, tensors(psi,at=Site(2)) => t2n_alt)
ERROR: DimensionMismatch: size(tensor,##286)=1 but should be equal to size(tn,##286)=4
Stacktrace:
 [1] push!(tn::TensorNetwork, tensor::Tensor{Float64, 3, Array{Float64, 3}})
   @ Tenet ~/.julia/dev/Tenet/src/TensorNetwork.jl:538
 [2] replace!(tn::Chain, pair::Pair{Tensor{Float64, 3, Array{Float64, 3}}, Tensor{Float64, 3, Array{Float64, 3}}})
   @ Tenet ~/.julia/dev/Tenet/src/TensorNetwork.jl:228
 [3] top-level scope
   @ REPL[93]:1

julia> size.(tensors(psi))
4-element Vector{Tuple{Int64, Int64, Vararg{Int64}}}:
 (2, 2)
 (4, 2, 4)
 (4, 2, 2)
 (2, 2)

@mofeing
Copy link
Member

mofeing commented Sep 4, 2024

About the first error, it fails because @unsafe_region just postpones the check, but you need to replace all the tensors with the index which you have modified the size.

The second error is normal, because what you made in @unsafe_region is... unsafe. That's why is not generally allowed, but we added this macro to have exceptions. But as said, that is unsafe and you have to be careful.

@starsfordummies
Copy link
Contributor Author

No, what I'm saying in the second comment is happening without unsafe_region, the replace method is throwing an error but still doing something destructive (which is not what I would naively expect, it should throw an error and do nothing):

julia>  using Tenet
       psi = rand(Chain, Open, State; n=5, χ=8)
       i2 = inds(tensors(psi, at=Site(2)))
       t2n_alt = Tensor(rand(2,2,10), i2)
       size.(tensors(psi))
5-element Vector{Tuple{Int64, Int64, Vararg{Int64}}}:
 (2, 2)
 (2, 2, 4)
 (4, 2, 4)
 (4, 2, 2)
 (2, 2)

julia> replace!(psi, tensors(psi,at=Site(2)) => t2n_alt)
ERROR: DimensionMismatch: size(tensor,G)=10 but should be equal to size(tn,G)=4
Stacktrace:
 [1] push!(tn::TensorNetwork, tensor::Tensor{Float64, 3, Array{Float64, 3}})
   @ Tenet ~/.julia/dev/Tenet/src/TensorNetwork.jl:537
 [2] replace!(tn::Chain, pair::Pair{Tensor{Float64, 3, Array{Float64, 3}}, Tensor{Float64, 3, Array{Float64, 3}}})
   @ Tenet ~/.julia/dev/Tenet/src/TensorNetwork.jl:228
 [3] top-level scope
   @ REPL[65]:1

julia> size.(tensors(psi))
4-element Vector{Tuple{Int64, Int64, Vararg{Int64}}}:
 (2, 2)
 (4, 2, 4)
 (4, 2, 2)
 (2, 2)

see what I mean ?

@mofeing mofeing added the bug Something isn't working label Sep 4, 2024
@mofeing
Copy link
Member

mofeing commented Sep 4, 2024

Ahh okay. That's clearly a bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants