B612-Asteroid-Institute / precovery

Fast precovery of small body observations at scale
BSD 3-Clause "New" or "Revised" License
5 stars 2 forks source link

Adam 64 #50

Open ntellis opened 1 year ago

ntellis commented 1 year ago

Thanks for the detailed review @moeyensj. Will incorporate and update.

I think you're absolutely right that we will want to be able to call precovery with a method as you describe, ultimately. Probably written in such a way as to default to "nelson-force" so as not to break anything current.

One limitation that makes just swapping out the mode difficult is that the brute force method is a huge memory hog. Doing the propagation in advance and only pulling intersecting and neighboring frames helps. It's also not nearly as bad at higher-nside (pulling less observations per ephemeride), but with many orbits over 7 years, the alg as written still could demand loading some tens of gb of observations into memory. Just something to be aware of. This could be avoided by setting a max memory usage and batch processing, but that isn't written in as of now - optimization for the future.

Now, we may not be running this algorithm at huge scale repeatedly - I think mostly as a consistency check for the nelson method - so this may not be an issue.

ntellis commented 1 year ago

The brute force algorithm has now been refactored to return PrecoveryCandidates (or, a superclass with the probabilistic extras included).

However, it now breaks on per-obs MJDs - as prophesized. As written, the alg:

This breaks down because the calculated ephemerides now don't match the observation MJDs. Another change is needed: propagate from "frame midpoint" to the mjd of each observation. This implementation is tending towards nelson-force as time goes on.