Skip to content
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

Handle omitted star missing from miniagasc 1p8 in V&V #305

Merged
merged 2 commits into from
Oct 24, 2024

Conversation

jeanconn
Copy link
Contributor

@jeanconn jeanconn commented Oct 6, 2024

Description

Handle omitted star missing from miniagasc 1p8 in V&V.

This code tweak is for a very specific case - a star was used as a guide star in an historical observation, and also excluded from processing in the final aspect solution (either for centroid quality or position error or whatever). For these OMITTED stars, the V&V code uses the agasc to make a "mock" entry for gsprops to calculate residuals and such for whatever data is available - if that star isn't in the miniagasc 1p8 the "mock" gsprops entry code was failing. We could just handle the exception, but I figured it made just as much sense to use the full agasc for this processing code anyway. I originally set this PR to use the 1.7 miniagasc, but I think that's a little too complicated and confusing.

It is a different question about if the residuals for the whole historical V&V database should be calculated against the positions of agasc 1.8.

Noticed this as a little bug in obsid 5778 which has an OMITTED guide star not found in miniagasc 1.8.

Interface impacts

V&V processing completes on historical observation with OMITTED guide star missing from miniagasc 1.8

Testing

I tested against a ska3 masters env.

Unit tests

  • Linux
jeanconn-fido> git rev-parse HEAD
d9e51682ddf8d1214164dd302e1f82deb25074fc
jeanconn-fido> pytest
========================================================= test session starts =========================================================
platform linux -- Python 3.11.8, pytest-7.4.4, pluggy-1.4.0
rootdir: /proj/sot/ska/jeanproj/git
configfile: pytest.ini
plugins: timeout-2.2.0, anyio-4.3.0
collected 108 items                                                                                                                   

mica/archive/tests/test_aca_dark_cal.py ..................                                                                      [ 16%]
mica/archive/tests/test_aca_hdr3.py .                                                                                           [ 17%]
mica/archive/tests/test_aca_l0.py ...                                                                                           [ 20%]
mica/archive/tests/test_asp_l1.py .......                                                                                       [ 26%]
mica/archive/tests/test_cda.py ..............................................                                                   [ 69%]
mica/archive/tests/test_obspar.py .                                                                                             [ 70%]
mica/report/tests/test_write_report.py .                                                                                        [ 71%]
mica/starcheck/tests/test_catalog_fetches.py ...............                                                                    [ 85%]
mica/stats/tests/test_acq_stats.py ...                                                                                          [ 87%]
mica/stats/tests/test_guide_stats.py ....                                                                                       [ 91%]
mica/vv/tests/test_vv.py .........                                                                                              [100%]

=================================================== 108 passed in 516.38s (0:08:36)

Independent check of unit tests by [REVIEWER NAME]

  • [PLATFORM]:

Functional tests

For obsid 5778 old code:

In [3]: import mica.vv.process
In [4]: mica.vv.process.process(5778, 4)
...
    755     )
    757 if id_rows is None or len(id_rows) == 0:
--> 758     raise IdNotFound()
    760 t = Table(id_rows)
    761 add_pmcorr_columns(t, date)

IdNotFound: 

New code:

Generating V&V for obsid 5778                                                                                                          
Processing aspect interval pcadf234286822N004                                                                                          
Processing GOOD       star in slot 3                                                                                                   
Processing GOOD       star in slot 5                                                                                                   
Processing GOOD       star in slot 6                                                                                                   
Processing GOOD       star in slot 7                                                                                                   
Processing OMITTED star in slot 4                                                                                                      
Processing fid ACIS-I-1   in slot 0                                                                                                    
Processing fid ACIS-I-5   in slot 1                                                                                                    
Processing fid ACIS-I-6   in slot 2
...
obsid 5778 is in table
updating obsid 5778 rev 4 in place

@jeanconn jeanconn marked this pull request as ready for review October 6, 2024 15:17
@jeanconn jeanconn changed the title Handle omitted star missing from agasc 1p8 in V&V Handle omitted star missing from miniagasc 1p8 in V&V Oct 6, 2024
Copy link
Contributor

@javierggt javierggt left a comment

Choose a reason for hiding this comment

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

The change seems simple enough to me, so I am ok.

I can't make the functional test, because /data/aca/archive/tempvv is owned by jeanconn.

Base automatically changed from stats-bug to master October 24, 2024 18:48
@jeanconn
Copy link
Contributor Author

Right, to do the functional test you'd probably need to set MICA_ARCHIVE to not-flight. I think I can move a bunch of this to not-me as we go through and separate out the updating and trending etc.

@jeanconn jeanconn merged commit af13909 into master Oct 24, 2024
2 checks passed
@jeanconn jeanconn deleted the omitted-missing-star branch October 24, 2024 18:50
@javierggt javierggt mentioned this pull request Nov 7, 2024
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