Skip to content

Fix A/D tangent basis vectors to canonical (RA, Dec) frame#325

Open
matthewholman wants to merge 2 commits into
mainfrom
fix/ad-tangent-basis-vectors
Open

Fix A/D tangent basis vectors to canonical (RA, Dec) frame#325
matthewholman wants to merge 2 commits into
mainfrom
fix/ad-tangent-basis-vectors

Conversation

@matthewholman
Copy link
Copy Markdown
Collaborator

@matthewholman matthewholman commented May 12, 2026

This fixes an error in the A/D vectors that Adam spotted before I could fix it.

a_vec_from_rho_hat and d_vec_from_rho_hat in src/lib/detection.cpp
were producing unnormalized tangent vectors of a y-pole
parameterization
(rotated ~70-120° about ρ̂ from the canonical
RA/Dec basis):

A_old = (ρ̂_z, 0, -ρ̂_x) (not unit length)
D_old = (-ρ̂_x ρ̂_y, ρ̂_x²+ρ̂_z², -ρ̂_z ρ̂_y) (not unit length)

These are silent under symmetric σ_RA = σ_Dec — rotation
invariance of the diagonal weight matrix in the LM normal equations
hides the mistake — but bias the fit whenever per-observation RA/Dec
uncertainties differ. The basis is also degenerate at ρ̂_y = ±1
(arbitrary point on the celestial equator) rather than the much less
common ρ̂_z = ±1 (the actual celestial poles).

Replaced with the canonical equatorial-tangent basis:

A = ∂ρ̂/∂α = (-sin α, cos α, 0)
D = ∂ρ̂/∂δ = (-sin δ cos α, -sin δ sin α, cos δ)

Both unit length, mutually orthogonal, orthogonal to ρ̂ everywhere
except at the celestial poles.

Adds tests/layup/test_tangent_basis.py (9 tests) that pin the basis
to the canonical formulas and verify the (ρ̂, A, D) frame is
orthonormal at representative points across the sky.

Dependencies

Stacks on feat/pybind11-eigen-includes (#322), which provides the
pybind11/eigen.h / pybind11/stl.h headers required for the
Python-side tests to access obs.a_vec and obs.d_vec.

Review Checklist for Source Code Changes

  • Does pip install still work?
  • Have you written a unit test for any new functions? — tests/layup/test_tangent_basis.py
  • Do all the units tests run successfully?
  • Does Layup run successfully on a test set of input files/databases?
  • Have you used black on the files you have updated? — C++ change; N/A

… Python

Without these, accessing rho_hat / a_vec / d_vec from Python raises
"Unable to convert function return value to a Python type" because the
binding can't see the Eigen and std::array conversions.

This is a minimal enabler (no behavior change); the A/D-vector
correctness fix on fix/tangent-basis-vectors is independent and lives
on its own branch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@matthewholman matthewholman force-pushed the fix/ad-tangent-basis-vectors branch from a516fe6 to d403880 Compare May 14, 2026 21:06
The previous `a_vec_from_rho_hat` and `d_vec_from_rho_hat` produced
unnormalized tangent vectors of a y-pole parameterization (rotated
~70-120° about ρ̂ from the canonical RA/Dec basis):

  A_old = (ρ̂_z, 0, -ρ̂_x)                     (unnormalized)
  D_old = (-ρ̂_x ρ̂_y, ρ̂_x²+ρ̂_z², -ρ̂_z ρ̂_y) (unnormalized)

These are silent under symmetric σ_RA = σ_Dec — rotation invariance
of the diagonal weight matrix in the LM normal equations hides the
mistake — but bias the fit whenever per-observation RA/Dec
uncertainties differ. The basis is also degenerate at ρ̂_y = ±1
rather than at the (much less common) ρ̂_z = ±1.

Replace with the canonical equatorial-tangent basis:

  A = ∂ρ̂/∂α  =  (-sin α, cos α, 0)
  D = ∂ρ̂/∂δ  =  (-sin δ cos α, -sin δ sin α, cos δ)

Both unit length, mutually orthogonal, orthogonal to ρ̂ everywhere
except at the celestial poles ρ̂_z = ±1.

Adds `tests/layup/test_tangent_basis.py` (9 tests) that pin the
basis to the canonical formulas and verify the (ρ̂, A, D) frame is
orthonormal at representative points.

## Dependencies
Stacks on `feat/pybind11-eigen-includes` (PR #322), which adds the
`pybind11/eigen.h` and `pybind11/stl.h` headers required for the
Python-side tests to access `obs.a_vec` and `obs.d_vec`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant