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

[ITensors] OpSum to MPO conversion type stability #1184

Closed
wants to merge 7 commits into from

Conversation

mtfishman
Copy link
Member

Description

Supersedes #1183, with some additional code reorganization and cleanup.

src/physics/autompo/opsum_to_mpo.jl Outdated Show resolved Hide resolved
src/physics/autompo/opsum_to_mpo_generic.jl Outdated Show resolved Hide resolved
src/physics/autompo/opsum_to_mpo_qn.jl Outdated Show resolved Hide resolved
mtfishman and others added 3 commits August 25, 2023 15:58
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@mtfishman mtfishman changed the title [ITensors] OpSum to MPO conversion type stability [ITensors] OpSum to MPO conversion type stability Aug 25, 2023
@mtfishman
Copy link
Member Author

[test ITensors mps]

@github-actions
Copy link
Contributor

Run ITensors mps tests from comment trigger: failed ❌
https://github.com/ITensor/ITensors.jl/actions/runs/5980089302

1 similar comment
@github-actions
Copy link
Contributor

Run ITensors mps tests from comment trigger: failed ❌
https://github.com/ITensor/ITensors.jl/actions/runs/5980089302

@mtfishman
Copy link
Member Author

[test ITensors mps]

@github-actions
Copy link
Contributor

Run ITensors mps tests from comment trigger: failed ❌
https://github.com/ITensor/ITensors.jl/actions/runs/5980485940

1 similar comment
@github-actions
Copy link
Contributor

Run ITensors mps tests from comment trigger: failed ❌
https://github.com/ITensor/ITensors.jl/actions/runs/5980485940

@terasakisatoshi
Copy link
Contributor

Hi @mtfishman, thank you for feedback in my PR in #1183

Here is what I did on your ITensors_opsum_type_stability branch 8edfa07

ITensors.jl on  ITensors_opsum_type_stability [?] is 📦 v0.3.41 via ஃ v1.9.3 via 🐍 v3.10.8 git show --format='%h' --no-patch
bbecc8b6c

ITensors.jl on  ITensors_opsum_type_stability [?] is 📦 v0.3.41 via ஃ v1.9.3 via 🐍 v3.10.8 cat mpo_bench.jl                                                 
using ITensors

include("paulis.jl")
conserve_qns = false
@info "conserve_qns" conserve_qns
sites = siteinds("Qubit", N; conserve_qns)
os = ITensors.OpSum()

for p in paulis
  coeff = rand()
  global os
  os += ITensors.Ops.op_term((coeff, p...))
end

@time MPO(os, sites)
@time MPO(os, sites)
@time MPO(os, sites)

ITensors.jl on  ITensors_opsum_type_stability [?] is 📦 v0.3.41 via ஃ v1.9.3 via 🐍 v3.10.8 julia mpo_bench.jl 
┌ Info: conserve_qns
└   conserve_qns = false
 28.424578 seconds (199.12 M allocations: 10.196 GiB, 6.61% gc time, 46.17% compilation time)
 14.791576 seconds (168.27 M allocations: 8.186 GiB, 7.82% gc time)
 15.261162 seconds (168.27 M allocations: 8.186 GiB, 7.64% gc time)

My pull request #1183 is getting better results:

ITensors.jl on  improve-type-stability-of-svdMPO [?] is 📦 v0.3.41 via ஃ v1.9.3 via 🐍 v3.10.8 took 2m51s 
❯ julia mpo_bench.jl 
┌ Info: conserve_qns
└   conserve_qns = false
 22.455729 seconds (31.28 M allocations: 7.094 GiB, 5.38% gc time, 55.11% compilation time)
 10.126017 seconds (3.96 M allocations: 5.315 GiB, 8.45% gc time)
 10.914117 seconds (3.96 M allocations: 5.315 GiB, 6.65% gc time)

versioninfo() outputs the following information:

julia> versioninfo()
Julia Version 1.9.3
Commit bed2cd540a1 (2023-08-24 14:43 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: macOS (x86_64-apple-darwin22.4.0)
  CPU: 16 × Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-14.0.6 (ORCJIT, skylake)
  Threads: 1 on 16 virtual cores
Environment:
  JULIA_PROJECT = @.
  JULIA_EDITOR = code

Unfortunately, I get an error when conserve_qns is true:

ITensors.jl on  ITensors_opsum_type_stability [?] is 📦 v0.3.41 via ஃ v1.9.3 via 🐍 v3.10.8 git show --format='%h' --no-patch
bbecc8b6c

ITensors.jl on  ITensors_opsum_type_stability [?] is 📦 v0.3.41 via ஃ v1.9.3 via 🐍 v3.10.8 cat mpo_bench.jl                 
using ITensors

include("paulis.jl")
conserve_qns = true
@info "conserve_qns" conserve_qns
sites = siteinds("Qubit", N; conserve_qns)
os = ITensors.OpSum()

for p in paulis
  coeff = rand()
  global os
  os += ITensors.Ops.op_term((coeff, p...))
end

@time MPO(os, sites)
@time MPO(os, sites)
@time MPO(os, sites)

ITensors.jl on  ITensors_opsum_type_stability [?] is 📦 v0.3.41 via ஃ v1.9.3 via 🐍 v3.10.8 julia mpo_bench.jl               
┌ Info: conserve_qns
└   conserve_qns = true
ERROR: LoadError: BoundsError: attempt to access 1×1 Matrix{Float64} at index [1, 2]
Stacktrace:
 [1] getindex(::Matrix{Float64}, ::Int64, ::Int64)
   @ Base ./essentials.jl:14
 [2] svd_mpo(coefficient_type::Type, os::Sum{Scaled{ComplexF64, Prod{Op}}}, sites::Vector{Index{Vector{Pair{QN, Int64}}}}; mindim::Int64, maxdim::Int64, cutoff::Float64)
   @ ITensors ~/work/fork/ITensors.jl/src/physics/autompo/opsum_to_mpo_qn.jl:134
 [3] svd_mpo
   @ ~/work/fork/ITensors.jl/src/physics/autompo/opsum_to_mpo_qn.jl:1 [inlined]
 [4] mpo_specified_coefficient_type(coefficient_type::Type, os::Sum{Scaled{ComplexF64, Prod{Op}}}, sites::Vector{Index{Vector{Pair{QN, Int64}}}}; splitblocks::Bool, kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
   @ ITensors ~/work/fork/ITensors.jl/src/physics/autompo/opsum_to_mpo_generic.jl:311
 [5] mpo_specified_coefficient_type
   @ ~/work/fork/ITensors.jl/src/physics/autompo/opsum_to_mpo_generic.jl:304 [inlined]
 [6] MPO(os::Sum{Scaled{ComplexF64, Prod{Op}}}, sites::Vector{Index{Vector{Pair{QN, Int64}}}}; kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
   @ ITensors ~/work/fork/ITensors.jl/src/physics/autompo/opsum_to_mpo_generic.jl:301
 [7] MPO(os::Sum{Scaled{ComplexF64, Prod{Op}}}, sites::Vector{Index{Vector{Pair{QN, Int64}}}})
   @ ITensors ~/work/fork/ITensors.jl/src/physics/autompo/opsum_to_mpo_generic.jl:300
 [8] top-level scope
   @ ./timing.jl:273
in expression starting at /Users/terasakisatoshi/work/fork/ITensors.jl/mpo_bench.jl:15

@terasakisatoshi
Copy link
Contributor

The code below is a MWE that reproduces the error above.

using ITensors

N=4
paulis = [
    ["Z",1],
    ["Z",1,"Z",4],
    ["Z",2,"Z",3],
]

conserve_qns = true
@info "conserve_qns" conserve_qns
sites = siteinds("Qubit", N; conserve_qns)
os = ITensors.OpSum()

for p in paulis
  coeff = rand()
  global os
  os += ITensors.Ops.op_term((coeff, p...))
end

@time MPO(os, sites)

@mtfishman
Copy link
Member Author

Thanks for checking @terasakisatoshi. I also noticed that bug. Let's just go ahead with your more minimal PR for now. I'll make some comments over there.

@mtfishman mtfishman closed this Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants