Test fixes for float precision issues (ie, on Linux aarch64) #2115
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
...and lower it for doubles. Previously, we used the same tolerance (1e-6) for both floats and doubles. This sets the epsilon to 1e-5 for floats, and 1e-10 for doubles. Running on an ARM system (Arm Neoverse N1) yielded an absolute error of 1.0259421223521911e-06 for one of the GetInverse tests, which exceeded this tolerance. Testing on a more "standard" AMD cpu (AMD Ryzen Threadripper PRO 3975WX) yielded an error of 9.610960183499514e-07 for the same test - juuuuust below the threshold. My guess is that the threshold was never meant to be so tight, and that 1e-6 was chosen arbitrarily, and it seemed to work. 1e-5 should provide a bit more tolerance for float architecture differences. At the same time, increasing the tolerance for double matrices gives us increased safety for the code paths that will generally be used for matrix math. (In my tests, the maximum error for both ARM and AMD architectures never got out of the e-15 range, so e-10 should still be plenty of headroom.)
pmolodo
changed the title
|
Filed as internal issue #USD-7816 |
This was referenced Dec 12, 2022
2 tasks
|
We are fine with this change, but are curious if there are any specific compiler flags being used, like "-ffast-math" or others which break IEEE-754 compliance, as the differences with arm numbers seem a bit large than expected. Thanks |
|
We're not setting that, so not unless build_usd.py sets it...
…On Thu, Feb 16, 2023, 3:19 PM Varun ***@***.***> wrote:
We are fine with this change, but are curious if there are any specific
compiler flags being used, like "-ffast-math" or others which break
IEEE-754 compliance, as the differences with arm numbers seem a bit large
than expected.
Thanks
—
Reply to this email directly, view it on GitHub
<#2115 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAARQ6FCRRQO3SIQD7LZZGLWX2YWTANCNFSM6AAAAAASO7E4MI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change(s)
A collection of float precision tolerance fixes, to resolve test failures on Linux aarch64 platforms (tested on an Arm Neoverse N1).
For more aarch64 related fixes, also see:
Fixes Issue(s)
testGfMatrix failure on Linux aarch64
testGfRotation failure on Linux aarch64
testUsdDancingCubesExample failure on Linux aarch64
I have verified that all unit tests pass with the proposed changes
I have submitted a signed Contributor License Agreement