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

Allow small deviation in builtin tests #1148

Closed
wants to merge 1 commit into from
Closed

Conversation

jpf91
Copy link
Contributor

@jpf91 jpf91 commented Sep 27, 2012

On systems where real is just an alias for double (e.g. ARM) the exact literal values might not be representable, so allow a deviation of real.epsilon.

On systems where real is just an alias for double (e.g. ARM)
the exact literal values might not be representable, so allow
a deviation of real.epsilon.
@yebblies
Copy link
Member

yebblies commented Oct 7, 2012

Do we really need to loosen the tests? Could it be done using a version statement to compare with the exact value double precision produces on those platforms?

@jpf91
Copy link
Contributor Author

jpf91 commented Oct 10, 2012

Probably, but I'm not sure if we can expect exactly the same result on all CPUs (even if they have the same architecture). There are lots of floating point implementations on ARM (vfp1-3, VFPLite, FPA, FPE, iwMMXt, software emulated) and I don't know if they all produce the same results for sin/cos/...

@donc
Copy link
Collaborator

donc commented Oct 11, 2012

Actually, the existing tests are wrong. x86 is wrong in the last two bits.
static assert(sin(6.8) == 0x1.f9f8d9aea10fdf1cp-2); Should be:
static assert(sin(6.8) == 0x1.f9f8d9aea10fdf10p-2); // precise value is 0x1.f9f8d9aea10fdf10d323f9c7p-2
But this will fail, so it will need the epsilon test.

Likewise cos(6.8) should be:
0x1.bd21aaf88dcfa13ap-1 // existing
0x1.bd21aaf88dcfa13ep-1 // new
0x1.bd21aaf88dcfa13d1879e9a5f2f02p-1 // precise

tan(6.8) should be:
0x1.22fd752af75cd08cp-1 // existing
0x1.22fd752af75cd084p-1 // new
0x1.22fd752af75cd083fbe4115c995ep-1 //precise

Please change the tests to use the 'new' numbers, with the epsilon tests, and include the comment below in the file.
/*
Precise values:
sin(6.8) = 0x1.f9f8d9aea10fdf10d323f9c7p-2
cos(6.8) = 0x1.bd21aaf88dcfa13d1879e9a5f2f02p-1
tan(6.8) = 0x1.22fd752af75cd083fbe4115c995ep-1
*/

@jpf91
Copy link
Contributor Author

jpf91 commented Nov 6, 2012

Closed. I'll reopen this pull when I've implemented Don's suggestions.

@jpf91 jpf91 closed this Nov 6, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants