Add cached Sun/Moon calculations in Observer class

[jak574]

Add cached Sun/Moon calculations in Observer class

Introduction

I've been investigating the use of astroplan for the next major release of the API for NASA's Swift mission (https://www.swift.psu.edu/too_api/). This would involve some perhaps unusually intensive usage of astroplan, so I've been looking at small optimizations that can shave seconds off some API calls.

Reason for pull

astroplan makes use of the the astropy get_sun and get_body to calculate the position of the Moon and Sun. Unfortunately these calls are computationally expensive, which can lead to inefficiencies when calculating the positions of the Sun and Moon from scratch each time we need them. As the Sun/Moon positions are only unique for a given Observer location and set of times, it would be more efficient to cache them, rather than recalculating them every time we need them.

Solution

This pull request adds a get_body method to the Observer class, which calculates the position of any Solar System body supported by the astropy get_body, for the current Observer.location, at a given time. This method caches the results, using the same scheme as alt/az caching (reusing the _make_cache_key function , which I had to move to observer.py to avoid a circular import).

I then replaced all calls to get_sun and get_body in Observer and in constraints.py where the Observer class is used, with this method.

This ensures that for a given Observer and time, the positions of the Sun and Moon are only ever calculated once. In my own code, this results in a 4x speedup when calculating multiple target visibilities, after an initial calculation.

Possible issues

Replacement of get_sun calls

I replaced get_sun with Observer.get_body('sun', ...) calls, which means that get_sun calls that previously did not pass the location parameter, now do. I do not believe this will cause any problems, and should be both negligibly different and "more correct". However, there is a possibility I misunderstood the reason for not passing location to get_sun in some cases. The solution to this would be to create a second get_sun method that mimics exactly the previous use, but with caching.

To investigate this, I used the Observer.altaz method to calculate the Alt/Az of the Sun, for the example given in the inline code comment. The use of get_body with location set instead of a plain get_sun does result in different values, but it differs by a negligible amount (angular separation of 0.005 arc-seconds between the two values).

I will note that my testing/development of this pull request has been done exclusively with astropy 6.0.

The get_body method name

Stylistic choice, but it might be better to use a different method name than the astropy get_body for this, to avoid confusion? I wavered between get_body and _get_body.

[jak574]

OK, I see a test failed that didn't happen in my checks. I can look into this.

[codecov]

Codecov Report

❌ Patch coverage is 80.55556% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.88%. Comparing base (b20a095) to head (dc226b9).

Files with missing lines	Patch %	Lines
astroplan/observer.py	77.41%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #578      +/-   ##
==========================================
- Coverage   72.90%   72.88%   -0.02%     
==========================================
  Files          14       14              
  Lines        1849     1859      +10     
==========================================
+ Hits         1348     1355       +7     
- Misses        501      504       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.