Skip to content
This repository has been archived by the owner on Sep 12, 2023. It is now read-only.

Enhance contract to support multiple input Tensors #31

Merged
merged 3 commits into from
Jul 12, 2023

Conversation

jofrevalles
Copy link
Member

This PR enhances the contract function for multiple tensors, which resolves issue #16 in EinExprs. The function utilizes reduce to perform pairwise Tensor contractions.
To ensure the robustness of these changes we added a testset that covers the new contract functionality, checking its behavior with various tensors in inputs.

@codecov
Copy link

codecov bot commented Jul 4, 2023

Codecov Report

Merging #31 (c6c1312) into master (5c2dae2) will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #31      +/-   ##
==========================================
+ Coverage   82.05%   82.12%   +0.07%     
==========================================
  Files           6        6              
  Lines         234      235       +1     
==========================================
+ Hits          192      193       +1     
  Misses         42       42              
Impacted Files Coverage Δ
src/Numerics.jl 87.30% <100.00%> (+0.20%) ⬆️

@jofrevalles jofrevalles requested a review from mofeing July 4, 2023 09:53
Copy link
Member

@mofeing mofeing left a comment

Choose a reason for hiding this comment

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

I prefer not to merge this PR. Reasons is that if you need to contract several Tensors while in a EinExpr, it's better to create a new EinExpr level.

Also, what if you have 3 tensors with 1 index? This implementation wouldn't work because the index would disappear after the first contraction.

In my opinion, it is semantically ambiguous what you want to achieve with contract(tensors...).

@jofrevalles
Copy link
Member Author

The problem is that EinExprs does not return only pairwise Tensors to be contracted, and that leads to error. You can check the issue I mentioned above. I think that this extension of the contract function simply fixed those problems.

@mofeing
Copy link
Member

mofeing commented Jul 5, 2023

The problem is that EinExprs does not return only pairwise Tensors to be contracted, and that leads to error. You can check the issue I mentioned above. I think that this extension of the contract function simply fixed those problems.

Yep, but a optimization pass can be written such that

EinExpr(a,b,c,out=labels)

gets transformed to

EinExpr(EinExpr(a,b,out=labels_ab),c,out=labels)

@mofeing
Copy link
Member

mofeing commented Jul 6, 2023

Yep, but a optimization pass can be written such that

EinExpr(a,b,c,out=labels)

gets transformed to

EinExpr(EinExpr(a,b,out=labels_ab),c,out=labels)

@jofrevalles Actually, we don't need to write any other optimization pass: this should be currently done by just calling einexpr(EinExpr([a,b,c], out=labels), optimizer=Exhaustive()).

I mean, just calling einexpr on the EinExpr that you want to "optimize".

@jofrevalles
Copy link
Member Author

@jofrevalles Actually, we don't need to write any other optimization pass: this should be currently done by just calling einexpr(EinExpr([a,b,c], out=labels), optimizer=Exhaustive()).

I mean, just calling einexpr on the EinExpr that you want to "optimize".

It should, but this doesn't work for unconnected tensors, since einexpr does not return them two-by-two:

julia> tensor_vector = [
                  Tensor(rand(2, 2), (:i, :j)),
                  Tensor(rand(2, 2), (:k, :l)),
                  Tensor(rand(2, 2), (:m, :n))]
3-element Vector{Tensor{Float64, 2, Matrix{Float64}}}:
...

julia> einexpr(Exhaustive, EinExpr(tensor_vector)) |> contract
ERROR: MethodError: no method matching contract(::Tensor{Float64, 2, Matrix{Float64}}, ::Tensor{Float64, 2, Matrix{Float64}}, ::Tensor{Float64, 2, Matrix{Float64}}; dims::Vector{Symbol})

Closest candidates are:
  contract(::Tensor, ::Tensor; dims)
   @ Tensors ~/.julia/packages/Tensors/O9RAx/src/Numerics.jl:36
  contract(::Tensor; dims)
   @ Tensors ~/.julia/packages/Tensors/O9RAx/src/Numerics.jl:49
  contract(::Tensor, ::TensorNetwork; kwargs...)
   @ Tenet ~/git/Tenet.jl/src/TensorNetwork.jl:480
  ...

That's why I think that in this case it would be useful to extend contract.

@mofeing
Copy link
Member

mofeing commented Jul 12, 2023

Okay, I'm gonna approve this but keep in mind that this is subject to changes in the future.

@mofeing mofeing merged commit abf4f47 into master Jul 12, 2023
@mofeing mofeing deleted the feature/contraction-multiple-tensors branch July 12, 2023 16:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue with contract handling EinExpr with multiple unconnected tensors
2 participants