Wrap2pi: what should it do?

Fell free not to follow my suggestions but

  1. with floating point numbers you should really use isapprox, not ==
  2. with your definition wra2pi is type-unstable
  3. you get that because for all Irrational the equality == with Real is defined to be always false: julia/base/irrationals.jl at 4633607ce9b9f077f32f89f09a136e04389bbac2 · JuliaLang/julia · GitHub. Which brings you again to point 1
3 Likes

Any specific reason you define a dedicated function for this?

julia> using IntervalSets

julia> mod(10, -π..π)
-2.5663706143591725

# can choose between open and closed intervals easily:
julia> mod(3π, iv"[-π, π]")
-3.141592653589793

julia> mod(3π, iv"[-π, π)")
-3.141592653589793

julia> mod(3π, iv"(-π, π]")
3.141592653589793

is both more general and less error-prone.

3 Likes

Well, I do not want to add packages to my project unless really needed. I use now:

"""
    wrap2pi(angle)

Limit the angle to the range -π .. π .
"""
wrap2pi(::typeof(pi)) = π
function wrap2pi(angle)
    y = rem(angle, 2π)
    abs(y) > π && (y -= 2π * sign(y))
    return y
end

which passes my tests:

    @test wrap2pi(0.0)   == 0.0
    @test wrap2pi(2π)    == 0.0
    @test wrap2pi(3π)    == float(π)
    @test wrap2pi(-2π)   == 0.0
    @test wrap2pi(-3π)   == float(-π)
    @test wrap2pi(π)     == π
    @test wrap2pi(3.14)  == 3.14
    @test wrap2pi(3.15)  <  0.0
    @test wrap2pi(-3.15) >  0.0
    @test wrap2pi(-3.14) == -3.14
    @test wrap2pi(-π)    == -π

I prefer to use == instead of isapprox if it works. And here it works just fine. The code is also simple and fast, about the same performance as sin().

Beware of false expectations though. Since wrap2pi(-π) == -π holds, then -wrap2pi(-π) == π holds too, right? (Warning: it does not.) Also, from your nine test cases, only one uses the Irrational method.

If I may take the liberty of interpreting giordano, he’s saying that you should embrace that you are, in fact, working with floating-point numbers and be explicit about what tolerances are acceptable for you. For example, the following passes:

wrap2pi(x::typeof(π)) = rem2pi(float(x), RoundNearest)
wrap2pi(x) = rem2pi(x, RoundNearest)

@test wrap2pi(0.0)   ≈ 0.0       atol=1e-7
@test wrap2pi(2π)    ≈ 0.0       atol=1e-7
@test wrap2pi(3π)    ≈ float(π)  atol=1e-7
@test wrap2pi(-2π)   ≈ 0.0       atol=1e-7
@test wrap2pi(-3π)   ≈ float(-π) atol=1e-7
@test wrap2pi(π)     ≈ π         atol=1e-7
@test wrap2pi(3.14)  ≈ 3.14      atol=1e-7
@test wrap2pi(-3.14) ≈ -3.14     atol=1e-7
@test wrap2pi(-π)    ≈ -π        atol=1e-7
# And this passes too:
@test -wrap2pi(-π) ≈ π atol=1e-7
1 Like

Is this not an example of a “really needed” situation?

No. I have this function (written slightly differently) since years in my code, and it works well. I decided to move it to a common package, KiteUtils.jl, added some tests and improved it a little bit. No reason whatsoever to add another package (my project already depends on 500 packages).

Is there any particular reason why you expect the signs of the odd multiples to be conserved, or is this just a regression test of the signs you have observed?