Add pybind11 eigen/stl includes so Observation.rho_hat is usable from…#322
Open
matthewholman wants to merge 1 commit into
Open
Add pybind11 eigen/stl includes so Observation.rho_hat is usable from…#322matthewholman wants to merge 1 commit into
matthewholman wants to merge 1 commit into
Conversation
… 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>
This was referenced May 12, 2026
matthewholman
added a commit
that referenced
this pull request
May 14, 2026
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>
matthewholman
added a commit
that referenced
this pull request
May 14, 2026
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>
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This was developed with Claude code. It goes with another two PRs.
PR 1 — feat/pybind11-eigen-includes
Title: Add pybind11 eigen/stl includes so Observation.rho_hat is usable from Python
Adds
pybind11/eigen.handpybind11/stl.htosrc/lib/detection.cpp.Without these, accessing
Observation.rho_hat,a_vec, ord_vecfrom Python raises "Unable to convert function return value to a
Python type" because the binding can't see the Eigen and
std::arrayconversion machinery.
Mechanical enabler with no behavior change. Required by downstream
Python code in two follow-up PRs:
feat/bk-engine-dispatch— readsobs.rho_hatinbk_orbitfit.pyfeat/iod-picker-pipeline— readsobs.rho_hatin the IOD prefilterThis is the same include change carried by the unmerged
fix/tangent-basis-vectorsbranch, factored out so it can landindependently of the A/D formula correctness fix.
Review Checklist for Source Code Changes