JuliaGeometry / JuliaGeometry/CoordinateTransformations.jl

Tricky interface for `AffineMap(::LinearMap, ::Vector)`

Open
#90 3 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Julia
Stars
185
Forks
24
PR merge metrics
No merged PRs in 30d

Description

I just encountered a very tricky silent bug while slinging around AffineMap, LinearMap, Transformation etc.

In summary, I have some 3D rotation matrices stored as LinearMap and tried to combine it with a translation stored as Vector. Spot the error in this code:

using CoordinateTransformations, Rotations
rot = LinearMap(RotY(1/2*pi))
# ...
loc = rand(3)
# ...
pose_map = AffineMap(rot, loc)
# ...
f(pmap::AffineMap) = ...
@test f(pose_map) == test_value # <- compiles just fine, but result completely wrong

It turns out that AffineMap(::LinearMap, ::Vector) actually calls an overloaded function AffineMap(::Transformation, ::Any) and returns an AffineMap just fine, but with AffineMap.v == zeros(3)!
https://github.com/JuliaGeometry/CoordinateTransformations.jl/blob/b97c74a35c6835f6b015440cb4af6298824dbe1d/src/affine.jl#L121-L125
I.e.

# continued
@assert pose_map.linear == rot.linear # true
@assert pose_map.translation == loc # false. instead
@assert pose_map.translation == zeros(length(loc)) # <- 🤯

This is a super tricky bug, as it's completely silent, and occurs from a "misuse" of the interface that is very subtle.

Perhaps it would make sense to rename the function

function AffineMap(trans::Transformation, x0)
    dT = transform_deriv(trans, x0)
    Tx = trans(x0)
    AffineMap(dT, Tx - dT*x0)
end

to something like AffineMapApprox (or something similar), although I realize that would be a breaking change.
Alternatively, we could overload AffineMap(::LinearTransformation, ::Any), e.g. giving a warning like

@warn "AffineMap(rot::LinearTransformation, ::Any) might not do what you want. Try AffineMap(rot.linear, x) instead."

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/affine.jl at the AffineMap(::Transformation, ::Any) definition around lines 121–125, then reproduce the example using LinearMap, RotY, and a Vector in Julia. Compare the resulting linear and translation fields with the supplied assertions. Done means the constructor’s behavior for this interface is no longer silently wrong, with the intended API behavior covered by a regression test.

Written by the indexing model from the issue text.

Assessment

Tech stack
julia
Domain
api
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.