sociomantic-tsunami / ocean

General purpose, platform-dependent, high-performance library for D
Other
61 stars 58 forks source link

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

Open joseph-wakeling-frequenz opened 4 years ago

joseph-wakeling-frequenz commented 4 years ago

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: https://github.com/sociomantic-tsunami/ocean/blob/63ef0e886955986a99c89aeb4d0f939149c5975a/src/ocean/math/Elliptic.d#L349

ocean.math.Probability: https://github.com/sociomantic-tsunami/ocean/blob/63ef0e886955986a99c89aeb4d0f939149c5975a/src/ocean/math/Probability.d#L261-L271 https://github.com/sociomantic-tsunami/ocean/blob/63ef0e886955986a99c89aeb4d0f939149c5975a/src/ocean/math/Probability.d#L368

ocean.math.Math: https://github.com/sociomantic-tsunami/ocean/blob/63ef0e886955986a99c89aeb4d0f939149c5975a/src/ocean/math/Math.d#L422 https://github.com/sociomantic-tsunami/ocean/blob/63ef0e886955986a99c89aeb4d0f939149c5975a/src/ocean/math/Math.d#L428-L429 https://github.com/sociomantic-tsunami/ocean/blob/63ef0e886955986a99c89aeb4d0f939149c5975a/src/ocean/math/Math.d#L435 https://github.com/sociomantic-tsunami/ocean/blob/63ef0e886955986a99c89aeb4d0f939149c5975a/src/ocean/math/Math.d#L490 https://github.com/sociomantic-tsunami/ocean/blob/63ef0e886955986a99c89aeb4d0f939149c5975a/src/ocean/math/Math.d#L513-L514 https://github.com/sociomantic-tsunami/ocean/blob/63ef0e886955986a99c89aeb4d0f939149c5975a/src/ocean/math/Math.d#L632 https://github.com/sociomantic-tsunami/ocean/blob/63ef0e886955986a99c89aeb4d0f939149c5975a/src/ocean/math/Math.d#L740 https://github.com/sociomantic-tsunami/ocean/blob/63ef0e886955986a99c89aeb4d0f939149c5975a/src/ocean/math/Math.d#L762 https://github.com/sociomantic-tsunami/ocean/blob/63ef0e886955986a99c89aeb4d0f939149c5975a/src/ocean/math/Math.d#L976 https://github.com/sociomantic-tsunami/ocean/blob/63ef0e886955986a99c89aeb4d0f939149c5975a/src/ocean/math/Math.d#L1504 https://github.com/sociomantic-tsunami/ocean/blob/63ef0e886955986a99c89aeb4d0f939149c5975a/src/ocean/math/Math.d#L1506

don-clugston-sociomantic commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

don-clugston-sociomantic commented 4 years ago

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 commented 4 years ago

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.