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

Set starcheck to use agasc module default agasc file #444

Merged
merged 6 commits into from
Jul 1, 2024
Merged

Conversation

jeanconn
Copy link
Contributor

@jeanconn jeanconn commented Jun 23, 2024

Description

Set starcheck to use agasc module default agasc file. This also removes the agasc_file cmdline option for starcheck as that interface wasn't being used and proseco respects the agasc module environment variables for control of this.

This also fixes an unrelated bug introduced in b4fa401 where the arguments to the warning print for centroid perturbation (ASPQ1) were not carried forward into the warning when it was made "orange". This came up in testing as a used star hit this warning due to AGASC 1.8 changes.

Interface impacts

Testing

Unit tests

  • Mac
(ska3-flight-2024.7rc2) flame:starcheck jean$ git rev-parse HEAD
4297c1dc2102459d8ce311b2091bd1d601790349
(ska3-flight-2024.7rc2) flame:starcheck jean$ pytest
======================================================================== test session starts =========================================================================
platform darwin -- Python 3.11.8, pytest-7.4.4, pluggy-1.4.0
rootdir: /Users/jean/git
configfile: pytest.ini
plugins: timeout-2.2.0, anyio-4.3.0
collected 14 items                                                                                                                                                   

starcheck/tests/test_state_checks.py .............                                                                                                             [ 92%]
starcheck/tests/test_utils.py .                                                                                                                                [100%]

========================================================================== warnings summary ==========================================================================
../../miniconda3/envs/ska3-flight-2024.7rc2/lib/python3.11/site-packages/tables/node.py:251
../../miniconda3/envs/ska3-flight-2024.7rc2/lib/python3.11/site-packages/tables/node.py:251
starcheck/starcheck/tests/test_state_checks.py::test_get_obs_man_angle[646940289.224-143.4]
  /Users/jean/miniconda3/envs/ska3-flight-2024.7rc2/lib/python3.11/site-packages/tables/node.py:251: DeprecationWarning: `alltrue` is deprecated as of NumPy 1.25.0, and will be removed in NumPy 2.0. Please use `all` instead.
    self._v_objectid = self._g_open()

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=================================================================== 14 passed, 3 warnings in 4.93

Independent check of unit tests by [REVIEWER NAME]

  • [PLATFORM]:

Functional tests

Running starcheck with a supplied agasc via env var works as expected and completes without error.

env AGASC_HDF5_FILE=/Users/jean/ska/data/agasc/proseco_agasc_1p7.h5 ./sandbox_starcheck -dir MAY1324B -out may1324_1p7_test
...
Using agasc_file file /Users/jean/ska/data/agasc/proseco_agasc_1p7.h5
...
Wrote text report to may1324_1p7_test.txt

Running starcheck in an environment where the proseco 1p8 agasc is the default picks that up correctly, but actually doesn't finish, due to missing agasc id (with faint star commanded as an acq in MAY1324B). This shows the correct proseco file is being used not just printed in the output.

./sandbox_starcheck -dir MAY1324B -out may1324_def_test
...
Using agasc_file file /Users/jean/ska/data/agasc/proseco_agasc_1p8.h5
...
Checking star catalog for obsid 28807

Python exception:
command = {
  'kwargs' => {
                't_ccd_acq' => '-8.52441995434356',
                't_ccd_guide' => '-8.52393833874498',
                'n_acq' => 8,
                'include_ids_acq' => [
                                       44966344,
                                       44965624,
                                       44960448,
                                       44961784,
                                       44959152,
                                       45490016,
                                       44960960,
                                       44963160
                                     ],
                'man_angle' => '99.3805492843303',
                'detector' => 'ACIS-I',
                'obsid' => 27211,
                'att' => [
                           '-0.400675762',
                           '0.872739542',
                           '-0.269730121',
                           '0.0709245214'
                         ],
                'n_fid' => 3,
                'dither_guide' => [
                                    '15.9978965344218',
                                    '15.9978965344218'
                                  ],
                'date' => '2024:139:10:12:51.931',
                'acq_indexes' => [
                                   4,
                                   5,
                                   6,
                                   7,
                                   9,
                                   10,
                                   11,
                                   12
                                 ],
                'dither_acq' => [
                                  '15.9978965344218',
                                  '15.9978965344218'
                                ],
                'include_ids_guide' => [
                                         44966344,
                                         44965624,
                                         44960448,
                                         44961784,
                                         44965328
                                       ],
                'n_guide' => 5,
                'fid_ids' => [
                               1,
                               5,
                               6
                             ],
                'sim_offset' => 0,
                'include_halfws_acq' => [
                                          160,
                                          160,
                                          160,
                                          160,
                                          60,
                                          60,
                                          120,
                                          80
                                        ]
              },
  'func' => 'utils.proseco_probs',
  'key' => 'il4jvW25rNGEjIaq',
  'args' => []
}

Traceback (most recent call last):
  File "/Users/jean/miniconda3/envs/ska3-flight-2024.7rc2/lib/python3.11/site-packages/proseco/core.py", line 494, in get_id_idx
    idx = (self._id_index_mon if mon else self._id_index)[id]
          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^
KeyError: 44963160

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/jean/miniconda3/envs/ska3-flight-2024.7rc2/lib/python3.11/site-packages/proseco/core.py", line 499, in get_id_idx
    idx = (self._id_index_mon if mon else self._id_index)[id]
          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^
KeyError: 44963160

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/jean/miniconda3/envs/ska3-flight-2024.7rc2/lib/python3.11/site-packages/proseco/core.py", line 890, in process_include_ids
    star = stars.get_id(include_id)
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/jean/miniconda3/envs/ska3-flight-2024.7rc2/lib/python3.11/site-packages/proseco/core.py", line 478, in get_id
    return self[self.get_id_idx(id, mon)]
                ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/jean/miniconda3/envs/ska3-flight-2024.7rc2/lib/python3.11/site-packages/proseco/core.py", line 502, in get_id_idx
    raise KeyError(f"{id} is not in table")
KeyError: '44963160 is not in table'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/jean/git/starcheck/starcheck/server.py", line 71, in handle
    result = func(*args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^
  File "/Users/jean/git/starcheck/starcheck/utils.py", line 548, in proseco_probs
    aca = get_aca_catalog(**args)
          ^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/jean/miniconda3/envs/ska3-flight-2024.7rc2/lib/python3.11/site-packages/proseco/catalog.py", line 65, in get_aca_catalog
    aca = _get_aca_catalog(obsid=obsid, raise_exc=raise_exc, **kwargs)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/jean/miniconda3/envs/ska3-flight-2024.7rc2/lib/python3.11/site-packages/proseco/catalog.py", line 132, in _get_aca_catalog
    aca.acqs = get_acq_catalog(stars=aca.stars, **kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/jean/miniconda3/envs/ska3-flight-2024.7rc2/lib/python3.11/site-packages/proseco/acq.py", line 235, in get_acq_catalog
    acqs.cand_acqs = acqs.get_acq_candidates(acqs.stars)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/jean/miniconda3/envs/ska3-flight-2024.7rc2/lib/python3.11/site-packages/proseco/acq.py", line 540, in get_acq_candidates
    self.process_include_ids(cand_acqs, stars)
  File "/Users/jean/miniconda3/envs/ska3-flight-2024.7rc2/lib/python3.11/site-packages/proseco/acq.py", line 695, in process_include_ids
    super().process_include_ids(cand_acqs, stars)
  File "/Users/jean/miniconda3/envs/ska3-flight-2024.7rc2/lib/python3.11/site-packages/proseco/core.py", line 892, in process_include_ids
    raise ValueError(
ValueError: cannot include star id=44963160 that is not a valid star in the ACA field of view

The full agasc 1p8 works in this case

env AGASC_HDF5_FILE=/Users/jean/ska/data/agasc/agasc1p8.h5 ./sandbox_starcheck -dir MAY1324B -out may1324_agasc1p8_test
...
Using agasc_file file /Users/jean/ska/data/agasc/agasc1p8.h5
...
Checking star catalog for obsid 28807
Checking star catalog for obsid 27211
Checking star catalog for obsid 43438
Checking star catalog for obsid 28118
Checking star catalog for obsid 28272
Checking star catalog for obsid 29409
Checking star catalog for obsid 43437
Checking star catalog for obsid 29405
Checking star catalog for obsid 27402
Checking star catalog for obsid 28792
Checking star catalog for obsid 28225
Checking star catalog for obsid 29408
Checking star catalog for obsid 43436
Wrote HTML report to may1324_agasc1p8_test.html
Wrote text report to may1324_agasc1p8_test.txt

With regard to the update to the centroid perturbation warning - without the update, on the same MAY1324 products one sees:

Missing argument in sprintf at starcheck/src/lib/Ska/Starcheck/Obsid.pm line 1266.
Missing argument in sprintf at starcheck/src/lib/Ska/Starcheck/Obsid.pm line 1266.
Missing argument in sprintf at starcheck/src/lib/Ska/Starcheck/Obsid.pm line 1266.

and gets output with numeric missing values replaced incorrectly with zeros:

>> WARNING : [ 0] Centroid Perturbation Warning.  : ASPQ1 =  0

With the fixes around line 1266, there's no sprintf warning and the output looks correctly like:

>> WARNING : [10] Centroid Perturbation Warning.  1024867184: ASPQ1 = 41

@jeanconn jeanconn marked this pull request as ready for review June 23, 2024 14:01
@jeanconn jeanconn requested a review from taldcroft June 23, 2024 14:01
Copy link
Member

@taldcroft taldcroft left a comment

Choose a reason for hiding this comment

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

Looks good, just a couple of minor comments.

starcheck/src/lib/Ska/Starcheck/Obsid.pm Outdated Show resolved Hide resolved
starcheck/src/starcheck.pl Outdated Show resolved Hide resolved
@taldcroft
Copy link
Member

Can you document that you inspected the perturbation warning (that was changed) and saw the expected outputs?

@jeanconn jeanconn requested a review from taldcroft June 30, 2024 12:04
@jeanconn
Copy link
Contributor Author

jeanconn commented Jul 1, 2024

Last lines of description now include functional test of centroid perturbation warning.

@jeanconn jeanconn merged commit 1b5ec5e into master Jul 1, 2024
@jeanconn jeanconn deleted the agasc branch July 1, 2024 20:42
@javierggt javierggt mentioned this pull request Jul 30, 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