Skip to content

Fix for GPU Diag issue #1679#1684

Closed
kmp5VT wants to merge 0 commit into
ITensor:mainfrom
kmp5VT:main
Closed

Fix for GPU Diag issue #1679#1684
kmp5VT wants to merge 0 commit into
ITensor:mainfrom
kmp5VT:main

Conversation

@kmp5VT

@kmp5VT kmp5VT commented Nov 15, 2025

Copy link
Copy Markdown
Collaborator

Description

This is a generic Fix for the outer product of two diag tensors (i.e. outer product of two vectors blas ger function.) This uses Julia a .* b' feature from julia and seems to work on CPU and GPU with no indexing problem.
I added a unit tests to check in the NDTensors/diag test

@github-actions

github-actions Bot commented Nov 15, 2025

Copy link
Copy Markdown
Contributor

Your PR requires formatting changes to meet the project's style guidelines.
Please consider running Runic (git runic main) to apply these changes.

Click here to view the suggested changes.
diff --git a/NDTensors/test/test_diag.jl b/NDTensors/test/test_diag.jl
index ac54e023f..86640c254 100644
--- a/NDTensors/test/test_diag.jl
+++ b/NDTensors/test/test_diag.jl
@@ -123,10 +123,10 @@ end
 
     ## Test dot on GPU
     @test dot(t, A) ≈ dot(dev(array(t)), array(A)) rtol = sqrt(eps(elt))
-    
-    NDTensors.outer!(A, t,t);
+
+    NDTensors.outer!(A, t, t)
     for i in NDTensors.cpu(A)
-       @test i == one(elt)
+        @test i == one(elt)
     end
 end
 nothing

@kmp5VT kmp5VT requested a review from mtfishman November 15, 2025 21:45
end
t1 = T1.storage.data
t2 = T2.storage.data
array(R) .= t1 .* t2'

@mtfishman mtfishman Nov 15, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It looks like there are a few issues with this proposal:

  1. t2' will complex conjugate the elements, while outer[!] isn't supposed to conjugate the elements. Maybe transpose could be used as an alternative.
  2. t1 * transpose(t2) when t1 and t2 are vectors will output a matrix whose length this the product of the lengths of the diagonals of T1 and T2, I don't understand how the shapes of this operation work out since R in general is a tensor whose length is the product of the total lengths of T1 and T2 (not just the lengths of their diagonals). I think maybe you want to do something like t1 * transpose(t2) and reshape it into a vector (or relatedly, something like kron(t1, t2), which has an in-place counterpart kron!) and use that to write into the diagonal values of R.

It looks like you'll need to investigate this more to make sure it really does the correct operation.

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.

2 participants