Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change a to a_over_Rsun in KeplerianOrbit #299

Open
ideasrule opened this issue Jul 19, 2023 · 2 comments
Open

Change a to a_over_Rsun in KeplerianOrbit #299

ideasrule opened this issue Jul 19, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@ideasrule
Copy link

Is your feature request related to a problem? Please describe.
Multiple people have gotten confused by the "a" in KeplerianOrbit. Among astronomers studying transiting exoplanets, "a" is always given either in stellar radii or AU, but never in solar radii. It is easy to mistakenly give a/R_* instead of a/R_sun, especially for Sun-like stars where the two numbers are similar.

This confusion has made it into the literature at least twice. Due to a confusion between a/R_* and a/R_sun, the semimajor axes of three planets (the three around TOI 2076) were misreported by 22%. I then published a paper using the incorrect semimajor axes, causing my flux estimates to be off by 40%.

Describe the solution you'd like
Change "a" to a_over_Rsun, a_star to a_star_over_Rsun, and a_planet to a_planet_over_Rsun. This has the added benefit that both developers and users can easily see where "a" is being used with Ctrl + F.

Additional context
I'm happy to do the change myself and make a pull request.

@ideasrule ideasrule added the enhancement New feature or request label Jul 19, 2023
@dfm
Copy link
Member

dfm commented Jul 19, 2023

Interesting proposal! I'm not so keen to change these variable names, since the units are consistent across all parameters/arguments of the KeplerianOrbit, so we'd need to change all of the names. I believe that the docs are reasonably unambiguous, but it's always a good idea to specifically call out issues that have arisen. Perhaps, a reasonable approach would be to add an admonition box to the docstring, the "orbital conventions" section, and perhaps call this out a few times throughout the tutorials?

@ideasrule
Copy link
Author

I agree that the docs are unambiguous, so the problem probably arises when people don't read the docs carefully, which I've certainly been guilty of. I've invited the first author of the TOI 2076 paper to comment here to get his insights.

Why do all the parameters/arguments of the KeplerOrbit need to change? As far as I can tell, "a" is the only one that has high potential for confusion, because the other reasonable units for the other parameters are different by orders of magnitude (and so mistakes are very noticeable). If you mean that it's inconsistent for "a" to specify its units while the other parameters don't, I'd agree, but I think the practical benefits are compelling enough to justify the inconsistency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants