-
Notifications
You must be signed in to change notification settings - Fork 0
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
Support AGASC 1.8 and fix tests and modernize #30
Conversation
I'll fix the ruff issues in a followup PR. |
I updated the description with a post-install activity to use the 1.8 distances. However, even with the legacy 1.7 distances the new code will function correctly. |
Regarding: "Change the faint magnitude limit for pairs from 10.5 mag to 10.0 mag" - this seems fine. I note technically the default faint limit for the old default FFS looks to be 10.19, but it seems unlikely we'll get stars between 10 and 10.19 and we may change the default FFS. |
Description
This PR makes a number of improvements for supporting AGASC 1.8.
NOTE: this has a post-install activity like:
Fix tests
As noted in #29, one existing test was failing when run using AGASC 1.8 with the legacy (1.7)
distances.h5
file. The root cause was that AGASC 1.8 allows ASPQ1=0 for very close stars. That test case included such a close pair, and so sometimes the star identification got the wrong one of the pair and did not match expectation.This is fixed with a new test helper function to remove close pairs from test cases. In real operations the ACA would get a "star consistent with either one of the pair and it does not matter which one is identified for the attitude solution.
Also:
miniagasc
instead of the defaultproseco_agasc
to get stars for testing.Make a new distances file and update
make_distances.py
distances_1p8.h5
. The plan is to rename the legacy file on HEAD todistances_1p7.h5
and then make a linkdistances.h5 -> distances_1p8.h5
. This is up for discussion if anyone has a better idea.make_distances.py
was moved into the package and given a couple of command line arguments.microagasc.fits
file for caching since on-the-fly processing takes only a few seconds.search_around_sky
to find pairs instead of the previous ad-hoc technique. This cleaned up a lot of code (with the slight downside that after renaming, GitHub sees this as a completely new file).Modernize
find_attitude.py
ska_helpers.logging.basic_logger
instead ofpyyaks.logger
networkx
to be installed. I don't understand why the import was semi-optional before since the downstream code will fail withoutnetworkx
.SKA
env var and get rid of defunctska_path
.Interface impacts
Testing
Unit tests
Make distances_1p8.h5
Unit tests with AGASC 1.8 and distances_1p8.h5
Note the 20 seconds for running tests vs. 57 using the legacy 1.7
distances.h5
below.Unit test with AGASC 1.8 and
$SKA/data/find_attitude/distances.h5
(1.7)Pseudo-random attitudes using AGASC 1.8 and distances_1p8.h5
Finding attitude for 100 random-but-reproducible attitudes. This ran with no failures.
Independent check of unit tests by Jean
Functional tests
No functional testing.