Skip to content

[ROCm] SDPA fix mem fault when dropout is enabled #154864

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

Jump to bottom
Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

[ROCm] SDPA fix mem fault when dropout is enabled #154864

wants to merge 25 commits into from

Conversation

alugorey
Copy link
Contributor

@alugorey alugorey commented Jun 2, 2025 •
edited by pytorch-bot bot
Loading

Fixes issue that exhibited a device side memory access fault due to incorrect tensor life management

cc @jeffdaily @sunway513 @jithunnair-amd @pruthvistony @ROCmSupport @dllehr-amd @jataylo @hongxiayang @naromero77amd

@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented Jun 2, 2025 •
edited
Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/154864

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 22ff165 with merge base df55ec7 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jun 2, 2025

This PR needs a release notes: label

If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

jeffdaily
jeffdaily requested changes Jun 2, 2025
View reviewed changes
Copy link
Collaborator

@jeffdaily jeffdaily left a comment

Choose a reason for hiding this comment

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

Why did you add template <int Version = 2>?

@soulitzer soulitzer added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jun 2, 2025
@alugorey alugorey force-pushed the sdpa_mem_fault_dropout branch from 6ca22cc to dd7dfe9 Compare June 20, 2025 22:02
@jeffdaily jeffdaily added the ciflow/rocm Trigger "default" config CI on ROCm label Jun 20, 2025
@jeffdaily jeffdaily changed the title Sdpa mem fault dropout [ROCm] SDPA fix mem fault when dropout is enabled Jun 20, 2025
@pytorch-bot pytorch-bot bot added the module: rocm AMD GPU support for Pytorch label Jun 20, 2025
jeffdaily
jeffdaily approved these changes Jun 20, 2025
View reviewed changes
Copy link
Collaborator

@jeffdaily jeffdaily left a comment

Choose a reason for hiding this comment

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

approved, pending CI

@pytorch-bot pytorch-bot bot removed the ciflow/rocm Trigger "default" config CI on ROCm label Jun 20, 2025
@jeffdaily jeffdaily added the ciflow/rocm Trigger "default" config CI on ROCm label Jun 20, 2025
@pytorch-bot pytorch-bot bot removed the ciflow/rocm Trigger "default" config CI on ROCm label Aug 10, 2025
alugorey added 17 commits August 10, 2025 03:09
@alugorey
Mostly debug traces. about to implement philox changes
dee8c7f
@alugorey
Started making changes for philox seed
c805182
@alugorey
May have fixed forward. gets to backward but complains about dtype
c9c188a
@alugorey
memfault fixed but bad answer
aad13cd
@alugorey
no memfault, wrong answer with traces
ed43847
@alugorey
Debugging dropout mask
235e3d6
@alugorey
blind port of dropout code from xformers
aded22a
@alugorey
DELETE ME saving place to start work again. testing new CK based RNG
41e3136
@alugorey
Calling with xformers op, need to change to ck
7a41b0e
@alugorey
Used ck namespace instead of copying xformers
dc21af6
@alugorey
Uncomment code and swithc to cK in test
dd532ed
@alugorey
just final row of output is off now
18925fe
@alugorey
WORKING! need to productize
201517d
@alugorey
commented out traces for cleaner testing
b913ec8
@alugorey
Enable a few more tests, all pass so far
7df77c7
@alugorey
add fp16 to test
3b593fd
@alugorey
figured out gross boundaries for tolerances. need to tighten
9ea6e3c
@alugorey alugorey force-pushed the sdpa_mem_fault_dropout branch from ca719d8 to 9c02e37 Compare August 10, 2025 03:16
@jeffdaily jeffdaily added the ciflow/rocm Trigger "default" config CI on ROCm label Aug 11, 2025
@alugorey alugorey force-pushed the sdpa_mem_fault_dropout branch from 9c02e37 to 22ff165 Compare August 11, 2025 22:36
@pytorch-bot pytorch-bot bot removed the ciflow/rocm Trigger "default" config CI on ROCm label Aug 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeffdaily jeffdaily jeffdaily approved these changes

@eqy eqy Awaiting requested review from eqy eqy is a code owner

@syed-ahmed syed-ahmed Awaiting requested review from syed-ahmed syed-ahmed is a code owner

Assignees
No one assigned
Labels
module: rocm AMD GPU support for Pytorch open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@alugorey @jeffdaily @soulitzer @pytorchbot