Wrap2pi: what should it do?

I wrote the following function:

"""
    wrap2pi(angle)

Limit the angle to the range -π .. π .
"""
function wrap2pi(angle)
    if angle == pi
        return pi
    end
    num2pi = floor(angle / 2π + 0.5)
    angle - 2π * num2pi
end

I am now writing tests for it, and I am not sure how it should behave:

  • return pi for pi+2n pi ?
  • or return -pi for pi+2n pi?

What would you suggest?

1 Like

I think such functions should restrict the answer to the half-open interval (-\pi, \pi]

But then the function above is wrong. How could I fix it?

Why not using rem2pi(x, RoundNearest) instead?

2 Likes

Well, it makes my tests fail:

  Test threw exception
  Expression: wrap2pi(π) == π
  MethodError: no method matching rem2pi(::Irrational{:π}, ::RoundingMode{:Nearest})

That doesn’t look hard to fix with a float around pi.

This is type unstable when you call with pi, you get that:

julia> wrap2pi(pi)
π = 3.1415926535897...

julia> typeof(ans)
Irrational{:π}

julia> wrap2pi(float(pi))
-3.141592653589793

julia> wrap2pi(2pi)
0.0

julia> typeof(ans)
Float64

Was it intentional to be type-unstable (well it’s not type-unstable, unless your calling code is, so then in practice), for pi (as that type) to get that back? Since it doesn’t seem rather useful when used with floats, and you pay for that never true test(?).

Why are you making that function in the first place, and if, not wrapping to 0 …2pi? And either way, you might want to have your units in multiples of pi, and limit to 0…2, and use sinpi?

Faster because (no check and) no longer division, and probably still accurate enough:

function wrap2pi(angle)
  num2pi = floor(angle * (1/2π) + 0.5)
  angle - 2π * num2pi
end

Also fails my tests:

KiteUtils.jl: Transformations: Test Failed at /home/ufechner/repos/KiteUtils.jl/test/runtests.jl:144
  Expression: wrap2pi(π) == π
   Evaluated: -3.141592653589793 == π

Of course I could change my tests. No convincing solution yet.

You’re most likely going to end up converting from pi to a float anyway, just return the converted version and change your tests.

1 Like

Even this fails:

julia> rem2pi(2pi, RoundNearest) ≈ 0
false

You know isapprox(x, 0) is equivalent to x == 0, right? Related recent topic: History of `isapprox` in languages

Matlab has a similar function and it outputs over closed interval [-pi, pi]:

Another alternative:

function wrap2pi(x)
   y = rem(x, 2π)
   abs(y) > π && (y -= 2π * sign(y))
   return y
end

So how shall I write the test then?

Just adjust the tolerance, as discussed in that linked thread already.

1 Like

Thank you!

This code passes my test without the need to add any tolerances or conversions.

# """
#     wrap2pi(angle)

# Limit the angle to the range -π .. π .
# """
function wrap2pi(angle::Irrational)
    if angle == π
        return π
    else
        return wrap2pi(float(angle))
    end
end
function wrap2pi(angle)
    y = rem(angle, 2π)
    abs(y) > π && (y -= 2π * sign(y))
    return y
end
julia> wrap2pi(ℯ)

julia>
1 Like

Fixed.

I guess I’m missing why not just defining

wrap2pi(::typeof(pi)) = float(pi)

The branch if angle == -π is totally useless since it’ll never be hit (unless someone defines a custom -pi irrational number).

2 Likes

I fixed the branch that was never hit.

I am not following your suggestion otherwise because

float(pi) == pi
false

This seems simplest to me:

function wrap2pi(x)
    y = rem2pi(x, RoundNearest)
    isequal(y,-pi) && return float(pi)
    return y
end
wrap2pi(::Irrational{:π}) = pi
1 Like