8000 Improve the precision of abs() and sign() for large values by lezcano · Pull Request #99550 · pytorch/pytorch · GitHub
[go: up one dir, main page]

Skip to content

Improve the precision of abs() and sign() for large values #99550

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.

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

Closed
wants to merge 3 commits into from

Conversation

lezcano
Copy link
Collaborator
@lezcano lezcano commented Apr 19, 2023

Stack from ghstack (oldest at bottom):

@ev-br found in
Quansight-Labs/numpy_pytorch_interop#117 (comment)
that the precision of abs() for large values in the vectorised case is less-than-good.
This PR fixes this issue. While doing that, we are able to comment out a
few tests on extremal values.

Fixes #53958 #48486

cc @jgong5 @mingfeima @XiaobingSuper @sanchitintel @ashokei @jingxu10

@ev-br found in
Quansight-Labs/numpy_pytorch_interop#117 (comment)
that the precision of `abs()` for large values in the vectorised case is less-than-good.
This PR fixes this issue. While doing that, we are able to comment out a
few tests on extremal values.

[ghstack-poisoned]
@lezcano lezcano requested review from mruberry and ngimel as code owners April 19, 2023 17:56
@pytorch-bot
Copy link
pytorch-bot bot commented Apr 19, 2023

🔗 Helpful Links

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

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

❗ 1 Active SEVs

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

✅ No Failures

As of commit 92cbe49:
💚 Looks good so far! There are no failures yet. 💚

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

lezcano added a commit that referenced this pull request Apr 19, 2023
ev-br found in
Quansight-Labs/numpy_pytorch_interop#117 (comment)
that the precision of `abs()` for large values in the vectorised case is less-than-good.
This PR fixes this issue. While doing that, we are able to comment out a
few tests on extremal values.

ghstack-source-id: 96a8b77
Pull Request resolved: #99550
@github-actions github-actions bot added the module: cpu CPU specific problem (e.g., perf, algorithm) label Apr 19, 2023
@lezcano lezcano requested review from peterbell10 and removed request for mruberry April 19, 2023 17:57
@lezcano lezcano added module: vectorization Related to SIMD vectorization, e.g., Vec256 release notes: performance_as_product topic: performance topic category labels Apr 19, 2023
ev-br found in
Quansight-Labs/numpy_pytorch_interop#117 (comment)
that the precision of `abs()` for large values in the vectorised case is less-than-good.
This PR fixes this issue. While doing that, we are able to comment out a
few tests on extremal values.

cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10

[ghstack-poisoned]
@lezcano
Copy link
Collaborator Author
lezcano commented Apr 20, 2023

@peterbell10, addressed the review. I also cleaned sgn a bit, making it more uniform across the 4 implementations.

ev-br found in
Quansight-Labs/numpy_pytorch_interop#117 (comment)
that the precision of `abs()` for large values in the vectorised case is less-than-good.
This PR fixes this issue. While doing that, we are able to comment out a
few tests on extremal values.

Fixes #53958 #48486

cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10

[ghstack-poisoned]
lezcano added a commit that referenced this pull request Apr 20, 2023
ev-br found in
Quansight-Labs/numpy_pytorch_interop#117 (comment)
that the precision of `abs()` for large values in the vectorised case is less-than-good.
This PR fixes this issue. While doing that, we are able to comment out a
few tests on extremal values.

ghstack-source-id: c3ca417
Pull Request resolved: #99550
@lezcano
Copy link
Collaborator Author
lezcano commented Apr 24, 2023

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Apr 24, 2023
@pytorchmergebot
Copy link
8000 Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@facebook-github-bot facebook-github-bot deleted the gh/Lezcano/190/head branch June 8, 2023 14:45
@Flamefire
Copy link
Collaborator

Unfortunately the VSX code hasn't been updated and produces NaNs where inf is expected for those test_reference_numerics_extremal__refs_abs_cpu_complex* tests

@lezcano
Copy link
Collaborator Author
lezcano commented Jan 5, 2024

Feel free to send a PR

@Flamefire
Copy link
Collaborator

Done: #116859

pytorchmergebot pushed a commit to Flamefire/pytorch that referenced this pull request Jan 5, 2024
Use a similar approach with Sleef as in pytorch#99550
to improve the precision and extremal value handling of the `abs`
function for complex tensors.

This fixes
- test_reference_numerics_extremal__refs_abs_cpu_float64
- test_reference_numerics_extremal__refs_abs_cpu_float128

which failed on PPC.
pytorchmergebot pushed a commit that referenced this pull request Jan 5, 2024
Use a similar approach with Sleef as in #99550
to improve the precision and extremal value handling of the `abs` function for complex tensors.

This fixes
- test_reference_numerics_extremal__refs_abs_cpu_float64
- test_reference_numerics_extremal__refs_abs_cpu_float128

which failed on PPC.

Pull Request resolved: #116859
Approved by: https://github.com/lezcano
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk Trigger trunk jobs on your pull request Merged merging module: cpu CPU specific problem (e.g., perf, algorithm) module: vectorization Related to SIMD vectorization, e.g., Vec256 50BC open source topic: performance topic category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
0