Repository navigation
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
dmsdeg accumulated in float, giving up to 0.05 arcsec of RA error; results for short arcs move by a fraction of their quoted uncertainty. The 80-column MPC reader also dropped the last RA/Dec digit and lost the signs of satellite positions on two-line records. Co-authored-by: Cursor <cursoragent@cursor.com>
Free all work matrices in predict, predict_helio, mrqfit and on error paths; return NaN rather than stale results on failure; check fopen; bound the observation count; initialise pivots in ludcmp/gaussj; fix the kbo2d/kbo3d NULL checks and ec_to_proj partials; keep orbitElements finite for circular and ecliptic orbits. A degenerate linear fit now falls back to the energy-constrained fit instead of exiting. Co-authored-by: Cursor <cursoragent@cursor.com>
The C library no longer calls exit(). Errors deep in a calculation (out-of-range ephemeris dates, Numerical Recipes failures, bad observatory files, unbound orbits in elements_to_xv) call orbfit_fail(), which longjmps back to a guard in each entry point. The guard frees all Numerical Recipes allocations (now tracked in a list), resets the orbit integrator, and returns NaN results. Input errors are recorded with orbfit_error() and propagated through return codes. The message is available from orbfit_last_error(). The ephemeris reader checks its reads, can be retried after a failed initialization, and seeks to records by absolute position. bk_orbit.py loads the library once, raises BKOrbitError with the library's message, and holds a module lock around every call because the library keeps its state in static storage. Remove the unused read_options() command-line parser. Co-authored-by: Cursor <cursoragent@cursor.com>
Build with -fvisibility=hidden and mark the entry points used through ctypes (and utc_jd_to_tt, used by the tests) with ORBFIT_API in orbfit_api.h, so the generic Numerical Recipes names and other internals cannot collide with other libraries loaded in the Python process. Make file-local helpers and state static, remove the unused accel() and write_obs(), and send the ephemeris debug output to stderr. Co-authored-by: Cursor <cursoragent@cursor.com>
The .abg file is now written at full precision, so a fit on Linux x86-64 differs from the committed macOS-generated file by up to ~4e-13 relative instead of being hidden by 8-decimal rounding. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@jmpetit this PR on the liborbfit attempts to address a number of issues in the library, likely things you would be concerned about for space based observations. Would you like to review the code changes? |
|
I had a look at the proposed changes, they look good to me. I tried to install and run the code. All unittests in test/ ran fine, once I understood how to run them. Since the only "documentation" is the README file, that contains very little indications, I decided to run the srcipt/example.py script. As is, it doesn't work. It was written in python2, and the print statement uses the old "statement" format rather than the new function format. When I changed the call to print in the code on my computer, I ran it with all test/data/*.mpc files, almost fine. In the prediction, when changing time from end of 2028 to start of 2029, get warning from ERFA function "taiutc": %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% Probably nothing we can do, just annoying. It's because this is too far in the future. Print(orb.summarize()) has a weird output, besides missing some carriage returns: %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% 2022 08 31.35746 -0.08 +0.09 # FbAS2001 C2022 08 31.35746 22 24 20.536-11 34 27.54 568 arc length 2.115639999974519 d Expected accuracy on 2026-10-09 14:20:16.256925: 0.11 arcsec 0.11 arcsec moving at 2.15 arcsec / h The ephemeris uncertainty reported on line 4 is actually the one for prediction at the current date, but not mentionned. The uncertainty given on the last line should be that of the current date, but it's not. A "carriage return" before "ephemeris uncertainty" and "Expected accuracy" would help reading output. More important, I noticed that if one doesn't run orb.summarize, then the following orb.predict will fail: %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% During handling of the above exception, another exception occurred: Traceback (most recent call last): That's weird, given that first thing done in orb.summarize is to call self.predict... We need an explanation that summarize or something else is needed before predict, or make sure that this something is either called at start of predict, or when creating the NKOrbit object. JM PS: I cannot push the changes I made on my local copy; branch review-pr26 is not visible on github. |
This branch has not been deployed
2 participants
Summary
This hardens the C orbit-fitting library that
mp_ephem.BKOrbitcalls through ctypes. Results change slightly because RA/Dec are now parsed in double precision, so this should be released as a major version. All other changes leave the fitted numbers bit-for-bit the same.c-checksjob compiles every C file with-std=gnu2x -Wall -Wextra -Werror, runs a standalone C driver under AddressSanitizer, UBSan and LeakSanitizer, and runs pytest against a sanitizer build. New tests pin the o3o08 fit, check error reporting and thread safety, and check the exported symbols.dmsdegparsed sexagesimal RA/Dec throughfloat, which caused errors up to 0.05 arcsec. It now usesdoubleand agrees with astropy to about 1e-10 arcsec..abgfiles are written with%.17g. The o3o08 semi-major axis moves by 0.0018 au and the 2016 RA prediction by 0.22 arcsec, both well inside the fit uncertainties. Pinned values intest_orbit.pywere updated.+. Dates, RA/Dec and observatory codes are now read by column.MAXOBSbounds, uncheckedfopen/fread, NULL derivative pointers inkbo2d/kbo3d, wrong partials inec_to_proj,acosdomain errors inorbitElements, and pointer arithmetic before the start of an allocation innrutil.exit(): errors inside the library used to end the whole Python process. Each entry point now sets asetjmpguard. A failure frees all live allocations, resets the integrator and returns NaN, andBKOrbitErroris raised with the library's message (fromorbfit_last_error()). The ephemeris reader checks its reads and seeks to records by absolute position.bk_orbit.pynow loads the library once and holds a lock around every call.-fvisibility=hidden. Only the ctypes entry points, theset_*functions andutc_jd_to_ttare exported, so generic names such asgaussjanddvectorno longer leak into the process. File-local helpers arestatic, and dead code (read_options,accel,write_obs,degdms,deghms,ephem_read.h) is removed.nrutil.cis converted from K&R definitions to ANSI. GCC 15 only warns on K&R definitions, but clang in C23 mode rejects them.Three numerical questions that need a domain decision rather than a code fix are in #25.
Test plan
pytest test: 36 passed (macOS, Python 3.12)-Werrorcompile with clang (macOS) and with GCC 15 (gcc:15Docker image), including-O2leaks --atExitreports 0 leaksc-checksand the Python 3.12-3.14 builds