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

Add CRlibm-1.0.0 compatibility #440

Closed
wants to merge 1 commit into from

Conversation

c42f
Copy link

@c42f c42f commented Feb 18, 2021

This allows IntervalArithmetic to be used with the binarybuilder version of CRlibm.

@dpsanders
Copy link
Member

Thanks! I was labouring under the impression that I had already done this, but apparently not...

@coveralls
Copy link

coveralls commented Feb 18, 2021

Coverage Status

Coverage remained the same at 91.08% when pulling 2e6c271 on c42f:cjf/crlibm-update into b70940d on JuliaIntervals:master.

@c42f
Copy link
Author

c42f commented Feb 22, 2021

Are these real failures due to the updated build of crlibm?

x86, julia-1.4, julia-1.5

minimal_log10_test: Test Failed at C:\projects\intervalarithmetic-jl\test\ITF1788_tests\libieeep1788_tests_elem.jl:3460
  Expression: log10(Interval(5.0e-324, 10.0)) == Interval(-323.3062153431158, 1.0)
   Evaluated: Interval(-323.3062153431158, 1.0000000000000002) == Interval(-323.3062153431158, 1.0)
Stacktrace:
 [1] top-level scope at C:\projects\intervalarithmetic-jl\test\ITF1788_tests\libieeep1788_tests_elem.jl:3460
 [2] top-level scope at C:\buildbot\worker\package_win32\build\usr\share\julia\stdlib\v1.5\Test\src\Test.jl:1115
 [3] top-level scope at C:\projects\intervalarithmetic-jl\test\ITF1788_tests\libieeep1788_tests_elem.jl:3446
Test Summary:      | Pass  Fail  Total
minimal_log10_test |   19     1     20

Nightly failures seem unrelated (I think they're JuliaLang/julia#38427 ?)

fma consistency: Error During Test at C:\projects\intervalarithmetic-jl\test\interval_tests\consistency.jl:76
  Test threw exception
  Expression: fma(a, zero(a), c) == c
  could not load symbol "fesetround":
  The specified procedure could not be found. 

@dpsanders
Copy link
Member

Those seem to be real failures from incorrect rounding.

Clearly log10(10.0) should give 1.0 with correct rounding.
x86 means 32-bit I think?

The weird thing is that almost all tests pass except this one. I don't know what's going on or how to fix this, except by
reverting to using MPFR for that function.

The CRlibm library no longer seems to be under development, so there is not even an upstream to report to...

@c42f
Copy link
Author

c42f commented Feb 22, 2021

If this was working in the previous local build (build.jl) of CRLibm and it's not working in the binary builder version, perhaps it's due to compilation flags in the code to create CRLibm_jll ? Then perhaps we should report it to Yggdrasil?

Any thoughts @giordano ?

@giordano
Copy link
Member

Any thoughts @giordano ?

No idea what's the issue, I guess someone needs to investigate whether the build process in BinaryBuilder did something different from what the local build used to do. The log of compilation with BinaryBuilder is in the tarball itself

@c42f
Copy link
Author

c42f commented Feb 22, 2021

Ok thanks.

So I see that prior to JuliaIntervals/CRlibm.jl#41, CRlibm wasn't used on windows — a warning was emitted and things fell back to MPFR.

So this may be the first time that code path has ever been tested with the windows build.

@c42f
Copy link
Author

c42f commented Feb 22, 2021

I believe this is a bug in CRlibm on 32 bit. It's not restricted to windows. On the official linux 86 build of Julia 1.5.3, I have:

julia> ccall((:log10_ru, CRlibm.CRlibm_jll.libcrlibm), Float64, (Float64,), 10.0)
1.0000000000000002

I don't think this is due to the binarybuilder build, I expect it was just never tested before on a 32 bit system.

@dpsanders
Copy link
Member

Thanks for the detective work, @c42f!

How do you suggest we handle this? This is from an automatically-generated test file.

@c42f
Copy link
Author

c42f commented Feb 23, 2021

Well it depends on our appetite for debugging CRlibm ;-)

I did some extra investigation in JuliaIntervals/CRlibm.jl#45 and found that our build of CRlibm is badly broken on 32 bit when using the obvious build options and I didn't find a simple way to fix it. My suggestion would be to fall back to MPFR on all 32 bit systems.

This allows IntervalArithmetic to be used with the binarybuilder version
of CRlibm.  We need CRlibm-1.0.1 at minimum due to an issue with 32 bit
systems.
@lucaferranti
Copy link
Member

done in #503

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

Successfully merging this pull request may close these issues.

None yet

5 participants