exoplanet-dev / jaxoplanet

Astronomical time series analysis with JAX
https://jax.exoplanet.codes
MIT License
41 stars 12 forks source link

Rethink returned quantities from starry `light_curve` function #148

Closed dfm closed 4 months ago

dfm commented 9 months ago

Over in https://github.com/exoplanet-dev/jaxoplanet/pull/144, @shashankdholakia, @dgegen, @lgrcia, and I were discussing what would be the most useful output from the light_curve function under the starry submodule. Our proposal over there was to return a tuple with (central_light_curve, body_light_curves) where central_light_curve would have shape (num_times,) and body_light_curves would have shape (num_bodies, num_times). Then the full light curve would be the sum over bodies + the central.

There are several subtleties here. In particular, @lgrcia and I realized that it looks like multi-body systems won't be handled properly because it's probably not appropriate to just sum the central light curves, as we're currently doing. Furthermore, there are some questions about how we handle None maps, especially in the case where some of the orbiting bodies have maps, but others don't.

lgrcia commented 9 months ago

Thanks @dfm! I like the idea of having a (1 + num_bodies) shaped output, and that any None map will naturally have a light curve with zeros.

I think I know how to refactor map_light_curve to properly compute the light curve of the central being occulted by multiple bodies. I am working on a PR right now.

shashankdholakia commented 9 months ago

Imo, as far as None maps go, I would also suggest they produce 0s as the flux rather than nans, as nans can cause issues with gradients (e.g when taking the sum over all bodies). Plus a None map is just a statement that the Body has zero flux contribution.

dfm commented 9 months ago

My only -1 on zeros (vs a literal None - I definitely wouldn't recommend Nan!) is that we might end up allocating large arrays of unnecessary zeros which would be a waste of memory and compute, but I'm sure we can navigate that.

dgegen commented 9 months ago

That is what I thought as well. One way to avoid assigning large 0-arrays would be to additionally return a list of indices indicating to which of the orbiting bodies which column belongs. But while this is more memory efficient, it is likely a less straightforward result to return or work with...