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] Fix issue with directsum involving EmptyStorage #1443

Merged
merged 9 commits into from
May 16, 2024

Conversation

kmp5VT
Copy link
Collaborator

@kmp5VT kmp5VT commented May 14, 2024

Description

In this I attempt to fix issue 1430 where there is an issue in directsum when an EmptyTensor is fed in as the first argument instead of the second. In tensor_algebra.jl the directsum code attempts to call one(EmptyNumber).

To fix this code I am modifying this function

function directsum_projectors(
  elt1::Type{<:Number}, elt2::Type{<:Number}, i::Index, j::Index, ij::Index
)
  D1 = zeros_itensor(elt1, dag(i), ij)
  D2 = zeros_itensor(elt1, dag(j), ij)
  directsum_projectors!(tensor(D1), tensor(D2))
  return D1, D2
end

I have 2 fix ideas, the first is slightly intrusive elt = promote_type(elt1, elt2) and use elt instead of elt1 in both D1 and D2. The second is less intrusive, it is

directsum_projectors(
  elt1::Type{<:EmptyNumber}, elt2::Type{<:Number}, i::Index, j::Index, ij::Index) = directsum_projector(elt2, elt1, i, j, ij) 

which is what I have implemented.

How Has This Been Tested?

I added some tests in the test_itensors.jl I test an empty tensor in front and in back.

@mtfishman mtfishman changed the title [NDTensors][Bug] Fix issue with directsum of EmptyTensors [ITensors] Fix issue with directsum of EmptyTensors May 14, 2024
@mtfishman
Copy link
Member

mtfishman commented May 15, 2024

I would prefer a solution based on elt = promote_type(elt1, elt2), it is more general (for example it doesn't require code logic that is hard-coded to EmptyNumber like the current solution) and promote_type(elt1, elt2) should be the resulting element type of the final tensor of the operation anyway.

As another corner case, what happens if both inputs are EmptyStorage with EmptyNumber, i.e.:

using ITensors
i, j, k = Index.(2, ("i", "j", "k"))
A = ITensor(i, j)
B = ITensor(i, k)
directsum(A => j, B => k)

? Probably the result should be EmptyStorage but it may be tricky to make that work with the current code logic. Something to keep in mind, but best to leave that for a future PR to keep this PR simpler and more focused.

@kmp5VT
Copy link
Collaborator Author

kmp5VT commented May 15, 2024

@mtfishman Would it be reasonable to have this function for now

directsum_projectors(
  elt1::Type{<:EmptyNumber}, elt2::Type{<:EmptyNumber}, i::Index, j::Index, ij::Index
) = error("It is not possible to call directsum on two tensors with element type EmptyNumber")

@mtfishman
Copy link
Member

mtfishman commented May 15, 2024

@mtfishman Would it be reasonable to have this function for now

directsum_projectors(
  elt1::Type{<:EmptyNumber}, elt2::Type{<:EmptyNumber}, i::Index, j::Index, ij::Index
) = error("It is not possible to call directsum on two tensors with element type EmptyNumber")

Sure, that sounds like a good idea. Maybe add a sentence like:

If you are inputting ITensors constructor like ITensor(i, j), try specifying the element type, e.g. ITensor(Float64, i, j), or filling them with zero value, e.g. ITensor(0.0, i, j).

@mtfishman mtfishman changed the title [ITensors] Fix issue with directsum of EmptyTensors [ITensors] Fix issue with directsum involving EmptyStorage May 16, 2024
@mtfishman mtfishman merged commit 9294874 into ITensor:main May 16, 2024
15 checks passed
@mtfishman
Copy link
Member

Thanks!

@kmp5VT kmp5VT deleted the kmp5/debug/issue_1430 branch May 16, 2024 16:52
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.

[NDTensors] [ITensors] [BUG] directsum with EmptyStorage
2 participants