Skip to content
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

Test “self_touching” fails on aarch64, ppc64le, s390x #97

Open
musicinmybrain opened this issue Sep 21, 2021 · 5 comments
Open

Test “self_touching” fails on aarch64, ppc64le, s390x #97

musicinmybrain opened this issue Sep 21, 2021 · 5 comments
Labels

Comments

@musicinmybrain
Copy link
Contributor

On aarch64, ppc64le, or s390x:

# self_touching
ok 82 124 triangles when expected 124
not ok 83 earcut deviation 0.000000% is not less than 0.000000%
ok 84 libtess2 deviation 0.001527% is less than 0.200000%

On x86_64, i686, or armv7hl/armhfp:

# self_touching
ok 82 124 triangles when expected 124
ok 83 earcut deviation 0.000000% is less than 0.000000%
ok 84 libtess2 deviation 0.001527% is less than 0.200000%

As you can see, the test output is not very informative. Changing the appropriate setprecision(6) to setprecision(16) in test/test.cpp gives, on all three failing architectures:

# self_touching
ok 82 124 triangles when expected 124
not ok 83 earcut deviation 0.0000000000052002% is not less than 0.0000000000043556%
ok 84 libtess2 deviation 0.0015267554740246% is less than 0.2000000000000000%
@mrgreywater
Copy link
Collaborator

Seems like the allowed deviation need to be loosened a little for the failing fixtures to accommodate your architecture and compiler configuration. Functionally I wouldn't worry about it though.

@musicinmybrain
Copy link
Contributor Author

I agree, and I’m planning to patch in a looser bound downstream as I package this library for Fedora Linux.

If it matters, the compiler is gcc 11.2.1, and there are no particularly exotic compiler flags in use.

Is there any interest in special-casing this test to accommodate these architectures upstream? If not, it’s probably best to close this issue.

Thanks for your feedback.

@mrgreywater
Copy link
Collaborator

I'm not averse to merging changes to support the library/tests on more platforms. That being said, unless we create travis-ci instances testing the builds on these architectures, there is no guarantee there won't be changes in the future breaking it for those arch's again.

@marcoffee
Copy link

I had a similar problem in aarch64 which was solved by passing the flag -ffp-contract=off to the compiler.
Such flag disables MADD / MSUB instructions, which, as I found out, behave differently on aarch64 architectures when compared to x86.
Here is the topic that led me to do this: https://bugs.mysql.com/bug.php?id=82760

@musicinmybrain
Copy link
Contributor Author

I had a similar problem in aarch64 which was solved by passing the flag -ffp-contract=off to the compiler. Such flag disables MADD / MSUB instructions, which, as I found out, behave differently on aarch64 architectures when compared to x86. Here is the topic that led me to do this: https://bugs.mysql.com/bug.php?id=82760

Thanks. This is a good suggestion. I can confirm that it fixes this test on all three architectures without needing to loosen the tolerance.

Whether it is actually necessary or appropriate to disable floating-point contraction, or whether perhaps the tolerance is simply tighter than it needs to be, seems still to be an open question. For now, I’m going to leave the compiler flags as they are in Fedora, and keep loosening the tolerance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants