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

Add functions get_att_for_sun_pitch_yaw() and get_nsm_attitude() #38

Merged
merged 8 commits into from
Aug 12, 2024

Conversation

taldcroft
Copy link
Member

@taldcroft taldcroft commented Aug 2, 2024

Description

This adds a new convenience function get_att_for_sun_pitch_yaw() which is roughly the inverse of get_sun_pitch_yaw(), namely return an attitude with the specified sun pitch, yaw, and off_nom_roll .

It also migrates chandra_maneuver.NSM_attitude() to a function here get_nsm_attitude(), adding the option to specify the offset pitch angle.

Additional changes

time / sun_ra / sun_dec

While in the code, I noticed a logical flaw in the handling of time, sun_ra and sun_dec in a few functions. The original logic was:

    # If ``time`` is provided then that is used to compute the sun position. Otherwise
    # ``sun_ra`` and ``sun_dec`` must be provided.
    if time is not None:
        sun_ra, sun_dec = position(time, method=method)
    roll = _nominal_roll(ra, dec, sun_ra, sun_dec)

The problem is that the user could supply time=None (meaning NOW) which satisfies "providing a time", but this would leave sun_ra=None and sun_dec=None and cause a confusing exception in numba.

The more robust logic is:

    # If both ``sun_ra`` and ``sun_dec`` are provided then those are used for the Sun
    # position; otherwise ``time`` is used to compute the Sun position.
    if sun_ra is None or sun_dec is None:
        sun_ra, sun_dec = position(time, method=method)
    roll = _nominal_roll(ra, dec, sun_ra, sun_dec)

This won't ever fail and adheres to the broad idea that not providing a time implies NOW. Note: in some cases the code was already doing this but the docstring was wrong.

Docs

The docs have been converted to numpydoc format and generally improved.

Interface impacts

Testing

Unit tests

  • Mac
(ska3) ➜  ska_sun git:(get-att-from-sun-pitch-yaw) git rev-parse HEAD           
5d90ce880d6bea29326515165558ec7574da57d2
(ska3) ➜  ska_sun git:(get-att-from-sun-pitch-yaw) pytest
======================================================= test session starts ========================================================
platform darwin -- Python 3.11.8, pytest-7.4.4, pluggy-1.4.0
rootdir: /Users/aldcroft/git
configfile: pytest.ini
plugins: timeout-2.2.0, anyio-4.3.0
collected 29 items                                                                                                                 

ska_sun/tests/test_sun.py .............................                                                                      [100%]

======================================================== 29 passed in 2.11s ========================================================

Independent check of unit tests by Jean

  • Linux
ska3-jeanconn-fido> git rev-parse HEAD
5d90ce880d6bea29326515165558ec7574da57d2
ska3-jeanconn-fido> pytest -vs
================================================ test session starts ================================================
platform linux -- Python 3.11.8, pytest-7.4.4, pluggy-1.4.0 -- /proj/sot/ska3/flight/bin/python3.11
cachedir: .pytest_cache
rootdir: /proj/sot/ska/jeanproj/git
configfile: pytest.ini
plugins: anyio-4.3.0, timeout-2.2.0
collected 29 items                                                                                                  

ska_sun/tests/test_sun.py::test_allowed_rolldev_with_roll_table PASSED
ska_sun/tests/test_sun.py::test_allowed_rolldev_vector_without_roll_table PASSED
ska_sun/tests/test_sun.py::test_duplicate_pitch_rolldev PASSED
ska_sun/tests/test_sun.py::test_position PASSED
ska_sun/tests/test_sun.py::test_position_diff_methods PASSED
ska_sun/tests/test_sun.py::test_nominal_roll PASSED
ska_sun/tests/test_sun.py::test_nominal_roll_range PASSED
ska_sun/tests/test_sun.py::test_off_nominal_roll_and_pitch PASSED
ska_sun/tests/test_sun.py::test_apply_get_sun_pitch_yaw[spacecraft] PASSED
ska_sun/tests/test_sun.py::test_apply_get_sun_pitch_yaw[ORviewer] PASSED
ska_sun/tests/test_sun.py::test_apply_sun_pitch_yaw[spacecraft] PASSED
ska_sun/tests/test_sun.py::test_apply_sun_pitch_yaw[ORviewer] PASSED
ska_sun/tests/test_sun.py::test_apply_sun_pitch_yaw_with_grid PASSED
ska_sun/tests/test_sun.py::test_get_sun_pitch_yaw PASSED
ska_sun/tests/test_sun.py::test_apply_get_sun_pitch_yaw_spacecraft_sign PASSED
ska_sun/tests/test_sun.py::test_roll_table_meta PASSED
ska_sun/tests/test_sun.py::test_roll_table_pitch_increasing PASSED
ska_sun/tests/test_sun.py::test_array_input_and_different_formats[fast] PASSED
ska_sun/tests/test_sun.py::test_array_input_and_different_formats[accurate] PASSED
ska_sun/tests/test_sun.py::test_nsm_attitude_random_atts[90] PASSED
ska_sun/tests/test_sun.py::test_nsm_attitude_random_atts[130] PASSED
ska_sun/tests/test_sun.py::test_nsm_attitude_random_atts[160] PASSED
ska_sun/tests/test_sun.py::test_nsm_attitude_corner_case PASSED
ska_sun/tests/test_sun.py::test_get_att_for_sun_pitch_yaw[spacecraft-True] PASSED
ska_sun/tests/test_sun.py::test_get_att_for_sun_pitch_yaw[spacecraft-False] PASSED
ska_sun/tests/test_sun.py::test_get_att_for_sun_pitch_yaw[ORviewer-True] PASSED
ska_sun/tests/test_sun.py::test_get_att_for_sun_pitch_yaw[ORviewer-False] PASSED
ska_sun/tests/test_sun.py::test_get_att_for_sun_pitch_yaw[None-True] PASSED
ska_sun/tests/test_sun.py::test_get_att_for_sun_pitch_yaw[None-False] PASSED

================================================ 29 passed in 16.80s

Functional tests

No functional testing.

@taldcroft taldcroft changed the title WIP Add function get_att_for_sun_pitch_yaw() Add functions get_att_for_sun_pitch_yaw() and get_nsm_attitude() Aug 10, 2024
@jeanconn
Copy link
Contributor

@taldcroft do you have an example of the code path that triggered the previous numba exception?

@taldcroft
Copy link
Member Author

taldcroft commented Aug 12, 2024

@taldcroft do you have an example of the code path that triggered the previous numba exception?

In current flight:

>>> ska_sun.nominal_roll(10, 20)
...
TypingError: Failed in nopython mode pipeline (step: nopython frontend)
Failed in nopython mode pipeline (step: nopython frontend)
No implementation of function Function(<ufunc 'radians'>) found for signature:

This PR:

>>> ska_sun.nominal_roll(10, 20)
122.98232642214438

@taldcroft taldcroft merged commit 646785e into master Aug 12, 2024
2 checks passed
@taldcroft taldcroft deleted the get-att-from-sun-pitch-yaw branch August 12, 2024 16:35
@javierggt javierggt mentioned this pull request Nov 7, 2024
@javierggt javierggt mentioned this pull request Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants