bsc-quantic / Tenet.jl

Composable Tensor Network library in Julia
https://bsc-quantic.github.io/Tenet.jl/
Apache License 2.0
17 stars 1 forks source link

Fix chain rules for contraction #39

Closed jofrevalles closed 1 year ago

jofrevalles commented 1 year ago

Summary

The previous code failed the test_rrule and test_frule for complex numbers (#38). In this PR we address this issue and fix the chain rrule for contraction. We also added rrule and frule tests for the contraction of Tensors with complex values.

Extra

The code now is not fully functional, and I believe that is because we have to somehow introduce the ChainRulesCore.ProjectTo for the adjoint Tensor types. I think that is the reason why or return a Matrix instead of a Tensor sometimes:

julia> @testset "[number-tensor product]" begin
           b = Tensor(rand(2, 2), (:i, :j))
           z = 1.0 + 1im

           test_frule(contract, 5.0, b)
           test_rrule(contract, 5.0, b)
       end
test_rrule: contract on Float64,Tensor{Float64, 2, Matrix{Float64}}: Error During Test at /home/jofrevalles/.julia/packages/ChainRulesTestUtils/lERVj/src/testers.jl:202
  Got exception outside of a @test
  MethodError: no method matching (::ProjectTo{Float64, NamedTuple{(), Tuple{}}})(::Matrix{Float64})
  Closest candidates are:
    (::ProjectTo)(::Thunk) at ~/.julia/packages/ChainRulesCore/a4mIA/src/projection.jl:124
    (::ProjectTo{<:Number})(::Tangent{<:Complex}) at ~/.julia/packages/ChainRulesCore/a4mIA/src/projection.jl:192
    (::ProjectTo{<:Number})(::Tangent{<:Number}) at ~/.julia/packages/ChainRulesCore/a4mIA/src/projection.jl:193
    ...
  Stacktrace:
    [1] (::Tenet.var"#contract_pullback#173"{Float64, Tensor{Float64, 2, Matrix{Float64}}, Float64, Tensor{Float64, 2, Matrix{Float64}}, ProjectTo{Tensor{Float64, 2, Matrix{Float64}}, NamedTuple{(:data, :labels, :meta), Tuple{ProjectTo{AbstractArray, NamedTuple{(:element, :axes), Tuple{ProjectTo{Float64, NamedTuple{(), Tuple{}}}, Tuple{Base.OneTo{Int64}, Base.OneTo{Int64}}}}}, Tuple{Symbol, Symbol}, Dict{Symbol, Any}}}}, ProjectTo{Float64, NamedTuple{(), Tuple{}}}})(c̄::Tensor{Float64, 2, Matrix{Float64}})
      @ Tenet ~/git/Tenet.jl/src/Differentiation.jl:57
    ...
Test Summary:                                                         | Pass  Error  Total  Time
[number-tensor product]                                               |    5      1      6  0.9s
  test_frule: contract on Float64,Tensor{Float64, 2, Matrix{Float64}} |    4             4  0.8s
  test_rrule: contract on Float64,Tensor{Float64, 2, Matrix{Float64}} |    1      1      2  0.0s
ERROR: Some tests did not pass: 5 passed, 0 failed, 1 errored, 0 broken.
codecov[bot] commented 1 year ago

Codecov Report

Merging #39 (ca6a81e) into master (0bbd73d) will increase coverage by 0.00%. The diff coverage is 0.00%.

@@          Coverage Diff           @@
##           master     #39   +/-   ##
======================================
  Coverage    0.38%   0.39%           
======================================
  Files          13      13           
  Lines         772     769    -3     
======================================
  Hits            3       3           
+ Misses        769     766    -3     
Impacted Files Coverage Δ
src/Differentiation.jl 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

jofrevalles commented 1 year ago

@mofeing I was thinking that here we can not do the adjoint of the tensors in the rrules, since this is not general for N>2-dimensional Tensors. Should this maybe be something else?

mofeing commented 1 year ago

mmm adjoint should only be applied to Tensors contained in a Quantum TN. As such, there is a "directionality" sense in the TN. I see two solutions:

Right now you can leave it like we talked so it works for Tensors with real numbers and we can continue discussing.

jofrevalles commented 1 year ago

Okay, so right now it will work with rank-2 tensors. Still, we should consider adding the conjugate for larger dimensional tensors in the future (or maybe just raising an error), since the rrule will not work for (N>2)-dimensional tensors.

mofeing commented 1 year ago

If it's just for the rrule, you can call conj inside the rrule instead of calling adjoint.

jofrevalles commented 1 year ago

Okay, good idea, let's do that.

jofrevalles commented 1 year ago

Cool! Now our tests are not failing anymore. We will have to fix some that are currently commented out for the contraction of TensorNetworks, but the Tensor type the chain rules are now working properly. @mofeing, Can I close the issue #39 and start one for the contraction of TensorNetworks?