Add Bernstein–Khushalani fitter as an engine option in do_fit#323
Open
matthewholman wants to merge 5 commits into
Open
Add Bernstein–Khushalani fitter as an engine option in do_fit#323matthewholman wants to merge 5 commits into
matthewholman wants to merge 5 commits 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>
New module exposing the Bernstein-Khushalani fitter as do_bk_fit(),
returning a layup-native FitResult so it slots into the orbitfit
pipeline. The wrapper:
- Loads liborbfit.so via ctypes (searching LIBORBFIT_PATH, the
repo symlink, and ~/Dropbox/claude_tests).
- Translates layup Observation objects to ObsInput.
- Chains BK parameters and 6x6 covariance to barycentric
ICRS-equatorial Cartesian via the same rotations used by the
diagnostic harness (proj->ec->eq is a pure rotation, so the cov
transform is J cov J^T with an analytic 6x6 J derived from
pbasis_to_bary_eq).
The BK library now defaults to the variational-particle Jacobian, so
no extra mode-setting is needed at the call site. set_ephem and
do_bk_fit handle the rest.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
do_fit now accepts engine={"auto","cartesian","bk"} and a
bk_threshold_AU parameter. The default is "cartesian" so existing
callers see no behavior change; "auto" routes based on the Gauss IOD's
first-solution heliocentric distance (BK above the threshold,
Cartesian below).
Validation: on the diagnostic/scan dataset (98 cases, 88 with all
three engines succeeding), engine="auto" matches the per-case best of
{BK, Cartesian} in 77/88 (87.5%). It beats Cartesian-only on 43 cases
(distant short-arc) and beats BK-only on 16 cases (mainbelt-like).
Remaining ~12% are short-arc TNO cases where Gauss IOD's r estimate
is unreliable and the dispatch picks the wrong engine.
Auto-dispatch falls back to Cartesian if the BK library isn't
loadable or the BK fit returns flag != 0.
Tests skip gracefully when liborbfit.so is absent.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
liborbfit now ships its own ctypes-based Python wrapper at https://github.com/matthewholman/liborbfit (python/liborbfit/), providing Observation, BKFitResult, set_ephem, set_jacobian_mode, and do_bk_fit. layup's bk_orbitfit.py shrinks accordingly: - 349 lines -> 174 lines. Drops everything that's not layup- specific: the ctypes Structure mirrors, the library-discovery search, _load_lib, the αβγ→Cartesian mathematics, the helper rotation matrices. All of that now lives once in liborbfit. - Keeps the layup glue: layup Observation -> liborbfit Observation conversion (_build_obs), liborbfit BKFitResult -> layup FitResult packing (_to_layup_result, using BKFitResult.to_cartesian), the Gauss-IOD-based compute_r_au helper. - Public surface unchanged (set_ephem, do_bk_fit, compute_r_au), so orbitfit.do_fit's existing imports and call sites work as-is. - Module docstring documents the LIBORBFIT_PATH + PYTHONPATH setup needed to enable the BK engine. Previously these instructions lived in create_lib_links.sh; that file was removed when the assist/rebound submodules were switched to wheels on main (50ee49c), so the BK setup docs moved into bk_orbitfit.py. tests/layup/test_bk_dispatch.py: - _liborbfit_available() now imports liborbfit and calls set_jacobian_mode to force a real ctypes load, instead of checking for liborbfit.so on disk. Catches all three failure modes (module not on PYTHONPATH, .so not loadable, ABI mismatch) in one place. Validated: - All 5 tests in test_bk_dispatch.py pass with: LIBORBFIT_PATH=\$HOME/Dropbox/liborbfit/liborbfit.so \\ PYTHONPATH=\$HOME/Dropbox/liborbfit/python:src \\ pytest tests/layup/test_bk_dispatch.py - Without those env vars the tests skip cleanly, so CI on machines without the BK build is unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
32bf6fa to
91539d7
Compare
Pure formatting (def line wrapping, trailing commas in multi-arg calls, blank line after import inside try-block). No behavior change; all 5 tests in tests/layup/test_bk_dispatch.py still pass. 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
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 branch was developed with Claude Code, on top of an initial
implementation I had started.
Adds a Bernstein–Khushalani (BK) fitter as a second engine option in
do_fit. BK parameterizes the orbit in a tangent-plane (α/β/γ) framerather than barycentric Cartesian, which is much better conditioned
for distant short-arc targets where Cartesian-LM is near-degenerate.
do_fitgains anengineparameter:"cartesian"(default, unchanged behavior)"bk"(always use BK)"auto"(route on the Gauss IOD's heliocentric distance:r ≥
bk_threshold_AU→ BK, else Cartesian)The BK fitter is now distributed as a separate library, liborbfit:
https://github.com/matthewholman/liborbfit. It ships both the
liborbfit.soC library and aliborbfitPython package wrapping itvia ctypes. layup's
src/layup/bk_orbitfit.pyis a thin (~174-line)adapter: it converts layup
Observationobjects to liborbfit'sObservation, callsliborbfit.do_bk_fit, and packs theBKFitResultback into a layupFitResult— using liborbfit'sBKFitResult.to_cartesian()for the αβγ→ICRS-equatorial Cartesiantransform with analytic 6×6 covariance propagation.
Setup
Clone and build liborbfit, then export two env vars so layup can load
both the C library (via ctypes) and the Python wrapper:
When the env vars are not set, the BK engine is unavailable: layup's
engine="auto"falls back to Cartesian, and explicitengine="bk"calls return a
FitResultwithflag=3. Setup instructions alsolive in
src/layup/bk_orbitfit.py's module docstring.Validation (diagnostic/scan, 98 cases)
cases (77/88). Routes to BK on 56, Cartesian on 35.
beats BK-only on 16 cases (mainbelt / NEO-like).
tests/layup/test_bk_dispatch.pycovers the dispatch logic; thesuite skips cleanly on machines without liborbfit installed, so CI
is unaffected.
Dependencies
Stacks on
feat/pybind11-eigen-includes(PR #322 — addspybind11/eigen.h+pybind11/stl.hsoObservation.rho_hatworksfrom Python). Once #322 lands, this PR collapses to its 3 new
commits.
Review Checklist for Source Code Changes
tests/layup/test_bk_dispatch.pydiagnostic/scan