rem2pi called with NaN returns constant number instead of NaN

[JKrehl]

Calling rem2pi with a NaN (also NaN16, NaN32, NaN128) returns, depending on the rounding mode, the constants 1.3470949413735402 or -4.936090365806046 which is obviously incorrect.

This also means that mod2pi is incorrect.

[ravibitsgoa]

I think we can solve this bug by checking for NaN argument explicitly in rem2pi.
I would like to work on this.

[ravibitsgoa]

Currently, rem2pi methods are defined in base/math.jl, but the tests for them are in test/numbers.jl. I think they should be moved to test/math.jl too.

[ravibitsgoa]

I think rem2pi(NaN, r) should return NaN and rem2pi(Inf, r) should throw a DomainError. Is it correct behaviour?

Also, should rem(Inf, y) be NaN, as it currently is, or should be a DomainError?

[JKrehl]

For performance reasons alone i would prefer rem2pi returning NaN (or NaN32..., according to input type) whenever an invalid value is passed. Errors are interrupts and cost far more (when not optimized away by the compiler) than a poison value. All float types offer a mathematically sound poison value (i.e. NaN) for this situation so that should be used in my opinion.
I cannot judge how that relates to possibly existing on the use of poison values and errors in numerical computations.

Thank you for working on this. :)

[ravibitsgoa]

I agree about the performance impact of Errors, but most of the trigonometric functions in base/math.jl throw Errors on Inf input (E.g. sin, cos, tan etc.). Although sin(NaN)=NaN.

Hence, I thought it's a good idea to continue that behaviour.

[StefanKarpinski]

These functions are generally expensive enough that the difference between returning nan and throwing an error doesn’t matter.

[KristofferC]

These functions are generally expensive enough that the difference between returning nan and throwing an error doesn’t matter.

With the exception that functions that throw now has a side effect and can't be optimized away even if the result is unused / multiple calls with the same argument can't be folded together etc. It's a pretty big semantic difference.

As an example

julia> @code_llvm ForwardDiff.derivative(sin, 2.0)
;  @ /Users/kristoffercarlsson/.julia/packages/ForwardDiff/cXTw0/src/derivative.jl:13 within `derivative'
define double @julia_derivative_17442(double) {
top:
   %1 = call double @julia_sin_17443(double %0) # <------------ unused
   %2 = call double @julia_cos_17446(double %0)
  ret double %2
}

julia> ForwardDiff.derivative(sin, Inf)
ERROR: DomainError with Inf:
sin(x) is only defined for finite x.

[JeffreySarnoff]

julia> mod2pi.((-Inf, Inf, NaN))
(1.1963318081441687, 5.086853499035418, 1.3470949413735402)