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

[NDTensors] Circumvent scalar indexing to improve GPU performance #1216

Closed
wants to merge 17 commits into from

Conversation

kmp5VT
Copy link
Collaborator

@kmp5VT kmp5VT commented Oct 23, 2023

Description

As a continued effort to make the ITensors efficient on GPU in this branch I am identifying and fixing areas that were missed in previous PRs. First I am looking at the profiling of GPU based SVD and will also address issues in 1193

TODO

  • Make sure that all SVD code works without scalar indexing on CUDA based GPU.
  • make performance bench marks for ITensors GPU SVD implementation

@kmp5VT kmp5VT changed the title Kmp5/enhancements/more gpu [NDTensors][NDTensorsGPUExt][Debug] Removing scalar operations from GPU code Oct 23, 2023
@kmp5VT kmp5VT requested a review from mtfishman October 23, 2023 13:21
@kmp5VT kmp5VT marked this pull request as draft October 23, 2023 13:21
Comment on lines +2 to +4
## TODO here it looks at the elements of S so convert to CPU when on GPU
## Could write this as a GPU impl which just converts S to array. S
## is not used again so we don't need to convert back to GPU.
Copy link
Member

Choose a reason for hiding this comment

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

Same suggestion here as above for truncate!!, I think we should use leaf_parenttype dispatch.

D[n] = R[n, n]
end
n = size(R)[1]
D = diag(R, (n - Nd))
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this is doing what you think it is doing, diag(M, k) selects the kth off-diagonal of the matrix.

I went with:

  D[1:Nd] = diag(R)[1:Nd]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah I see, thanks!

@mtfishman mtfishman changed the title [NDTensors][NDTensorsGPUExt][Debug] Removing scalar operations from GPU code [NDTensors] Circumvent scalar indexing to improve GPU performance Oct 23, 2023
@mtfishman
Copy link
Member

A lot of this PR is superseded by #1215.

@mtfishman
Copy link
Member

@kmp5VT can we close this? Should be superseded by #1215 and #1217. The only thing I didn't add was your change to copy but seems like we can add that later as needed (using leaf_parenttype dispatch).

@kmp5VT
Copy link
Collaborator Author

kmp5VT commented Oct 25, 2023

@mtfishman yes I'll close this PR and open a new one if I find something awry. Thanks!

@kmp5VT kmp5VT closed this Oct 25, 2023
@kmp5VT kmp5VT deleted the kmp5/enhancements/more_gpu branch April 15, 2024 14:27
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