Add a function to compute heliocentric angle (related to mu for limb darkening)#7979
Conversation
nabobalis
added
No Backport
This comment was marked as resolved.
This comment was marked as resolved.
github-actions
Bot
added
the
Stale
nabobalis
removed
the
Stale
| # Requires an observer | ||
| bad_skycoord = SkyCoord(0*u.arcsec, 0*u.arcsec, frame='heliographic_stonyhurst') | ||
| with pytest.raises(ConvertError, match="observer=None."): | ||
| get_mu_angle(bad_skycoord) | ||
|
|
||
| # Requires an obstime | ||
| bad_skycoord = SkyCoord(0*u.arcsec, 0*u.arcsec, frame='heliographic_stonyhurst', observer="earth") | ||
| with pytest.raises(ConvertError, match="frame needs a specified obstime"): | ||
| get_mu_angle(bad_skycoord) |
There was a problem hiding this comment.
I feel like this is testing not the function. I added it for completeness but it feels pointless.
| # Off disk with spherical screen for kicks | ||
| with SphericalScreen(hpc_coord_center.observer, radius=constants.radius): | ||
| mu_angle = get_mu_angle(hpc_coord_off_disk) | ||
| assert_quantity_allclose(mu_angle, 0.267803967191623*u.deg) |
There was a problem hiding this comment.
This I assume is gibberish but I wanted to see what it would output.
There was a problem hiding this comment.
The use of terms in this PR are confusing, at least as I understand the typical terminology:
- There is an angle (say,
$\theta$ ), sometimes called the "heliocentric angle", which is the angle between the observer LOS vector and the surface normal (or for a point not on the surface, the vector from Sun center to the point) - It's often useful, particularly for limb darkening, to transform this quantity by the cosine, which we then call "mu" ->
$\mu = cos(\theta)$
The text in the how-to guide and the function docstring mostly conform with the above, but there are multiple instances where "heliocentric angle" and "mu" are conflated. And, I don't know if there's a standard understanding of the term "mu angle", but some literature suggests that "mu angle" refers to the cosine, and is not actually an angle in degrees.
For example, the function is called get_mu_angle() and states that it returns mu (which a user may not think is the same thing as "mu angle"), but the description indicates it is actually returning the heliocentric angle, even though earlier in the docstring says that
I think this PR needs to avoid implying that "mu" is an angle, and outright avoid the term "mu angle": either call the function get_mu() and return the cosine of the heliocentric angle, or call the function get_heliocentric_angle() and explain that the user can get
Also, why are we citing the Stack Exchange discussion? It's not a scientific reference, and there are issues with its terminology, even more so than above.
|
See If I made it any better. I don't really understand any of this, so I am just winging it, if you cant tell. |
Cadair
changed the title
|
Sorry but I still don't see the point of a how-to guide, especially since it now just repeats what is the docstring of the function. |
|
I REMOVED IT SO REVIEW THE REST OF THE PR |
|
|
||
| def get_heliocentric_angle(coordinate_on_solar_disk): | ||
| r""" | ||
| The heliocentric angle is the angle between direction from a point on the surface of the body toward the observer, with respect to the local vertical of that body. |
There was a problem hiding this comment.
Should "body" not be "sun" here? This only works on the sun right?
|
I'd like Albert as lord of coord to look at this before we merge it. |
ayshih
changed the title
Co-authored-by: Albert Y. Shih <ayshih@gmail.com>
Co-authored-by: Albert Y. Shih <ayshih@gmail.com>
Cadair
left a comment
Cadair
left a comment
There was a problem hiding this comment.
The formatting on the mu equation did not work:
|
I had too many backslashes, now fixed |
4 participants
I asked Albert this question about 10 years ago and never wrote it up.
So here is this really rough first go.
TODO: