pytorch / executorch

On-device AI across mobile, embedded and edge for PyTorch
https://pytorch.org/executorch/
Other
2.22k stars 368 forks source link

[Executorch][Portable] Dont upcast to double for sigmoid #6931

Closed kirklandsign closed 1 week ago

kirklandsign commented 1 week ago

Pull Request resolved: https://github.com/pytorch/executorch/pull/6892

Upcasting to double for compute precision may not be aten compliant.

Reason for internal test change: Apparently running on broadwell CPU vs test runner with Cooper lake gives different results for this change.

Without this change: Both broadwell and Cooper lake will produce "Once upon a time, there was a little"

With this change: Broadwell still produces "Once upon a time, there was a little", while Cooperlake produces "Once upon a time, there was a girl".

So one possibility is that that some XNNPACK kernel for Cooper lake is produces slightly different numerical result that propagates through.

Still landing this change since upcasting to double for compute, does not seem necessary. ghstack-source-id: 253832495 @exported-using-ghexport

Differential Revision: D65928920

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[bot] commented 1 week ago

:link: Helpful Links

:test_tube: See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/6931

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

:heavy_exclamation_mark: 2 Active SEVs

There are 2 currently active SEVs. If your PR is affected, please view them below:

:x: 4 New Failures

As of commit 23a2b0affebf1f1a23f83a0c475a4bcbc0f3358a with merge base dc41596b1a61ca3de7b5ad35e0692a78d7b185eb (image):

NEW FAILURES - The following jobs have failed:

* [pull / android / run-emulator](https://hud.pytorch.org/pr/pytorch/executorch/6931#33156374594) ([gh](https://github.com/pytorch/executorch/actions/runs/11898346060/job/33156374594)) `##[error]The process '/usr/bin/sh' failed with exit code 1` * [pull / test-llama-runner-qnn-linux (fp32, qnn) / linux-job](https://hud.pytorch.org/pr/pytorch/executorch/6931#33154647520) ([gh](https://github.com/pytorch/executorch/actions/runs/11898346060/job/33154647520)) `Invalid image name executorch-ubuntu-22.04-qnn-sdk` * [pull / test-llava-runner-linux / linux-job](https://hud.pytorch.org/pr/pytorch/executorch/6931#33154645303) ([gh](https://github.com/pytorch/executorch/actions/runs/11898346060/job/33154645303)) `test_prefill_logits` * [pull / unittest / linux / linux-job](https://hud.pytorch.org/pr/pytorch/executorch/6931#33154649021) ([gh](https://github.com/pytorch/executorch/actions/runs/11898346060/job/33154649021)) `extension/llm/modules/test/test_mha.py::AttentionTest::test_attention_executorch`

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