Closed taldcroft closed 1 year ago
jeanconn
commented
1 year ago I was really just asking... what is the utility of the agasc id and what is the right agasc id when the star position in the commanded catalog was wrong enough that this fix is needed?
taldcroft
commented
1 year ago what is the utility of the agasc id and what is the right agasc id when the star position in the commanded catalog was wrong enough that this fix is needed?
The star position in the commanded catalog was off by less than 3 arcsec, which means it was good enough for Chandra to find that star (both for guide and acq) and use it in attitude control. So the AGASC ID is the unique identifier to specify the star that the ACA was nominally tracking.
So the only problem was that I would like to have a tight 1.5 arcsec tolerance for matching between the commanded catalog position and the star positions (to get the AGASC ID). With this fix that is possible and all the identifications are correct (they match the original IDs found in the guide summary or proseco pickle).
taldcroft
commented
1 year ago @jeanconn @javierggt - I made good progress on this and substantially reworked a lot of the starcats-related code. I have successfully fetched every star catalog in the archive in one command starcats = get_starcats(show_progress=True). This takes a few minutes but I added an option to show the progress along the way. The new cache file is a lot smaller.
Once you have populated your cache (one-time 4 minute exercise) then getting catalogs as a table is fast, about 8 seconds for the full mission.
Please prioritize and take some time to test this out so we can get it merged for the next release.
taldcroft
commented
1 year ago BTW this is passing tests except for the creating archive regression test which is expected to fail.
jeanconn
commented
1 year ago I've had varied results. (I embraced my common user mistake of supplying the obsid as an argument to see what would happen). I got a bunch of errors and then just deleted my ~/.kadi area and tried again. Maybe error handling should also clear the cache? Or maybe my failures were related to issues in the session and not the cache?
In [27]: cats = commands.get_starcats(45814)
HASH: Out of overflow pages. Increase page size
---------------------------------------------------------------------------
error Traceback (most recent call last)
<ipython-input-27-7c975816350d> in <module>
----> 1 cats = commands.get_starcats(45814)
~/git/kadi/kadi/commands/observations.py in get_starcats(start, stop, obsid, scenario, cmds, as_dict, starcat_date, show_progress)
450 set_fid_ids(starcat_dict)
451 set_star_ids(starcat_dict)
--> 452 starcats_db[db_key] = (starcat_dict["id"], starcat_dict["mag"])
453 else:
454 starcat_dict["id"] = starcat_ids
~/miniconda3/envs/ska3/lib/python3.8/shelve.py in __setitem__(self, key, value)
123 p = Pickler(f, self._protocol)
124 p.dump(value)
--> 125 self.dict[key.encode(self.keyencoding)] = f.getvalue()
126
127 def __delitem__(self, key):
error: cannot add item to database
In [28]: cats = commands.get_starcats()
---------------------------------------------------------------------------
ValueError Traceback (most recent call last)
<ipython-input-28-398d0664e645> in <module>
----> 1 cats = commands.get_starcats()
~/git/kadi/kadi/commands/observations.py in get_starcats(start, stop, obsid, scenario, cmds, as_dict, starcat_date, show_progress)
422 )
423 if db_key in starcats_db:
--> 424 starcat_ids, starcat_mags = starcats_db[db_key]
425 else:
426 starcat_ids = None
ValueError: too many values to unpack (expected 2)
In [29]: cats = commands.get_starcats(start="1999:001")
---------------------------------------------------------------------------
ValueError Traceback (most recent call last)
<ipython-input-29-d66f5886d19d> in <module>
----> 1 cats = commands.get_starcats(start="1999:001")
~/git/kadi/kadi/commands/observations.py in get_starcats(start, stop, obsid, scenario, cmds, as_dict, starcat_date, show_progress)
422 )
423 if db_key in starcats_db:
--> 424 starcat_ids, starcat_mags = starcats_db[db_key]
425 else:
426 starcat_ids = None
ValueError: too many values to unpack (expected 2)
In [30]:
Do you really want to exit ([y]/n)? y
(ska3) spark:kadi jean$ rm -r ~/.kadi/
(ska3) spark:kadi jean$ ipython
Python 3.8.12 (default, Oct 12 2021, 06:23:56)
Type 'copyright', 'credits' or 'license' for more information
IPython 7.29.0 -- An enhanced Interactive Python. Type '?' for help.
In [1]: from kadi import commands
In [2]: commands.conf.commands_version = "2"
In [3]: cats = commands.get_starcats(start="1999:001")
2022-09-16 13:16:43,351 get_cmds_obs_final: WARNING: no starcat for obsid 62624 at 2022:232:20:48:44.443 even though npnt_enab is True
(that worked fine, except for that warning)@jeanconn - the cache has been renamed. In addition I noticed a warnings.warn that doesn't need to alarm normal users but will show up in cron processing.
jeanconn
commented
1 year ago I note the unit testing is unchecked at this point.
taldcroft
commented
1 year ago I re-ran testing and confirmed just the one expected test failure.
taldcroft
commented
1 year ago The archive regression diff which causes the failure is below. This is the diff against the new archive from #248 (which itself created diffs from the current flight archive) is the removal of starcat_date=2021:296:04:59:06.931,. This is expected as the new processing does not set a starcat_date if there is no associated star catalog.
(ska3-flight-2022.9rc1) ➜ kadi git:(starcats-better) ✗ diff cmds_flight.txt cmds_local.txt
746c746
< 2021:297:02:05:11.042 | LOAD_EVENT | OBS | CMD_EVT | obsid=0, simpos=-99616, obs_stop=2021:298:02:00:00.000, manvr_start=2021:297:01:41:11.250, targ_att=(0.70546907, 0.32988307, 0.53440901, 0.32847766), npnt_enab=True, obs_start=2021:297:02:05:11.042, prev_att=(0.2854059718219833, 0.5642529438105833, 0.6343832026867755, 0.4446571701938166), starcat_date=2021:296:04:59:06.931, scs=0
---
> 2021:297:02:05:11.042 | LOAD_EVENT | OBS | CMD_EVT | obsid=0, simpos=-99616, obs_stop=2021:298:02:00:00.000, manvr_start=2021:297:01:41:11.250, targ_att=(0.70546907, 0.32988307, 0.53440901, 0.32847766), npnt_enab=True, obs_start=2021:297:02:05:11.042, prev_att=(0.2854059718219833, 0.5642529438105833, 0.6343832026867755, 0.4446571701938166), scs=0
Description
This makes a few improvements to the way star catalogs are handled in kadi commands:
Reset the star catalog after each observation
Previously the last commanded star catalog was kept as "continuity" for subsequent observation. This is actually how the spacecraft works which was the original motivation, but in practice the way loads are built there is always a unique new catalog associated with the end of each maneuver (when auto-transition to NPNT is enabled).
What was happening in kadi was that a stale catalog was being assigned to maneuvers during anomaly recovery. The new behavior is that such an observation gets no catalog but logs a warning that no catalog was found when one was expected.
Copy the cached starcat dict in case a key gets re-used
With the current flight version of
cmds2.h5there were cases of multiple observations sharing the same star catalog. This no longer happens, but just the same this commit fixes a problem that came up getting star catalogs when caching to file was disabled.Correct for MATLAB proper motion but in star identification
The comment in e4b5bda describes this in detail.
Interface impacts
None as far as I know. Undercover observations are still the same.
Testing
Unit tests
Independent check of unit tests by Jean
Functional tests
Correct for MATLAB proper motion but in star identification
With the debug code in this commit uncommented and the MATLAB PM correction disabled I got the output below. This shows many cases of high PM stars (mostly at higher Dec values) which have large offsets between the predicted yag/zag and the commanded catalog yag/zag.
With the proper motion correction enabled then the same command results in no output, meaning no stars had a residual of over 0.5 arcsec in the identification process.
Reset star catalog after each observation
Here is a test script asking for the observation and star catalog for the ground-commanded maneuver after the 2022:223 BSH:
New version (correct behavior, no catalog since the ground FOT request for the star catalog is not in the archive)
Flight (previously catalog erroneously assigned)
Copy the cached starcat dict in case a key gets re-used
I don't remember how to reproduce this, but at some point it came up (
KeyErrortrying to dostarcat_dict.pop("meta")) and this changed fixed it.