-
Notifications
You must be signed in to change notification settings - Fork 3
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
Better star catalog handling #253
Conversation
Otherwise the starcat_dict.pop("meta") can fail
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? |
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). |
Use starcat_dict as prime not ACATable Cache only ids and mags as pure Python
@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 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. |
BTW this is passing tests except for the creating archive regression test which is expected to fail. |
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?
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of the Monday meeting there was the plan to rename the cache.
This is so there are no problems with the existing file in users' directories.
@jeanconn - the cache has been renamed. In addition I noticed a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update.
I note the unit testing is unchecked at this point. |
I re-ran testing and confirmed just the one expected test failure. |
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
|
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.h5
there 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 (
KeyError
trying to dostarcat_dict.pop("meta")
) and this changed fixed it.