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

Many ocean.math unittests fail when built with LDC #730

Open
joseph-wakeling-frequenz opened this issue Aug 27, 2019 · 5 comments
Open

Many ocean.math unittests fail when built with LDC #730

joseph-wakeling-frequenz opened this issue Aug 27, 2019 · 5 comments

Comments

@joseph-wakeling-frequenz
Copy link
Contributor

I've given a go to building ocean v5.0.x tests with ldmd2 instead of dmd (v5.0.x because it includes the getIeeeFlags fix that was a blocker to using compilers other than DMD). The command used was:

DC=ldmd2 make V=1 test ALLOW_DEPRECATIONS=1

... with LDC 1.16.0 being used to test (matching DMD 2.086.1).

My suspicion (not yet confirmed) is that all of these reflect LDC using different core math routines to DMD (e.g. implementation of cos, sin, sqrt, etc.). I suspect that similar results might be obtained for GDC, for similar reasons.

No worries if this is a WONTFIX (or at least very low priority) from the maintainers' side, but since I went through this all systematically today I thought I'd share the results.

The following unittests all fail when LDC is used:

ocean.math.Elliptic:

test(ellipticEComplete(1-x) == ellipticE(PI_2, x) );

ocean.math.Probability:

test(isfeqabs(studentsTDistributionInv(10, 0.995), 3.169_272_67L, 0.000_000_005L));
test(isfeqabs(studentsTDistributionInv(8, 0.6), 0.261_921_096_769_043L,0.000_000_000_05L));
// -TINV(2*0.4, 18) == -0.257123042655869
test(isfeqabs(studentsTDistributionInv(18, 0.4), -0.257_123_042_655_869L, 0.000_000_000_05L));
test( feqrel(studentsTDistribution(18, studentsTDistributionInv(18, 0.4L)),0.4L)
> real.mant_dig-5 );
test( feqrel(studentsTDistribution(11, studentsTDistributionInv(11, 0.9L)),0.9L)
> real.mant_dig-2);

test(fabs((1-fDistribution(12, 23, 0.1)) - 0.99990562845505L)< 0.0000000000005L);

ocean.math.Math:

if (!isIdentical(r, t)) test(fabs(r-t) <= .0000001);

ocean/src/ocean/math/Math.d

Lines 428 to 429 in 63ef0e8

if (!isIdentical(r, t) && !(isNaN(r) && isNaN(t)))
test(fabs(r-t) <= .0000001);

test(isIdentical( tan(NaN(0x0123L)), NaN(0x0123L) ));

test(sin(2.0+0.0i) == sin(2.0L) );

ocean/src/ocean/math/Math.d

Lines 513 to 514 in 63ef0e8

test(cos(1.3L+0.0i)==cos(1.3L));
test(cos(5.2Li)== cosh(5.2L));

test(asin(sin(0+0i)) == 0 + 0i);

test(sinh(4.2L + 0i)==sinh(4.2L));

test(cosh(8.3L + 0i)==cosh(8.3L));

test(cfeqrel(sqrt(4+16i)*sqrt(4+16i), 4+16i)>=real.mant_dig-2);

test(exp(1.3e5Li)==cos(1.3e5L)+sin(1.3e5L)*1i);

test(exp(7.2 + 0.0i) == exp(7.2L));

@don-clugston-sociomantic
Copy link
Contributor

Some of these tests seems extremely reasonable, and shouldn't be failing.
test(asin(sin(0+0i)) == 0 + 0i);
But, the code for almost all of this should be the same as Phobos (the Phobos code was taken from Tango and improved, that code was copied from Phobos1 and improved)
It should be just copy-and-paste to make it work.

@joseph-wakeling-frequenz
Copy link
Contributor Author

TBH I wonder if we shouldn't just import/alias from phobos try to drop as much as possible of ocean.math. But it would be nice to work out why we get these failures, given (as you say) that they should not reasonably be happening.

@Geod24
Copy link
Contributor

Geod24 commented Sep 12, 2019

At the moment dmd-transitional does not include Phobos (on purpose) so a simple alias isn't possible.

@don-clugston-sociomantic
Copy link
Contributor

Honestly I'm horrified at some of the changes that were made to this module in Phobos. There's some good fixes which we want, that's all.

@joseph-wakeling-frequenz
Copy link
Contributor Author

Could we have a chat about that some time so that I get a good list of what those horror changes are? I remember approxEqual but that's the only one.

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

No branches or pull requests

3 participants