FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Basic tensor support for AMDGPU by kshyatt · Pull Request #341 · QuantumKitHub/TensorKit.jl · GitHub

Basic tensor support for AMDGPU - #341

Merged
kshyatt merged 1 commit into
mainfrom
ksh/amd
Apr 22, 2026
Merged

kshyatt merged 1 commit into
mainfrom
ksh/amd

Conversation

kshyatt commented Jan 5, 2026

Copy link
Copy Markdown
Member

Mostly copied from the CUDA support

kshyatt requested review from Jutho and lkdvos January 5, 2026 13:59

github-actions Bot commented Jan 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Your PR no longer requires formatting changes. Thank you for your contribution!

lkdvos previously approved these changes Jan 5, 2026

lkdvos left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

In principle looks good to me!

Do we want to wait with the AMD support until the dust settles on CUDA (mostly to avoid having to duplicate things we might still change), or should we just go ahead with this?

kshyatt commented Jan 6, 2026

Copy link
Copy Markdown
Member Author

This doesn't include factorization stuff which is the only inflight CUDA thing, I think? Most of the diff is the tests, tbh

lkdvos commented Jan 6, 2026

Copy link
Copy Markdown
Member

The tests is actually what I was thinking of, but maybe it's really not that bad

kshyatt force-pushed the ksh/amd branch 2 times, most recently from 33e01a6 to fa009dc Compare January 21, 2026 12:47
lkdvos previously approved these changes Jan 21, 2026

lkdvos left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Overall looks good to me, I would be happy to merge and gradually improve

Comment thread test/amd/tensors.jl Outdated
@test ht2 == TensorKit.to_cpu(dt2)
end

dt3 = AMDGPU.@allowscalar repartition(t, k)

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Are we tracking these @allowscalar calls somewhere? Technically this test is now not really testing whether or not it works :p

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

At least in the tests we can just search in the file, I can make a tracker comment at the top?

kshyatt commented Jan 21, 2026

Copy link
Copy Markdown
Member Author

Let me figure out where the segfaults are happening then I'm also ok to merge

codecov Bot commented Mar 3, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 52.11268% with 34 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
ext/TensorKitAMDGPUExt/roctensormap.jl 51.42% 34 Missing ⚠️
Files with missing lines Coverage Δ
ext/TensorKitAMDGPUExt/TensorKitAMDGPUExt.jl 100.00% <100.00%> (ø)
ext/TensorKitAMDGPUExt/roctensormap.jl 51.42% <51.42%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread ext/TensorKitAMDGPUExt/roctensormap.jl Outdated

kshyatt commented Mar 24, 2026

Copy link
Copy Markdown
Member Author

I think everything got addressed here. Much will have to wait for JuliaGPU/AMDGPU.jl#890

Comment thread test/amd/tensors.jl

lkdvos commented Mar 31, 2026

Copy link
Copy Markdown
Member

Going to cancel the tests here to try and get the test rework in first

kshyatt commented Apr 3, 2026

Copy link
Copy Markdown
Member Author

given that the other PR is not ready I'm going to restart these

kshyatt commented Apr 3, 2026

Copy link
Copy Markdown
Member Author

OK, relevant CI appears to be passing. @Jutho I think this is ready if you are ok with it. Much has to wait for the corresponding TensorOperations support, which I am now going to pick up again.

kshyatt commented Apr 20, 2026

Copy link
Copy Markdown
Member Author

The permute error seems very odd?

kshyatt commented Apr 20, 2026

Copy link
Copy Markdown
Member Author

CUDA fail here is unrelated, I think

kshyatt commented Apr 21, 2026

Copy link
Copy Markdown
Member Author

Can this be merged if the AMDGPU tests pass?

kshyatt commented Apr 22, 2026

Copy link
Copy Markdown
Member Author

OK, GPU tests are 🎉 passing 🎉 . Can we finally merge this?

kshyatt commented Apr 22, 2026

Copy link
Copy Markdown
Member Author

I'll merge this and make a second pass now that the Strided GPUArrays stuff is in

kshyatt merged commit 6d02541 into main Apr 22, 2026
59 of 70 checks passed
kshyatt deleted the ksh/amd branch April 22, 2026 11:11
lkdvos mentioned this pull request Apr 23, 2026
lkdvos referenced this pull request Apr 24, 2026
* Update changelog for v0.16.4

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Bump version to v0.16.4

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Update CITATION.cff for v0.16.4

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* manual improvements of Changelog

* amend changelog

* delete compatcheck for failing julia 1

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

3 participants


Back | FazBrowse Home | New Git URL