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

Fix index dimension mismatch in TensorNetwork construction #219

Merged
merged 3 commits into from
Oct 11, 2024

Conversation

Todorbsc
Copy link
Contributor

Closes #155

@Todorbsc Todorbsc added the bug Something isn't working label Oct 11, 2024
@Todorbsc Todorbsc self-assigned this Oct 11, 2024
@Todorbsc
Copy link
Contributor Author

@jofrevalles @mofeing the dimension mismatch seems to be fixed with the verification I implemented in TensorNetwork construction. I "uncommented" the TN dimension mismatch construction test and now it raises the proper exception as well as I tested the Chain example that Jofre commented in the issue, what raises an error now:

julia> mps = Chain(State(), Open(), [rand(2, 3), rand(2, 6, 4), rand(2, 5)])
ERROR: DimensionMismatch: Index D has inconsistent dimension: [3, 6]
Stacktrace:
 [1] TensorNetwork(tensors::Vector{Tensor{Float64, N, A} where {N, A<:AbstractArray{Float64, N}}})
   @ Tenet ~/QuanticGit/Tenet.jl/src/TensorNetwork.jl:58
 [2] Chain(::State, boundary::Open, arrays::Vector{Array{Float64}}; order::Tuple{Symbol, Symbol, Symbol})
   @ Tenet ~/QuanticGit/Tenet.jl/src/Ansatz/Chain.jl:99
 [3] Chain(::State, boundary::Open, arrays::Vector{Array{Float64}})
   @ Tenet ~/QuanticGit/Tenet.jl/src/Ansatz/Chain.jl:63
 [4] top-level scope
   @ REPL[3]:1

Also, let me know if there's something else needed, specially with what you commented in the issue about the unsafe_region.

Copy link
Member

@jofrevalles jofrevalles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, nice! Since we already have tests for @unsafe_region, and these are passing, we can merge this!

@mofeing
Copy link
Member

mofeing commented Oct 11, 2024

Do you mind benchmarking with and without the safe guards / checks for some large TNs? Like 100-1000 tensors

src/TensorNetwork.jl Outdated Show resolved Hide resolved
Co-authored-by: Sergio Sánchez Ramírez <15837247+mofeing@users.noreply.github.com>
@Todorbsc
Copy link
Contributor Author

Todorbsc commented Oct 11, 2024

Do you mind benchmarking with and without the safe guards / checks for some large TNs? Like 100-1000 tensors

I benchmarked for rand(TensorNetwork, 1000, 3).

Without the indices safe region:

julia> @benchmark tn = rand(TensorNetwork, 1000, 3)
BenchmarkTools.Trial: 88 samples with 1 evaluation.
 Range (min  max):  17.460 ms  560.275 ms  ┊ GC (min  max):  0.00%  12.05%
 Time  (median):     25.241 ms               ┊ GC (median):     2.20%
 Time  (mean ± σ):   56.874 ms ±  82.945 ms  ┊ GC (mean ± σ):  22.67% ± 24.72%

  █▅▁                                                           
  █████▅▁▅▁▁▅▆▆▅▇▅▁▁▅▁▆▁▁▁▁▁▅▁▁▁▁▁▁▁▁▁▅▁▁▁▅▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▅ ▁
  17.5 ms       Histogram: log(frequency) by time       407 ms <

 Memory estimate: 25.09 MiB, allocs estimate: 64188.

With the indices safe region:

julia> @benchmark tn = rand(TensorNetwork, 1000, 3)
BenchmarkTools.Trial: 89 samples with 1 evaluation.
Range (min  max):  23.384 ms  505.261 ms  ┊ GC (min  max):  0.00%  15.44%
Time  (median):     30.132 ms               ┊ GC (median):     0.00%
Time  (mean ± σ):   61.098 ms ±  85.135 ms  ┊ GC (mean ± σ):  12.13% ± 14.75%

 █▃ ▁                                                          
 ████▇█▁▅▁▇▁▁▁▆▁▁▆▅▁▁▁▁▁▁▁▁▁▁▁▅▅▁▁▁▁▁▁▁▁▁▁▅▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▅ ▁
 23.4 ms       Histogram: log(frequency) by time       485 ms <

Memory estimate: 28.02 MiB, allocs estimate: 103174.

Correct me if I am wrong, but as regularity may influence the times, I would just look at the minimum time: Without the checks 17.460 ms vs. With the checks 23.384 ms.

@Todorbsc
Copy link
Contributor Author

@mofeing Another benchmark for rand(TensorNetwork, 200, 3).

Using checks:

julia> @benchmark tn = rand(TensorNetwork, 200, 3)
BenchmarkTools.Trial: 400 samples with 1 evaluation.
 Range (min  max):   2.218 ms  253.500 ms  ┊ GC (min  max):  0.00%  23.87%
 Time  (median):      3.163 ms               ┊ GC (median):     0.00%
 Time  (mean ± σ):   12.499 ms ±  28.164 ms  ┊ GC (mean ± σ):  31.62% ± 21.23%

  █▄▁                                                           
  ███▇▅▆▅▅▅▄▅▅▅▅▄▅▁▅▁▁▄▄▁▁▁▄▁▁▁▄▄▆▁▁▅▆▁▅▁▄▁▄▄▁▄▁▅▁▁▁▁▁▁▁▄▁▄▁▄▄ ▆
  2.22 ms       Histogram: log(frequency) by time       129 ms <

 Memory estimate: 2.80 MiB, allocs estimate: 19763.

Wiithout checks:

julia> @benchmark tn = rand(TensorNetwork, 200, 3)
BenchmarkTools.Trial: 353 samples with 1 evaluation.
Range (min  max):   1.240 ms    1.031 s  ┊ GC (min  max):  0.00%   0.32%
Time  (median):      2.028 ms              ┊ GC (median):     0.00%
Time  (mean ± σ):   14.150 ms ± 65.195 ms  ┊ GC (mean ± σ):  30.34% ± 22.77%

 █                                                            
 ██▅▆▄▅▅▁▁▄▁▁▁▁▄▅▅▅▄▆▄▆▁▁▁▄▁▄▁▁▁▁▄▁▁▁▁▁▄▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▄ ▅
 1.24 ms      Histogram: log(frequency) by time       222 ms <

Memory estimate: 2.84 MiB, allocs estimate: 11968.

Min. time: Checks 2.218 ms vs. No checks 1.240 ms.

@mofeing
Copy link
Member

mofeing commented Oct 11, 2024

Thanks!

Correct me if I am wrong, but as regularity may influence the times, I would just look at the minimum time: Without the checks 17.460 ms vs. With the checks 23.384 ms.

Well, the min time can also be affected by jitter so I would take the mean which means there is a 20% increase in runtime... but it's still 25ms which is ok.

@mofeing mofeing merged commit b94af1c into master Oct 11, 2024
6 checks passed
@mofeing mofeing deleted the fix/tn_dimension_mismatch branch October 11, 2024 15:48
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 this pull request may close these issues.

Quantum constructor does not error for inconsistent dimensions in tensor networks
3 participants