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

sincos leads to stackoverflow #44

Closed
AwesomeQuest opened this issue Mar 11, 2024 · 3 comments
Closed

sincos leads to stackoverflow #44

AwesomeQuest opened this issue Mar 11, 2024 · 3 comments

Comments

@AwesomeQuest
Copy link

AwesomeQuest commented Mar 11, 2024

Because of this bit of code

_sincos(x::AbstractFloat) = sincos(x)
_sincos(x) = (sin(x), cos(x))

sincos(x) = _sincos(float(x))

in
julia\base\special\trig.jl

Calling sincos with a MultiFloat causes a stackoverflow.

Minimum working example

using MultiFloats
sincos(Float64x2(4))
ERROR: StackOverflowError:
Stacktrace:
     [1] _sincos(x::MultiFloat{Float64, 2})
       @ Base.Math .\special\trig.jl:203
     [2] sincos(x::MultiFloat{Float64, 2})
       @ Base.Math .\special\trig.jl:206
--- the last 2 lines are repeated 39990 more times ---
 [79983] _sincos(x::MultiFloat{Float64, 2})
       @ Base.Math .\special\trig.jl:203

I'm trying to use conv from DSP.jl and GenericFFT.jl, is there a workaround for this? I don't have a firm grasp on how to change the method used by conv in a scenario like this.

P.S. Why would one write code like the above, it seems designed to make a stack overflow in the case that calling float does nothing

@dzhang314
Copy link
Owner

Hi @AwesomeQuest, thanks for your interest in MultiFloats.jl! This is very strange code in Base, and I don't understand why Base.sincos is implemented this way, either. For a workaround, you can simply overload Base.sincos yourself, by adding the following code after you import MultiFloats:

Base.sincos(x::MultiFloat{T,N}) where {T,N} = MultiFloat{T,N}.(
    MultiFloats._call_big(sincos, x, precision(MultiFloat{T,N}) + 20))

This will call the BigFloat implementation of sincos. Here, the + 20 specifies a number of extra bits of precision -- you can set it lower for a (marginal) speed improvement, but I don't recommend setting it higher, since any additional bits are likely to be cut off when re-converting back to Float64xN.

@dzhang314 dzhang314 reopened this Mar 11, 2024
@dzhang314
Copy link
Owner

Whoops, accidentally closed in my last comment.

I'll also add overloads for sincos, sincosd, and sincospi in the next patch release for MultiFloats.jl, which will automatically be defined when you call use_bigfloat_transcendentals, so this manual workaround won't be necessary in the future. Thanks for pointing this out!

@dzhang314
Copy link
Owner

Fixed in commit 1d54726.

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

2 participants