Allow bonus faint guide stars for dyn bgd enabled #372
Conversation
|
@jeanconn - ready for final review |
There was a problem hiding this comment.
I am still running one of the proseco notebooks, just to check. I ran all tests and looked at the code and things seem reasonable. The only thing was that the test only verifies whether the functions do not fail and I was wondering whether there is some consistency test one could do.
One test one could do is to verify that with default arguments it returns the catalogs as in previous versions.
proseco/tests/test_catalog.py
Outdated
| acar_leg = aca_leg.get_review_table() | ||
| acar_dyn = aca_dyn.get_review_table() | ||
| print(np.isclose(acar_leg.guide_count, 4.46, rtol=0, atol=0.1)) | ||
| print(np.isclose(acar_dyn.guide_count, 4.92, rtol=0, atol=0.1)) |
There was a problem hiding this comment.
This test only tests that there are no exceptions.
Is it possible to check that it did something reasonable?
There was a problem hiding this comment.
Good point. I just thought of a way to functionally test the proseco bit here where it will get a different number of stars, showing that the star selection is improved.
There was a problem hiding this comment.
BTW, I had not originally noticed the print statements and this was the genesis of your issue. I had meant those to be asserts. But you stumbled on a real coverage problem which was that the catalogs being picked by proseco for leg and dyn were actually the same.
There was a problem hiding this comment.
See 9ad97b8. I should not have let myself fall into the trap of using an obsid, instead do a synthetic stars test that exactly tests the code. Bad Tom, good catch.
|
@javierggt - can you Approve the PR? |
|
yes, I was going over the output of the notebook that was running. My plots look slightly different, but I am guessing that's because I have not updated ska data in a while. |
Description
Per discussion at SS&AWG on 2022-Apr-27 (see the presentation there for full details), this adds two new parameters to the base
ACACatalogTableand theget_aca_catalogfunction to allow specifying the parameters of the bonus stars.Requires:
Interface impacts
Adds
dyn_bgd_n_faintanddyn_bgd_dt_ccdarguments toget_aca_catalog. The default settings (in particulardyn_bgd_n_faint = 0) have behavior that is identical to current flight star selection.Testing
Unit tests using above-mentioned branches
Independent check of unit tests by Javier
Functional tests