Skip to content

math/cartesian_to_polar: to_polar returns wrong angle for fourth-quadrant points (x>0, y<0) #1582

Description

@aimasteracc

Description

math/cartesian_to_polar.c::to_polar computes the wrong angle for fourth-quadrant points (x > 0 && y < 0). The Q4 branch is:

else if (x > 0 && y < 0)
{  // Q4
    thetaFinal = 2 * M_PI - *theta;   // *theta == atan(y/x), which is already negative here
}

For x > 0, y < 0, *theta = atan(y/x) is already the correct angle (a negative value in (-pi/2, 0), matching atan2(y, x)). The branch instead returns 2*pi - atan(y/x) = 2*pi + |atan(y/x)|, which is greater than 2*pi — neither the principal value atan2 returns nor a valid [0, 2*pi) angle. (For a [0, 2*pi) convention the correct value would be 2*pi + atan(y/x), i.e. 2*pi - |atan|; the code adds instead of subtracting the magnitude.)

The file's own test() asserts fabs(theta - atan2(y, x)) < 0.01, so this branch violates the function's own correctness criterion. The test passes only because it uses a fixed srand(10) whose 5 sample points never land in the pure fourth quadrant (and the Q1 condition's stray x == -y clause accidentally rescues points like (1, -1)).

Steps to reproduce

Compile to_polar and call it on fourth-quadrant points (verified with cc -lm):

(2,-1): to_polar theta=6.7468   atan2=-0.4636   diff=7.2105   <-- WRONG
(1,-3): to_polar theta=7.5322   atan2=-1.2490   diff=8.7813   <-- WRONG
(3,-4): to_polar theta=7.2105   atan2=-0.9273   diff=8.1378   <-- WRONG

Every pure-Q4 point fails the file's own fabs(theta - atan2(y, x)) < 0.01 assertion.

Expected behavior

to_polar(x, y, &r, &theta) sets theta to the polar angle equal to atan2(y, x) (the convention its test checks). For (2, -1) that is -0.4636.

Actual behavior

For Q4 it returns 2*pi - atan(y/x) (e.g. 6.7468 for (2,-1)), which is off by ~2*pi + 2|theta| from the correct angle.

Suggested fix

For Q4, atan(y/x) is already correct (same as Q1), so the branch should not transform it:

else if (x > 0 && y < 0)
{  // Q4
    thetaFinal = *theta;            // atan(y/x) already matches atan2(y, x)
}

(Alternatively, for a strict [0, 2*pi) output use thetaFinal = 2 * M_PI + *theta;.) A test point in the pure fourth quadrant — e.g. to_polar(2, -1, ...) — should be added so the suite covers this branch.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions