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.

Sign up for GitHub

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 sincosd function #30134

Merged
merged 7 commits into from
Aug 9, 2019
Merged

Add sincosd function #30134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
06773a0
Add sincosd function
ronisbr Nov 22, 2018
8905370
Apply suggestions from code review
oxinabox Nov 25, 2018
5d8aa1d
Remove @inline annotation from sincosd
ronisbr Apr 13, 2019
38ef1a3
Add compat annotation for sincosd
ronisbr May 28, 2019
5f20cbe
Add NEWS for sincosd
ronisbr May 28, 2019
c7b7d4e
Merge branch 'master' into sincosd
StefanKarpinski Jun 27, 2019
81632d5
Update NEWS.md
ronisbr Aug 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ New language features

* Support for Unicode 12.1.0 ([#32002]).
* Methods can now be added to an abstract type ([#31916]).
* Added `sincosd(x)` to simultaneously compute the sine and cosine of `x`, where `x` is in degrees ([#30134]).

Language changes
----------------
Expand Down
1 change: 1 addition & 0 deletions base/exports.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,7 @@ export
sin,
sinc,
sincos,
sincosd,
sind,
sinh,
sinpi,
Expand Down
2 changes: 1 addition & 1 deletion base/math.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export sin, cos, sincos, tan, sinh, cosh, tanh, asin, acos, atan,
asinh, acosh, atanh, sec, csc, cot, asec, acsc, acot,
sech, csch, coth, asech, acsch, acoth,
sinpi, cospi, sinc, cosc,
cosd, cotd, cscd, secd, sind, tand,
cosd, cotd, cscd, secd, sind, tand, sincosd,
acosd, acotd, acscd, asecd, asind, atand,
rad2deg, deg2rad,
log, log2, log10, log1p, exponent, exp, exp2, exp10, expm1,
Expand Down
22 changes: 22 additions & 0 deletions base/special/trig.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1073,6 +1073,28 @@ end

tand(x::Real) = sind(x) / cosd(x)

"""
sincosd(x)

Simultaneously compute the sine and cosine of `x`, where `x` is in degrees.

!!! compat "Julia 1.3"
This function requires at least Julia 1.3.
"""
function sincosd(x::Real)
if isinf(x)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't these conditions checked in sind and cosd? Why check here?

Copy link
Member Author

@ronisbr ronisbr Apr 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because without it, then the user will see an error on the function sind if Inf is passed. I though it could lead to confusion.

With respect of NaN, it can be removed. Do you want me to remove it? This was added when I was trying to make a function faster than calling both separated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we do error checking like that elsewhere though.

return throw(DomainError(x, "sincosd(x) is only defined for finite `x`."))
elseif isnan(x)
return (oftype(x,NaN), oftype(x,NaN))
end

# It turns out that calling those functions separately yielded better
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty surprising, any idea why? Part of the point of sincos, is it avoid the double cost of calling sin and cos on the same argument.

Copy link
Member Author

@ronisbr ronisbr Mar 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the compiler is clever enough to remove those duplicated executions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, the sincos_kernel use the same approach and it has a comment saying that inlining takes care of duplicated computations:

julia/base/special/trig.jl

Line 208 in 8221c87

# There's no need to write specialized kernels, as inlining takes care of remo-

# performance than considering each case and calling `sincos_kernel`.
return (sind(x), cosd(x))
end

sincosd(::Missing) = (missing, missing)

for (fd, f, fn) in ((:sind, :sin, "sine"), (:cosd, :cos, "cosine"), (:tand, :tan, "tangent"))
name = string(fd)
@eval begin
Expand Down
15 changes: 15 additions & 0 deletions test/math.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,9 +369,14 @@ end
@testset "degree-based trig functions" begin
@testset "$T" for T = (Float32,Float64,Rational{Int})
fT = typeof(float(one(T)))
fTsc = typeof( (float(one(T)), float(one(T))) )
for x = -400:40:400
@test sind(convert(T,x))::fT ≈ convert(fT,sin(pi/180*x)) atol=eps(deg2rad(convert(fT,x)))
@test cosd(convert(T,x))::fT ≈ convert(fT,cos(pi/180*x)) atol=eps(deg2rad(convert(fT,x)))

s,c = sincosd(convert(T,x))
@test s::fT ≈ convert(fT,sin(pi/180*x)) atol=eps(deg2rad(convert(fT,x)))
@test c::fT ≈ convert(fT,cos(pi/180*x)) atol=eps(deg2rad(convert(fT,x)))
end
@testset "sind" begin
@test sind(convert(T,0.0))::fT === zero(fT)
Expand All @@ -387,6 +392,16 @@ end
@test cosd(convert(T,-90))::fT === zero(fT)
@test cosd(convert(T,-270))::fT === zero(fT)
end
@testset "sincosd" begin
@test sincosd(convert(T,-360))::fTsc === ( -zero(fT), one(fT) )
@test sincosd(convert(T,-270))::fTsc === ( one(fT), zero(fT) )
@test sincosd(convert(T,-180))::fTsc === ( -zero(fT), -one(fT) )
@test sincosd(convert(T, -90))::fTsc === ( -one(fT), zero(fT) )
@test sincosd(convert(T, 0))::fTsc === ( zero(fT), one(fT) )
@test sincosd(convert(T, 90))::fTsc === ( one(fT), zero(fT) )
@test sincosd(convert(T, 180))::fTsc === ( zero(fT), -one(fT) )
@test sincosd(convert(T, 270))::fTsc === ( -one(fT), zero(fT) )
end

@testset "sinpi and cospi" begin
for x = -3:0.3:3
Expand Down