Allow users to define both the semi-major axis and the orbital period

[taylorbell57]

Thanks for working on this package - I am super excited to use this once it is a bit more developed/stable! I'll likely start experimenting with the package soon!

This is a reminder of the issue I raised for the starry package at https://github.com/rodluger/starry/issues/306

When fitting exoplanet time series observations, the observable features are the orbital period and the a/R*. It is frustrating when using starry to need to set only one of these values and have the other determined assuming a Keplerian orbit which necessitates that users provide the masses of the star and planet which are not relevant observable parameters. At this point, I have had to resort to solving for the planet's mass assuming a stellar mass and the fitted orbital period and semi-major axis in order to actually fit for these two parameters I care about: an annoying and wasteful step to have to manually add. It also isn't immediately clear whether it is the semi-major axis or the orbital period that gets overwritten by starry, and there are no warnings messages telling the user that one of the input values is going to be ignored if both values are provided when initializing the Secondary object. I'm not 100% sure that jaxoplanet currently works this way, but from an initial skim it seems it does, and if it is possible to make a change now before things get too far along that'd be ideal.

It would seem easy to me to allow users to specify both the orbital period and semi-major axis, and at no point calculate or make use of the planet's mass if both values are provided. I do understand that there may be users that would want jaxoplanet to just calculate the Keplerian orbital period for a fitted mass. My suggested solution would be to require any 2 of the following: semi-major axis, orbital period, object masses. If any one of those three is missing, it can be calculated using the others, and to my knowledge there's no point in calculating a missing mass if it wasn't provided.

You raised a fair point on my starry issue that it gets a bit complicated for multi-planet systems. One possibility would be to compute the system mass for each star+planet pair, set the stellar mass to the lowest of these reduced masses and set the corresponding planet mass to zero, and then set the other planet masses based on the computed maximum stellar mass.

[dfm]

Thanks for bringing this up! This is an important use case in mind, and we should make sure that we support it. In principle this is supported by the Central.from_orbital_properties, but the interface isn't quite as slick as you might want (because of the subtleties about multiplanet systems that you mention). That being said, one of the key motivations for jaxoplanet vs. exoplanet or starry is that it's much easier to swap out all the different elements, so it's "easy" (ish) to write your own orbit objects.

One option to improve the interface using the existing technology would be to write a custom orbit builder:

from jaxoplanet.orbits.keplerian import Central, System
from jaxoplanet.units import unit_registry as ureg

def build_tjb_orbit(*, period, semimajor, **kwargs):
    central = Central.from_orbital_properties(period=period, semimajor=semimajor)
    return System(central).add_body(period=period, **kwargs)

orbit = build_tjb_orbit(period=1.0 * ureg.day, semimajor=1.0 * ureg.au)
orbit.bodies[0]

Because of all of the possible concerns you brought up, I'd probably prefer to keep this explicit, with an example like this in the docs, rather than implementing it within the orbit itself. But I could certainly be convinced otherwise!

[taylorbell57]

I think your suggested idea makes sense, and I definitely understand wanting to keep things fairly explicit as to what is going on to avoid confusing behaviours. I'm not yet familiar enough with jaxoplanet to understand how that example will interface with the jaxoplanet.experimental.starry sub-modules (e.g. SurfaceMapSystem), but if a Central.from_orbital_properties call will work well with the starry sub-modules then that'll likely suffice. Out of curiousity, will the Central.from_orbital_properties call fail if the inferred mass is negative, or do you expect other functions will fail in the case of a negative central host mass?

[dfm]

Yes - this should absolutely work with the starry submodule, since the SurfaceMapSystem takes a Central object. We should add an appropriate test for this.

Out of curiousity, will the Central.from_orbital_properties call fail if the inferred mass is negative, or do you expect other functions will fail in the case of a negative central host mass?

Good question! JAX doesn't really have a good mechanism for runtime checking of values like this. The standard approach would be to manually set the value (of mass in this case, I guess) to nan if an in invalid value is encountered. I'm not sure if it would be better to do that or just allow the mass to go negative, and warn users that it's their responsibility to make sure that the parameters are always valid.