rsetienne / DDD

Diversity-Dependent Diversification
3 stars 4 forks source link

Problem on 32-bit Windows #17

Closed jeroen closed 4 years ago

jeroen commented 4 years ago

Hi! R on Windows recently switched to build with -mfpmath=sse -msse2 for the new GCC-8 toolchain. With this compiler flag, numeric results on 32-bit are consistent with 64-bit.

However this change seems to have revealed a bug in DDD that causes an occasional crash during the unit tests when running in 32-bit. I have narrowed down the problem to the two tests that are using bd_loglik:

https://github.com/rsetienne/DDD/blob/467f31eaa0a7664ae87c3a5ecd6b7465f39c7061/tests/testthat/test_DDD.R#L55-L59

If I comment out these tests on 32-bit, everything is OK. Would you have any guess on where in the code you may make memory assumptions on 32-bit floats that may not overlfloat icw/ -mfpmath=sse ?

32-bit is not widely used anymore, so if you think it's not important you can also just skip the test on 32-bit platforms. I see you are already ignoring the result for 32-bit on these platforms, so there I'm guessing this only works on 64-bit anyway.

You can reproduce the problem by downloading r-testing for Windows, or by submitting to R-devel ATC (alternative toolchain) on WinBuilder.

jeroen commented 4 years ago

Update the above post (my earlier diagnosis was incorrect, the problematic function is bd_loglik.

jeroen commented 4 years ago

I think it has to do with this compiler warning that I see when compiling with -Wall:

gfortran  -fPIC  -Wall -g -O2  -c  dd_loglik_rhs_FORTRAN.f95 -o dd_loglik_rhs_FORTRAN.o
dd_loglik_rhs_FORTRAN.f95:305:11:

       En = SUM(Envec)
           1
Warning: Possible change of value in conversion from REAL(16) to REAL(8) at (1) [-Wconversion]

I think the crash happens because you stick a quad precision REAL(16) :: Envec(N - kk) into a double.