Skip to content

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.

Sign up for GitHub

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

Merged
merged 20 commits into from
May 24, 2025
Merged

Fix FormNetwork constructor bug #238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
c845947
Blah
JoeyT1994 Sep 13, 2024
90c7251
Merge remote-tracking branch 'origin/main'
JoeyT1994 Oct 17, 2024
86f3087
Merge remote-tracking branch 'upstream/main'
JoeyT1994 Oct 17, 2024
6ff0cd5
Bug fix in current ortho. Change test
JoeyT1994 Oct 17, 2024
34e8e5e
Merge remote-tracking branch 'upstream/main'
JoeyT1994 Nov 22, 2024
d096722
Fix bug
JoeyT1994 Nov 26, 2024
70a3f7e
Merge remote-tracking branch 'upstream/main'
JoeyT1994 Dec 5, 2024
9d64fe8
Merge remote-tracking branch 'upstream/main'
JoeyT1994 Mar 19, 2025
9d6c1bc
File removed
JoeyT1994 Mar 19, 2025
ae17245
Merge remote-tracking branch 'upstream/main'
JoeyT1994 Mar 23, 2025
b648353
Merge remote-tracking branch 'upstream/main'
JoeyT1994 Apr 1, 2025
4e7d189
Merge remote-tracking branch 'upstream/main'
JoeyT1994 Apr 3, 2025
83c92b0
Merge remote-tracking branch 'upstream/main'
JoeyT1994 Apr 7, 2025
6f024ee
Merge remote-tracking branch 'upstream/main'
JoeyT1994 Apr 11, 2025
1106403
Merge remote-tracking branch 'upstream/main'
JoeyT1994 Apr 17, 2025
5641d32
Merge remote-tracking branch 'upstream/main'
JoeyT1994 Apr 30, 2025
59ef115
Merge remote-tracking branch 'upstream/main'
JoeyT1994 May 9, 2025
72bea86
Merge remote-tracking branch 'upstream/main'
JoeyT1994 May 13, 2025
bc14b33
Merge remote-tracking branch 'upstream/main'
JoeyT1994 May 20, 2025
9c5151d
Fix constructore bug for form networks
JoeyT1994 May 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name = "ITensorNetworks"
uuid = "2919e153-833c-4bdc-8836-1ea460a35fc7"
authors = ["Matthew Fishman <[email protected]>, Joseph Tindall <[email protected]> and contributors"]
version = "0.13.9"
version = "0.13.10"

[deps]
AbstractTrees = "1520ce14-60c1-5f80-bbc7-55ef81b5835c"
Expand Down
3 changes: 2 additions & 1 deletion src/formnetworks/bilinearformnetwork.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using ITensors: ITensor, Op, prime, sim
using ITensors.NDTensors: denseblocks

default_dual_site_index_map = prime
default_dual_link_index_map = sim
Expand Down Expand Up @@ -54,7 +55,7 @@ end

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))))
Copy link
Member

@mtfishman mtfishman May 24, 2025

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:

Suggested change
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?

Copy link
Collaborator Author

@JoeyT1994 JoeyT1994 May 24, 2025

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)])

returns
image

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)])

returns
image

Copy link
Collaborator Author

@JoeyT1994 JoeyT1994 May 24, 2025

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

Copy link
Member

@mtfishman mtfishman May 24, 2025

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.

Copy link
Member

@mtfishman mtfishman May 24, 2025

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.)

end
end

Expand Down
Loading