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

sinpi fails on bigfloats #39

Open
fph opened this issue Sep 7, 2020 · 13 comments
Open

sinpi fails on bigfloats #39

fph opened this issue Sep 7, 2020 · 13 comments

Comments

@fph
Copy link

fph commented Sep 7, 2020

sinpi fails on BigFloats:

julia> CRlibm.sinpi(BigFloat(0.5))
ERROR: StackOverflowError:
Stacktrace:
 [1] sinpi(::BigFloat, ::RoundingMode{:Nearest}) at /home/sad/.julia/packages/CRlibm/NFCH5/src/CRlibm.jl:137 (repeats 79984 times)
@dpsanders
Copy link
Member

Sorry I didn't see this before.
CRlibm is only designed to work with arguments of type Float64.

@dpsanders
Copy link
Member

So I guess that should just error.

@fph
Copy link
Author

fph commented Sep 23, 2020

Thanks! Then I guess the issue is with IntervalArithmetic.jl, because sin(@biginterval(0.5)), which appears in their test test/interval_tests/trig.jl, seems to use CRlibm.sin on BigFloat arguments internally. I guess I should report it there then, do you confirm?

@dpsanders
Copy link
Member

Where do you see that? It seems to work for me:

julia> sinpi(@biginterval(0.5))
[0.999999, 1]₂₅₆

@fph
Copy link
Author

fph commented Sep 28, 2020

I confirm I can replicate it on my machine. Ubuntu Linux 20.04, JuliaPro v1.5.0, CRlibm v0.8.0.

 Activating environment at `~/.julia/environments/JuliaPro_v1.5.0-1/Project.toml`
Starting Julia...
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.5.0 (2020-08-01)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

julia> using CRlibm

julia> Pkg.status("CRlibm")
Status `~/.julia/environments/JuliaPro_v1.5.0-1/Project.toml`
  [96374032] CRlibm v0.8.0

julia> CRlibm.sinpi(BigFloat(0.5))
ERROR: StackOverflowError:
Stacktrace:
 [1] sinpi(::BigFloat, ::RoundingMode{:Nearest}) at /home/sad/.julia/packages/CRlibm/NFCH5/src/CRlibm.jl:137 (repeats 79984 times)

@dpsanders
Copy link
Member

I meant where do you see that sinpi(x) for x::Interval{BigFloat} calls CRlibm.sin on a BigFloat.
sinpi(x) for a BigFloat Interval works, so I don't see how it can be calling this method.

But now I see that CRlibm.sin(y) does work when y is a BigFloat, so CRlibm.sinpi should too, I guess. (Or neither should work, preferably!)

@dpsanders
Copy link
Member

Ah I see, it was to define methods with arguments specifying the rounding mode, like
CRlibm.sin(big(0.5), RoundDown)

@fph
Copy link
Author

fph commented Sep 28, 2020

sinpi(@biginterval(0.5)) works on my machine, too, though.

@orkolorko
Copy link

orkolorko commented Mar 3, 2021

with @fph we were debugging an issue with our package InvariantMeasures.jl.
It seems like the fallback to MPFR raises this StackOverFlow error.

In other words, my installation of CRlibm was shadowing MPFR:

julia> CRlibm.sinpi(8.0, RoundDown)
ERROR: StackOverflowError:
Stacktrace:
[1] sinpi(::BigFloat, ::RoundingMode{:Down}) at /home/isaia/.julia/packages/CRlibm/NFCH5/src/CRlibm.jl:137 (repeats 79984 times)

julia> CRlibm.setup(false)
┌ Warning: CRlibm is falling back to use MPFR; it will have
│         the same functionality, but with slower execution.
│         This is currently the only option on Windows.
└ @ CRlibm ~/.julia/packages/CRlibm/NFCH5/src/CRlibm.jl:24
[ Info: CRlibm is shadowing MPFR.
true

Once I ran:

pkg> build CRlibm

julia> using CRlibm

julia> CRlibm.setup(false)
false

julia> CRlibm.sinpi(8.0, RoundUp)
0.0

things started working.

@dpsanders
Copy link
Member

I don't understand what is going on here.
Which version of CRlibm do you have?

] st CRlibm

And which OS?

Did you try in a new Julia session?

@orkolorko
Copy link

Hi, I'm on Ubuntu 20.10, CRlibm 1.0.1.

I think I found the reason: in CRlibm.jl line 152 we have

const MPFR_function_names = split("""exp expm1 log log1p log2 log10
sin cos tan asin acos atan
sinh cosh 
""")

so, no fallback MPFR function is defined for sinpi and cospi. Adding them to the list gives rise to an error in the test, seems like the output of MPFR is not tight enough.

@dpsanders
Copy link
Member

Thanks.

I think the problem is that MPFR does not seem to have a sinpi function implemented?

@orkolorko
Copy link

orkolorko commented Mar 3, 2021

Yes, it is true; when calling sinpi on BigFloat an helper function is called

@which sinpi(BigFloat(1))
sinpi(x::T) where T<:AbstractFloat in Base.Math at special/trig.jl:751

The function above assumes RoundToNearest when reducing x to x/pi and x/pi to [-1, 1]. I think this may be the reason why

sinpi(@biginterval(1))

is defined as

function sinpi(a::Interval{T}) where T
    isempty(a) && return a
    w = a * Interval{T}(π)
    return(sin(w))
end

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