Skip to content

Add Bernstein–Khushalani fitter as an engine option in do_fit#323

Open
matthewholman wants to merge 5 commits into
mainfrom
feat/bk-engine-dispatch
Open

Add Bernstein–Khushalani fitter as an engine option in do_fit#323
matthewholman wants to merge 5 commits into
mainfrom
feat/bk-engine-dispatch

Conversation

@matthewholman
Copy link
Copy Markdown
Collaborator

@matthewholman matthewholman commented May 12, 2026

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 (α/β/γ) frame
rather than barycentric Cartesian, which is much better conditioned
for distant short-arc targets where Cartesian-LM is near-degenerate.

do_fit gains an engine parameter:

  • "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.so C library and a liborbfit Python package wrapping it
via ctypes. layup's src/layup/bk_orbitfit.py is a thin (~174-line)
adapter: it converts layup Observation objects to liborbfit's
Observation, calls liborbfit.do_bk_fit, and packs the
BKFitResult back into a layup FitResult — using liborbfit's
BKFitResult.to_cartesian() for the αβγ→ICRS-equatorial Cartesian
transform 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:

git clone https://github.com/matthewholman/liborbfit
cd liborbfit
./setup_deps.sh && make
export LIBORBFIT_PATH=$PWD/liborbfit.so
export PYTHONPATH=$PWD/python:$PYTHONPATH

When the env vars are not set, the BK engine is unavailable: layup's
engine="auto" falls back to Cartesian, and explicit engine="bk"
calls return a FitResult with flag=3. Setup instructions also
live in src/layup/bk_orbitfit.py's module docstring.

Validation (diagnostic/scan, 98 cases)

  • 87/88 cases succeed with all three engines (cartesian / bk / auto)
  • Auto matches the per-case best of {BK, Cartesian} in 87.5% of
    cases (77/88). Routes to BK on 56, Cartesian on 35.
  • Auto beats cartesian-only on 43 cases (distant short-arc) and
    beats BK-only on 16 cases (mainbelt / NEO-like).

tests/layup/test_bk_dispatch.py covers the dispatch logic; the
suite skips cleanly on machines without liborbfit installed, so CI
is unaffected.

Dependencies

Stacks on feat/pybind11-eigen-includes (PR #322 — adds
pybind11/eigen.h + pybind11/stl.h so Observation.rho_hat works
from Python). Once #322 lands, this PR collapses to its 3 new
commits.

Review Checklist for Source Code Changes

  • Does pip install still work?
  • Have you written a unit test for any new functions? — tests/layup/test_bk_dispatch.py
  • Do all the units tests run successfully?
  • Does Layup run successfully on a test set of input files/databases? — full diagnostic/scan
  • Have you used black on the files you have updated? — formatting not strictly applied yet

matthewholman and others added 4 commits May 14, 2026 10:08
… 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>
@matthewholman matthewholman force-pushed the feat/bk-engine-dispatch branch from 32bf6fa to 91539d7 Compare May 14, 2026 15:11
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>
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