Skip to content

Add STK ephem source option to get_planet_chandra#196

Merged
jeanconn merged 8 commits intomasterfrom
more-planet
Oct 31, 2025
Merged

Add STK ephem source option to get_planet_chandra#196
jeanconn merged 8 commits intomasterfrom
more-planet

Conversation

@jeanconn
Copy link
Contributor

@jeanconn jeanconn commented Oct 16, 2025

Description

Add STK as an ephemeris source option to get_planet_chandra.

This will allow some code reuse instead of a bunch of different copies of very similar code and provide a way to do ACA planet checks on all platforms, including FOT laptops with no cheta archive.

Interface impacts

Testing

Unit tests

  • Mac
(ska3-latest) flame:chandra_aca jean$ git rev-parse HEAD
96d8b2fa5beeb6c405d0aa438e63c92b23a02e58
(ska3-latest) flame:chandra_aca jean$ pytest
============================================================= test session starts ==============================================================
platform darwin -- Python 3.12.8, pytest-8.3.4, pluggy-1.5.0
rootdir: /Users/jean/git
configfile: pytest.ini
plugins: socket-0.7.0, anyio-4.7.0, timeout-2.3.1
collected 256 items                                                                                                                            

chandra_aca/tests/test_aca_image.py .....................                                                                                [  8%]
chandra_aca/tests/test_all.py ........................                                                                                   [ 17%]
chandra_aca/tests/test_attitude.py .............................................................                                         [ 41%]
chandra_aca/tests/test_dark_model.py ............                                                                                        [ 46%]
chandra_aca/tests/test_dark_subtract.py .........                                                                                        [ 49%]
chandra_aca/tests/test_drift.py ..............................................                                                           [ 67%]
chandra_aca/tests/test_maude_decom.py .........................                                                                          [ 77%]
chandra_aca/tests/test_planets.py .................                                                                                      [ 83%]
chandra_aca/tests/test_psf.py ...                                                                                                        [ 85%]
chandra_aca/tests/test_residuals.py ss...                                                                                                [ 87%]
chandra_aca/tests/test_star_probs.py .................................                                                                   [100%]

=============================================================== warnings summary ===============================================================
chandra_aca/chandra_aca/tests/test_planets.py::test_planet_positions
chandra_aca/chandra_aca/tests/test_planets.py::test_planet_positions
chandra_aca/chandra_aca/tests/test_planets.py::test_venus_position1[stk]
chandra_aca/chandra_aca/tests/test_planets.py::test_venus_position1[stk]
chandra_aca/chandra_aca/tests/test_planets.py::test_venus_position2[stk]
chandra_aca/chandra_aca/tests/test_planets.py::test_venus_position2[stk]
chandra_aca/chandra_aca/tests/test_residuals.py::test_obc
  /Users/jean/miniforge3/envs/ska3-latest/lib/python3.12/site-packages/bs4/builder/_lxml.py:124: DeprecationWarning: The 'strip_cdata' option of HTMLParser() has never done anything and will eventually be removed.
    parser = parser(

chandra_aca/chandra_aca/tests/test_residuals.py::test_obc
  /Users/jean/miniforge3/envs/ska3-latest/lib/python3.12/site-packages/django/utils/encoding.py:266: DeprecationWarning: 'locale.getdefaultlocale' is deprecated and slated for removal in Python 3.15. Use setlocale(), getencoding() and getlocale() instead.
    encoding = locale.getdefaultlocale()[1] or 'ascii'

chandra_aca/chandra_aca/tests/test_residuals.py::test_obc
  /Users/jean/miniforge3/envs/ska3-latest/lib/python3.12/site-packages/django/http/request.py:1: DeprecationWarning: 'cgi' is deprecated and slated for removal in Python 3.13
    import cgi

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
============================================ 254 passed, 2 skipped, 9 warnings in 66.59s (0:01:06)


Independent check of unit tests by [REVIEWER NAME]

  • [PLATFORM]:

Functional tests

See the updated analysis notebook.

@jeanconn jeanconn requested a review from Copilot October 16, 2025 17:36

This comment was marked as resolved.

@taldcroft
Copy link
Member

@jeanconn - I just pushed a commit that takes advantage of the cheta computed MSID interface to reduce/simplify the code. It is passing tests, but if there is an issue we can revert the commit.

@taldcroft
Copy link
Member

taldcroft commented Oct 27, 2025

The choices could probably be ephem0, ephem1 or stk.

@taldcroft taldcroft changed the title Add stk ephem source option to get_planet_chandra Add STK ephem source option to get_planet_chandra Oct 30, 2025
@taldcroft
Copy link
Member

I updated the notebook so this is hopefully ready now.

@jeanconn
Copy link
Contributor Author

jeanconn commented Oct 31, 2025

It is still my PR so I can't approve it. I do note in passing that I didn't use the computed MSID because the fetch import was just a lot heavier, and I was looking to have more of this code used by proseco instead of its bespoke version for Jupiter. But we can always look at this again if that ends up being a problem.

@taldcroft
Copy link
Member

I do note in passing that I didn't use the computed MSID because the fetch import was just a lot heavier

On my laptop importing cheta.fetch seems to be a one-time penalty of around 120 ms relative to cheta.comps. The fetch import could probably be made lazier if need be. But in general none of this plays out unless there is a planet within 2 degrees of the target, right? So basically it should be rare?

It looks like chandra_aca.planets is pretty slow.

(ska3) ➜  ~ ipython
In [1]: %time import chandra_aca.planets
CPU times: user 208 ms, sys: 40.7 ms, total: 248 ms
Wall time: 253 ms

In [2]: %time import cheta.comps
CPU times: user 69 ms, sys: 8.48 ms, total: 77.5 ms
Wall time: 78.9 ms

(ska3) ➜  ~ ipython
In [1]: %time import chandra_aca.planets
CPU times: user 206 ms, sys: 40 ms, total: 246 ms
Wall time: 249 ms

In [2]: %time import cheta.fetch
CPU times: user 162 ms, sys: 27.1 ms, total: 190 ms
Wall time: 193 ms

Copy link
Member

@taldcroft taldcroft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Contingent on a "verbal" approval of the new code and testing from @jeanconn.

@jeanconn
Copy link
Contributor Author

Yep, I'm good with it.

For fetch - I am actually a little confused about how the Windows interface with cheta remote access does or doesn't do anything for these stk computed msids? It isn't a case I worked with much when I skipped right ahead to doing the ephem_stk version for proseco. So we can talk about that more. I agree that the "heavier" fetch import isn't that heavy and shouldn't even need to happen except for targets that already have a hit on a planet from the no-chandra-ephem-needed first round of checks.

@jeanconn jeanconn merged commit 856d455 into master Oct 31, 2025
2 checks passed
@jeanconn jeanconn deleted the more-planet branch October 31, 2025 18:39
@taldcroft
Copy link
Member

That was an excellent point about remote access and you were a bit quick to merge. It turns out that on Windows (or if remote access is enabled) then importing fetch is fine, but doing fetch.Msid('orbitephem_skt_x', ...) triggers the remote access machinery even though it is not actually needed.

This appears to be successfully fixed by this little patch in cheta.fetch to make the MSID.content attribute be instead a property. I would probably write it that way today anyway.

diff --git a/cheta/fetch.py b/cheta/fetch.py
index fde4c1a..807150e 100644
--- a/cheta/fetch.py
+++ b/cheta/fetch.py
@@ -511,7 +511,6 @@ class MSID(object):
         self.datestart = DateTime(self.tstart).date
         self.datestop = DateTime(self.tstop).date
         self.data_source = {}
-        self.content = content.get(self.MSID)
 
         if self.datestart < DATE2000_LO and self.datestop > DATE2000_HI:
             intervals = [(self.datestart, DATE2000_HI), (DATE2000_HI, self.datestop)]
@@ -529,6 +528,10 @@ class MSID(object):
         if "CHETA_FETCH_DATA_GAP" in os.environ:
             create_msid_data_gap(self, os.environ["CHETA_FETCH_DATA_GAP"])
 
+    @property
+    def content(self):
+        return content.get(self.MSID)
+
     def __len__(self):
         return len(self.vals)

Then:

>>> import os
>>> os.environ['SKA_ACCESS_REMOTELY'] = 'True'
>>> from cheta import fetch, remote_access
>>> fetch.add_logging_handler()
>>> assert remote_access.access_remotely is True

In [2]: dat = fetch.Msid("orbitephem_stk_x", "2018:001", "2018:010")
_get_data: Getting data for orbitephem_stk_x between 2018:001:00:00:00.000 to 2018:010:00:00:00.000
_get_comp_data: Getting computed values for orbitephem_stk_x
get_ephem_stk_paths: Checking local directory /Users/aldcroft/Documents/MATLAB/FOT_Tools/Ephemeris for STK files
get_ephem_stk_paths: Checking OCCweb directory FOT/mission_planning/Backstop/Ephemeris
get_ephem_stk_paths: Checking OCCweb directory FOT/mission_planning/Backstop/Ephemeris/ArchiveMCC
read_stk_file: Reading cached STK file FOT/mission_planning/Backstop/Ephemeris/ArchiveMCC/Chandra_17337_18154.stk from /Users/aldcroft/.cheta/cache/Chandra_17337_18154.stk.npz
read_stk_file: Reading cached STK file FOT/mission_planning/Backstop/Ephemeris/ArchiveMCC/Chandra_18002_18183.stk from /Users/aldcroft/.cheta/cache/Chandra_18002_18183.stk.npz

Hmm...

@jeanconn
Copy link
Contributor Author

I figured this was adding new functionality - so merging to master wasn't really breaking anything, and forward and fix would be fine - either with a fix in cheta or backing out your computed stk use here. Since my preference was to find a fix in cheta it seems like this on track!

@jeanconn
Copy link
Contributor Author

Oh and if we really care about stk speed and web queries I think we still probably could add a "just use what you've got in the cache" option. But I do think it is reasonable now that it always hits occweb to check for the latest names for paths.

@taldcroft
Copy link
Member

I also want to put a retry in the bit that checks the directory, I got hit with a hang on that bit once while testing.

@taldcroft
Copy link
Member

So there is now sot/cheta#282. Despite that fix, I'm almost leaning to reverting back to the original direct use of cheta.comps.

@jeanconn
Copy link
Contributor Author

jeanconn commented Nov 1, 2025

I think the cheta fix is great and very helpful so I see this as a useful conversation and progression. And fine either way by me if this chandra_aca code uses computed msids or ephem_stk directly in the end.

@javierggt javierggt mentioned this pull request Nov 18, 2025
@javierggt javierggt mentioned this pull request Jan 20, 2026
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