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

Improve HG, HG12, etc. documentation #386

Open
mkelley opened this issue Aug 23, 2023 · 4 comments
Open

Improve HG, HG12, etc. documentation #386

mkelley opened this issue Aug 23, 2023 · 4 comments
Assignees
Milestone

Comments

@mkelley
Copy link
Member

mkelley commented Aug 23, 2023

The documentation, particularly https://sbpy.readthedocs.io/en/stable/sbpy/photometry.html, but also the class docstrings, seems to be missing two key concepts regarding the disk integrated phase function models:

  • That the models are based on the astropy modeling framework.
    • The users should be directed to the astropy modeling pages for more help on using these models.
    • One especially important point that should be documented up front: calling the object with a phase angle produces the phase corrected absolute magnitude.
  • That the models do not include rh and delta corrections, i.e., they do not produce apparent magnitudes.

These two points could be illustrated with an example that plots apparent magnitude versus time. Perhaps a simplified version of the following:

import numpy as np
import matplotlib.pyplot as plt
from astropy.time import Time
import astropy.units as u
from astropy.visualization import time_support
from sbpy.photometry import HG1G2
from sbpy.data import Ephem

time_support()  # allow plotting Time objects with matplotlib

epochs={
    "start": Time("2022-09-26 23:14"),
    "stop": Time("2023-03-01"),
    "step": 1 * u.day,
}
eph = Ephem.from_horizons("didymos", epochs=epochs, location="500")
m_HG1G2 = HG1G2(H=18.11, G1=0.84, G2=0.05)

m = m_HG1G2(eph["phase"]) + 5 * np.log10(eph["rh"] / u.au * eph["delta"] / u.au)

plt.clf()
ax = plt.gca()
plt.plot(eph["date"], m)
ax.invert_yaxis()
@mkelley mkelley added the feature request request for new functionality label Aug 23, 2023
@mkelley
Copy link
Member Author

mkelley commented Aug 23, 2023

Thanks to @talister for the initial feedback. No good deed goes unpunished, so may ask you to review the updates 😸

@mkelley mkelley added documentation and removed feature request request for new functionality labels Aug 23, 2023
@mkelley
Copy link
Member Author

mkelley commented Aug 23, 2023

The users should be directed to the astropy modeling pages for more help on using these models.

This is done near the fitting examples, but I think something at the top-level description would be a big help.

@jianyangli
Copy link
Contributor

I believe the distance correction is built-in. If one passes a DataClass object that contains rh and/or delta fields as input to the object, then the appropriate correction will be applied behind the scene. I will double-check and make this clear in the docstrings.

@mkelley
Copy link
Member Author

mkelley commented Aug 24, 2023

Thanks for looking into it!

@mkelley mkelley assigned hhsieh00 and unassigned jianyangli Aug 31, 2023
@mkelley mkelley added this to the v0.5 milestone Jan 18, 2024
@mkelley mkelley modified the milestones: v0.5, v0.6 Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants