sblunt
commented
3 years ago Hey @semaphoreP, responding to your comment here about the structure of the lnlike/system functionality of the hipparcos module. I think your point that the current module structure doesn't match our current convention of separating the lnlike and system stuff is really good. I spent a while thinking about the best way to do it, and I'm still not sure. Here are some thoughts:
The original intent of separating out the lnlike function was to keep it agnostic to the details of the system we're fitting to. This becomes a bit harder with the scans, since we need more auxiliary information (barycentric position of the Earth at the epochs of interest, eg). It's quicker to calculate and store that stuff ahead of time since it doesn't change with the fit. So I see a few API options: 1. to pass all of that stuff into the lnlike function each time we call it (ew), 2. to get it from the System object within the lnlike function each time (which sort of goes against the philosophy of keeping lnlike independent of System stuff), and three to keep the current configuration. I sort of like keeping all the Hipparcos IAD stuff in one module, because I like having everything Hipparcos-related in one place. I think that will also make it easier to make a module that incorporates Tim Brandt's Hipparcos-Gaia accelerations. I totally agree that we will probably be able to abstract out some of the lnlike stuff once we have Gaia IAD, but that's currently very little code. I think it will be easy to abstract away once the time comes. We could probably also have a base class for both the Hipparcos and Gaia IAD, although for now I think the way we're using Gaia and Hipparcos data is different enough that we don't need to do that.
Ok, lots of stream of consciousness. Not sure if any of that made sense XD Let me know what you think.
semaphoreP
See discussion in PR #226 about remaining TODOs for this.