Skip to content

Fix vectorized trigonometric functions for VSX#86453

Closed
Flamefire wants to merge 1 commit intopytorch:masterfrom
Flamefire:fix_vsx_vector
Closed

Fix vectorized trigonometric functions for VSX#86453
Flamefire wants to merge 1 commit intopytorch:masterfrom
Flamefire:fix_vsx_vector

Conversation

@Flamefire
Copy link
Copy Markdown
Collaborator

@Flamefire Flamefire commented Oct 7, 2022

Replace the remaining hand-written code in vec256_float_vsx.h by calls to Sleef functions similar to what was done in #59382 & #82646 after #41541

This fixes wrong results for e.g. sin(1e20).
Fixes #85978

To fix #85978 I only needed to do the sin/cos functions to make the test pass but to not encounter the same issue again and again (see the previous PRs and issues) I checked the whole file for similar functions where a Sleef function could be used and changed those too. In the diff I've noticed the faulty whitespace so to make this complete I fixed that too, so it should now be done.

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

@pytorch-bot
Copy link
Copy Markdown

pytorch-bot Bot commented Oct 7, 2022

🔗 Helpful Links

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

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

✅ No Failures

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

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

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Oct 7, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: Flamefire / name: Alexander Grund (5bd16b2)

Copy link
Copy Markdown
Contributor

@malfet malfet left a comment

Choose a reason for hiding this comment

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

I trust that you've validated code outputs correct results

@pytorch-bot pytorch-bot Bot added the ciflow/trunk Trigger trunk jobs on your pull request label Oct 10, 2022
@malfet
Copy link
Copy Markdown
Contributor

malfet commented Oct 10, 2022

@pytorchbot rebase

@malfet
Copy link
Copy Markdown
Contributor

malfet commented Oct 10, 2022

@Flamefire you'll need to sign new CLA for the code needs to be merged in

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

@pytorchbot successfully started a rebase job. Check the current status here

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

Successfully rebased fix_vsx_vector onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout fix_vsx_vector && git pull --rebase)

Replace the remaining hand-written code in vec256_float_vsx.h by calls
to Sleef functions similar to what was done in pytorch#59382 & pytorch#82646 after pytorch#41541
Also remove some whitespace wrongly added in the above PRs.

This fixes wrong results for e.g. `sin(1e20)`.
Fixes pytorch#85978
@github-actions github-actions Bot added the module: cpu CPU specific problem (e.g., perf, algorithm) label Nov 22, 2022
@Flamefire
Copy link
Copy Markdown
Collaborator Author

Flamefire commented Nov 22, 2022

Rebased and CLA signed @malfet

I trust that you've validated code outputs correct results

Yes I used the patch/diff (before rebase) to build PyTorch on PPC and the tests succeeded. So assuming the rebase didn't change anything (shouldn't be the case) it does work now.

Edit: I used the current (rebased) patch on top of 1.12.1 and ran test_unary_ufuncs.py successfully on PPC.

@kit1980
Copy link
Copy Markdown
Contributor

kit1980 commented Nov 22, 2022

@pytorchbot merge

@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

@Flamefire Flamefire deleted the fix_vsx_vector branch November 23, 2022 10:33
kulinseth pushed a commit to kulinseth/pytorch that referenced this pull request Dec 10, 2022
Replace the remaining hand-written code in vec256_float_vsx.h by calls to Sleef functions similar to what was done in pytorch#59382 & pytorch#82646 after pytorch#41541

This fixes wrong results for e.g. `sin(1e20)`.
Fixes pytorch#85978

To fix pytorch#85978 I only needed to do the sin/cos functions to make the test pass but to not encounter the same issue again and again (see the previous PRs and issues) I checked the whole file for similar functions where a Sleef function could be used and changed those too. In the diff I've noticed the faulty whitespace so to make this complete I fixed that too, so it should now be done.

Pull Request resolved: pytorch#86453
Approved by: https://github.com/malfet
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 25, 2026
Replace the remaining hand-written code in vec256_float_vsx.h by calls to Sleef functions similar to what was done in pytorch#59382 & pytorch#82646 after pytorch#41541

This fixes wrong results for e.g. `sin(1e20)`.
Fixes pytorch#85978

To fix pytorch#85978 I only needed to do the sin/cos functions to make the test pass but to not encounter the same issue again and again (see the previous PRs and issues) I checked the whole file for similar functions where a Sleef function could be used and changed those too. In the diff I've noticed the faulty whitespace so to make this complete I fixed that too, so it should now be done.

Pull Request resolved: pytorch#86453
Approved by: https://github.com/malfet
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 module: cpu CPU specific problem (e.g., perf, algorithm) open source

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test_reference_numerics_large fails on PPC

5 participants