Skip to content

[BE] Add selected custom ops to CI #11744

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
Merged
merged 2 commits into from
Jul 2, 2025
Merged

[BE] Add selected custom ops to CI #11744

merged 2 commits into from
Jul 2, 2025

Conversation

kimishpatel
Copy link
Contributor

Summary:
Earlier custom sdpa and kv cache werent being tested in OSS CI. This diff changes that.

Tests
CI

ghstack-source-id: 35586450183e53b22971190f939bd6e362b3859a
Pull Request resolved: #11743

Summary

[PLEASE REMOVE] See CONTRIBUTING.md's Pull Requests for ExecuTorch PR guidelines.

[PLEASE REMOVE] If this PR closes an issue, please add a Fixes #<issue-id> line.

[PLEASE REMOVE] If this PR introduces a fix or feature that should be the upcoming release notes, please add a "Release notes: " label. For a list of available release notes labels, check out CONTRIBUTING.md's Pull Requests.

Test plan

[PLEASE REMOVE] How did you test this PR? Please write down any manual commands you used and note down tests that you have written if applicable.

@pytorch-bot PyTorch Bot
Copy link

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

🔗 Helpful Links

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

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

❌ 1 Cancelled Job

As of commit f318b9b with merge base 5cc5421 (image):

CANCELLED JOB - The following job was cancelled. Please retry:

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

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 16, 2025
@facebook-github-bot
Copy link
Contributor

@kimishpatel has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@github-actions GitHub Actions
Copy link

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

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

@kimishpatel kimishpatel force-pushed the enable-custom-ops-ci-testing branch from ddfd449 to ea8cc16 Compare June 17, 2025 03:04
@facebook-github-bot
Copy link
Contributor

@kimishpatel has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

mergennachin
mergennachin reviewed Jun 18, 2025
View reviewed changes
extension/llm/custom_ops/test_sdpa_with_kv_cache.py
)

@unittest.skipIf(
not is_fbcode(), "in OSS error is too large 0.0004 for some reason"
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's enable in OSS. Otherwise we will never enable in OSS.

We can finetune atol and rtol during comparison.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am little reluctant on this because on one hand yes we can make it pass by tuning this knob, but on the other hand if there is an issue we wont catch it. My preference would be to have this test at least running internally without compromise. I dont know the reason as to why it requires low atol

metascroy
metascroy reviewed Jul 1, 2025
View reviewed changes
extension/llm/custom_ops/test_quantized_sdpa.py
self.assertTrue(torch.allclose(ref_output, op_output, atol=atol))

@unittest.skipIf(
not is_fbcode(), "in OSS error is too large 0.0002 for some reason"
Copy link
Contributor

Choose a reason for hiding this comment

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

What does ""in OSS error is too large 0.0002 for some reason" mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I ran this test in OSS error i got was 0.0002. Which is too large for this test to pass. It does pass internally so not sure if this is blas lib issue or mac vs linux. Bug I wanted to enable the tests in oss since we have to coverage otherwise

@kimishpatel kimishpatel force-pushed the enable-custom-ops-ci-testing branch from ea8cc16 to 9000e3b Compare July 1, 2025 22:48
@kimishpatel
[BE] Add selected custom ops to CI
8eb31e2 
Summary:
Earlier custom sdpa and kv cache werent being tested in OSS CI. This diff changes that.

Tests
CI

ghstack-source-id: 3558645
Pull Request resolved: #11743
@kimishpatel kimishpatel force-pushed the enable-custom-ops-ci-testing branch from 9000e3b to 8eb31e2 Compare July 2, 2025 00:36
@facebook-github-bot
Copy link
Contributor

@kimishpatel has imported this pull request. If you are a Meta employee, you can view this in D76775724.

@kimishpatel kimishpatel merged commit 5a4ef46 into main Jul 2, 2025
96 of 97 checks passed
@kimishpatel kimishpatel deleted the enable-custom-ops-ci-testing branch July 2, 2025 14:04
BujSet pushed a commit to BujSet/executorch that referenced this pull request Jul 2, 2025
@kimishpatel @BujSet
[BE] Add selected custom ops to CI (pytorch#11744)
58fc34a 
Summary:
Earlier custom sdpa and kv cache werent being tested in OSS CI. This
diff changes that.

Tests
CI

ghstack-source-id: 3558645
Pull Request resolved: pytorch#11743

### Summary
[PLEASE REMOVE] See [CONTRIBUTING.md's Pull
Requests](https://github.com/pytorch/executorch/blob/main/CONTRIBUTING.md#pull-requests)
for ExecuTorch PR guidelines.

[PLEASE REMOVE] If this PR closes an issue, please add a `Fixes
#<issue-id>` line.

[PLEASE REMOVE] If this PR introduces a fix or feature that should be
the upcoming release notes, please add a "Release notes: <area>" label.
For a list of available release notes labels, check out
[CONTRIBUTING.md's Pull
Requests](https://github.com/pytorch/executorch/blob/main/CONTRIBUTING.md#pull-requests).

### Test plan
[PLEASE REMOVE] How did you test this PR? Please write down any manual
commands you used and note down tests that you have written if
applicable.
Tanish2101 pushed a commit to Tanish2101/executorch that referenced this pull request Jul 9, 2025
@kimishpatel @Tanish2101
[BE] Add selected custom ops to CI (pytorch#11744)
0710403 
Summary:
Earlier custom sdpa and kv cache werent being tested in OSS CI. This
diff changes that.

Tests
CI

ghstack-source-id: 3558645
Pull Request resolved: pytorch#11743

### Summary
[PLEASE REMOVE] See [CONTRIBUTING.md's Pull
Requests](https://github.com/pytorch/executorch/blob/main/CONTRIBUTING.md#pull-requests)
for ExecuTorch PR guidelines.

[PLEASE REMOVE] If this PR closes an issue, please add a `Fixes
#<issue-id>` line.

[PLEASE REMOVE] If this PR introduces a fix or feature that should be
the upcoming release notes, please add a "Release notes: <area>" label.
For a list of available release notes labels, check out
[CONTRIBUTING.md's Pull
Requests](https://github.com/pytorch/executorch/blob/main/CONTRIBUTING.md#pull-requests).

### Test plan
[PLEASE REMOVE] How did you test this PR? Please write down any manual
commands you used and note down tests that you have written if
applicable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mergennachin mergennachin mergennachin left review comments

@metascroy metascroy metascroy approved these changes

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

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

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

Assignees

No one assigned

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kimishpatel @facebook-github-bot @mergennachin @metascroy