-
Notifications
You must be signed in to change notification settings - Fork 23
Fix FormNetwork constructor bug #238
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
Conversation
|
Thanks, Joey. Looks good & this reordering of indices fixed the fermion bug I had. |
| function itensor_identity_map(i_pairs::Vector) | ||
| return prod(i_pairs; init=ITensor(one(Bool))) do i_pair | ||
| return delta(Bool, dag(first(i_pair)), last(i_pair)) | ||
| return denseblocks(delta(last(i_pair), dag(first(i_pair)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I merged prematurely. I think this should be:
| return denseblocks(delta(last(i_pair), dag(first(i_pair)))) | |
| return denseblocks(delta(Bool, last(i_pair), dag(first(i_pair)))) |
Otherwise, delta will make a tensor with element type Float64, which will then promote other tensors to double precision, which is an issue on GPU where it is generally better to use single precision. @JoeyT1994 can you make a new PR adding back Bool, or did you see some issue with that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mtfishman This introduces bugs to me that I don't understand. For instance
g = named_grid((4,))
s1, s2 = siteinds("S=1/2", g), siteinds("S=1/2", g)
sind1, sind2 = only(s1[(1,)]), only(s2[(1,)])
ITensorNetworks.itensor_identity_map([sind1=>prime(sind1), sind2=>prime(sind2)])
Whilst
#More complex constructor
g = named_grid((4,))
s1, s2 = siteinds("S=1/2", g; conserve_qns =true), siteinds("S=1/2", g; conserve_qns =true)
sind1, sind2 = only(s1[(1,)]), only(s2[(1,)])
ITensorNetworks.itensor_identity_map([sind1=>prime(sind1), sind2=>prime(sind2)])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there's a way to avoid these bugs and avoid making it Float64 type I can make that fix in an new PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, interesting. An alternative design could be:
function itensor_identity_map(elt::Type, i_pairs::Vector)
return prod(i_pairs; init=ITensor(one(elt))) do i_pair
return denseblocks(delta(elt, last(i_pair), dag(first(i_pair))))
end
end
itensor_identity_map(i_pairs::Vector) = itensor_identity_map(Float64, i_pairs)Then we can set the element type as needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Though it would be nice to fix that bug anyway.)


This PR fixes some bugs in the constructor of the
identitytensors in the middle of aQuadarticFormNetworkorBiLinearFormNetworkwhich emerge when quantum numbers are enabled.Specifically,
denseblocksis cast on the identities on each site to ensure they can be multiplied together and also the order of the indices is corrected so that the appropriate identity matrix is constructed whenenable_auto_fermionis active.@emstoudenmire @anmello hopefully this is sufficient to fix some of those fermionic issues that were arising.