Skip to content

Conversation

ogauthe
Copy link
Collaborator

@ogauthe ogauthe commented Feb 24, 2025

This PR adds extensive test for contract with BlockArray and BlockedArray as well as non-symmetric BlockSparseArray. There are several errors that need to be fixed.

@codecov
Copy link

codecov bot commented Feb 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.95%. Comparing base (9407722) to head (1a65744).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #34   +/-   ##
=======================================
  Coverage   93.95%   93.95%           
=======================================
  Files          15       15           
  Lines         364      364           
=======================================
  Hits          342      342           
  Misses         22       22           
Flag Coverage Δ
docs 0.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ogauthe
Copy link
Collaborator Author

ogauthe commented Feb 24, 2025

TBD: output type of full contraction (vector-vector). BlockSparseArray currently returns BlockSparseArray{elt,0} array while Array, BlockArray and BlockedArray return a Array{elt,0}. Maybe it is not worth it to enforce a given array type here and relax the test.

I believe all the other ones are legitimate and should pass at some point.

@ogauthe ogauthe marked this pull request as ready for review February 25, 2025 15:47
@mtfishman
Copy link
Member

mtfishman commented Feb 25, 2025

TBD: output type of full contraction (vector-vector). BlockSparseArray currently returns BlockSparseArray{elt,0} array while Array, BlockArray and BlockedArray return a Array{elt,0}. Maybe it is not worth it to enforce a given array type here and relax the test.

I would think the output type should be consistent with other contractions between those types of arrays.

@ogauthe
Copy link
Collaborator Author

ogauthe commented Feb 25, 2025

TBD: output type of full contraction (vector-vector). BlockSparseArray currently returns BlockSparseArray{elt,0} array while Array, BlockArray and BlockedArray return a Array{elt,0}. Maybe it is not worth it to enforce a given array type here and relax the test.

I would think the output type should be consistent with other contractions between those types of arrays.

The question is what should be the output of

v = BlockedArray(ones(4,), [2,2])
contract((), v, (1,), v, (1,)) isa Array{Float64, 0}  # currently true
contract((), v, (1,), v, (1,)) isa BlockedArray  # currently false

Currently it is a Array{Float64,0}, same for a BlockArray.

There are several possibilities for the output:

  • treat full contraction as a special case that directly returns a scalar (as v' * v and v ⋅ v return)
  • accept 0-dim as a special case that may relax type constraint and keep the current Array{Float64,0}.
  • impose same wrapper type as for non-full contraction and return a BlockedArray{Float64, 0}

The tests introduced in this PR expect the last option and are labelled as broken as the current type is not BlockedArray. We would then need to fix these tests.

@mtfishman
Copy link
Member

I see, I would expect it to output the same type as non-full contraction (your third option).

@mtfishman mtfishman merged commit f67770f into ITensor:main Feb 25, 2025
12 checks passed
@lkdvos
Copy link
Contributor

lkdvos commented Feb 25, 2025

I might be a little late to the party here, but an argument in favor of the third option: if you have a contraction of a network which is the outer product of a full contraction and a different tensor, you would want that to conserve the type, which is easier if you retain BlockedArray{T,0}. In einsum notation, subtracting the overlap onto a given direction could be written as: A[i,j] -= A[k,l] * B[k, l] * B[i,j] where you would probably prefer the final contraction to be BlockedArray{T,2} * BlockedArray{T,0}.

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.

3 participants