Skip to content

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

Closed
lezcano wants to merge 3 commits intogh/Lezcano/190/basefrom
gh/Lezcano/190/head
Closed

Improve the precision of abs() and sign() for large values#99550
lezcano wants to merge 3 commits intogh/Lezcano/190/basefrom
gh/Lezcano/190/head

Conversation

@lezcano
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Collaborator Author

lezcano commented Jan 5, 2024

Feel free to send a PR

@Flamefire
Copy link
Copy Markdown
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 open source topic: performance topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants