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.

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

Clean up optimized-oss.yaml #8549

Open
wants to merge 4 commits into
base: gh/swolchok/267/base
Choose a base branch
from
Open

Clean up optimized-oss.yaml #8549

wants to merge 4 commits into from

Conversation

swolchok
Copy link
Contributor

@swolchok swolchok commented Feb 18, 2025 •
edited
Loading

Stack from ghstack (oldest at bottom):

We don't need this now that we support log_softmax and gelu in OSS!

Differential Revision: D69475020

Clean up optimized-oss.yaml
7625584 
We don't need this now that we support log_softmax and gelu in OSS!

Differential Revision: [D69475020](https://our.internmc.facebook.com/intern/diff/D69475020/)

[ghstack-poisoned]
@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented Feb 18, 2025 •
edited
Loading

🔗 Helpful Links

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

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

❌ 1 New Failure

As of commit e3a9b96 with merge base fd318cc (image):

NEW FAILURE - The following job has failed:

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 Feb 18, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D69475020

@swolchok swolchok mentioned this pull request Feb 18, 2025
Merged
swolchok pushed a commit that referenced this pull request Feb 18, 2025
Clean up optimized-oss.yaml
3e91992 
We don't need this now that we support log_softmax and gelu in OSS!

Differential Revision: [D69475020](https://our.internmc.facebook.com/intern/diff/D69475020/)

ghstack-source-id: 267044106
Pull Request resolved: #8549
@swolchok swolchok added the release notes: build Changes related to build, including dependency upgrades, build flags, optimizations, etc. label Feb 18, 2025
Update on "Clean up optimized-oss.yaml"
f96da42 
We don't need this now that we support log_softmax and gelu in OSS!

Differential Revision: [D69475020](https://our.internmc.facebook.com/intern/diff/D69475020/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D69475020

swolchok pushed a commit that referenced this pull request Feb 19, 2025
Clean up optimized-oss.yaml
9b78b15 
Pull Request resolved: #8549

We don't need this now that we support log_softmax and gelu in OSS!
ghstack-source-id: 267214960
@exported-using-ghexport

Differential Revision: [D69475020](https://our.internmc.facebook.com/intern/diff/D69475020/)
@swolchok
Copy link
Contributor Author

wav2letter test is breaking. this change makes it use optimized log_softmax. not yet sure if this indicates an accuracy issue in optimized log_softmax or if the test tolerance just needs an update...

@swolchok
Copy link
Contributor Author

oh, the test value are WAY off:

eager output[0]: tensor([[-8.3237, -8.3237, -8.3236],
        [-8.3237, -8.3237, -8.3236],
        [-8.2994, -8.2991, -8.2993],
        ...,
        [-8.3237, -8.3237, -8.3236],
        [-8.3218, -8.3221, -8.3220],
        [-8.3113, -8.3117, -8.3116]])
executorch output[0]: tensor([[-8.3233, -8.3234, -8.3235],
        [ 0.9652,  0.9652, -8.3235],
        [ 0.9890,  0.9892, -8.2992],
        ...,
        [-8.3233, -8.3234, -8.3235],
        [-8.3215, -8.3218, -8.3219],
        [-8.3109, -8.3114, -8.3115]])

optimized log_softmax is clearly broken. need to expand tests for that op and figure this out

@mergennachin
Copy link
Contributor

optimized log_softmax is clearly broken. need to expand tests for that op and figure this out

@manuelcandales

@swolchok swolchok mentioned this pull request Feb 20, 2025
Closed
Update on "Clean up optimized-oss.yaml"
9ce041f 
We don't need this now that we support log_softmax and gelu in OSS!

Differential Revision: [D69475020](https://our.internmc.facebook.com/intern/diff/D69475020/)

[ghstack-poisoned]
This was referenced Feb 20, 2025
Open
Open
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D69475020

swolchok pushed a commit that referenced this pull request Feb 20, 2025
Clean up optimized-oss.yaml
33fcf11 
Pull Request resolved: #8549

We don't need this now that we support log_softmax and gelu in OSS!
ghstack-source-id: 267452085
@exported-using-ghexport

Differential Revision: [D69475020](https://our.internmc.facebook.com/intern/diff/D69475020/)
@swolchok
Copy link
Contributor Author

there's some problem with supported_features for optimized kernels in OSS IIRC. will have to debug & fix

larryliu0820
larryliu0820 approved these changes Feb 20, 2025
View reviewed changes
Copy link
Contributor

@larryliu0820 larryliu0820 left a comment

Choose a reason for hiding this comment

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

Thank you for doing this!

Update on "Clean up optimized-oss.yaml"
e3a9b96 
We don't need this now that we support log_softmax and gelu in OSS!

Differential Revision: [D69475020](https://our.internmc.facebook.com/intern/diff/D69475020/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D69475020

swolchok pushed a commit that referenced this pull request Feb 20, 2025
Clean up optimized-oss.yaml
3bccc98 
Pull Request resolved: #8549

We don't need this now that we support log_softmax and gelu in OSS!
ghstack-source-id: 267508548
@exported-using-ghexport

Differential Revision: [D69475020](https://our.internmc.facebook.com/intern/diff/D69475020/)
@swolchok swolchok requested a review from dbort as a code owner February 20, 2025 22:19
@swolchok
Copy link
Contributor Author

there's some problem with supported_features for optimized kernels in OSS IIRC. will have to debug & fix

OSS CMake setup was using the portable supported_features config for optimized. probably a copy/paste/forget-to-edit error. fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@larryliu0820 larryliu0820 larryliu0820 approved these changes

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

@cccclai cccclai Awaiting requested review from cccclai

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

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

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

@dbort dbort Awaiting requested review from dbort dbort 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. fb-exported release notes: build Changes related to build, including dependency upgrades, build flags, optimizations, etc.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@swolchok @facebook-github-bot @mergennachin @larryliu0820