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

Respect include_ids_guide in combination optimization #396

Closed
wants to merge 3 commits into from

Conversation

jeanconn
Copy link
Contributor

@jeanconn jeanconn commented Apr 10, 2024

Description

Respect include_ids_guide in combination optimization

Fixes bug noticed on prelim review of obsid 26719 that the combination optimization to avoid close guide stars ignored the include_ids_guide.

Interface impacts

Testing

Unit tests

  • Mac
(ska3-masters) flame:proseco jean$ pytest
================================================ test session starts ================================================
platform darwin -- Python 3.11.8, pytest-8.0.2, pluggy-1.4.0
rootdir: /Users/jean/git
configfile: pytest.ini
plugins: timeout-2.2.0, anyio-4.3.0
collected 167 items                                                                                                 

proseco/tests/test_acq.py .....................................                                               [ 22%]
proseco/tests/test_catalog.py ..........................................                                      [ 47%]
proseco/tests/test_core.py ...........................                                                        [ 63%]
proseco/tests/test_diff.py ......                                                                             [ 67%]
proseco/tests/test_fid.py ...............                                                                     [ 76%]
proseco/tests/test_guide.py ....................................                                              [ 97%]
proseco/tests/test_mon_full_cat.py ....                                                                       [100%]

=============================================== 167 passed in 30.78s ================================================
(ska3-masters) flame:proseco jean$ git rev-parse HEAD
480a134e9dde52303308f57c1567f173b91f6bd0

Independent check of unit tests by [REVIEWER NAME]

  • [PLATFORM]:

Functional tests

Previously 26719 did this:

acas = pickle.load(gzip.open('/Users/jean/APR0824_aca_question_2024_093_16_03_57_688.pkl.gz'))
acas[26719].pprint()
slot idx     id    type  sz p_acq  mag  maxmag   yang     zang   dim res halfw
---- --- --------- ---- --- ----- ----- ------ -------- -------- --- --- -----
   0   1         2  FID 8x8 0.000  7.00   8.00  -767.76 -1745.27   1   1    25
   1   2         4  FID 8x8 0.000  7.00   8.00  2145.67   163.39   1   1    25
   2   3         5  FID 8x8 0.000  7.00   8.00 -1820.84   156.93   1   1    25
   3   4 188504240  BOT 8x8 0.978  5.37   6.87   476.61  1572.62  16   1   100
   4   5 188744824  BOT 8x8 0.965  8.32   9.82 -2170.78  2349.37  16   1   100
   5   6 188747592  BOT 8x8 0.848  9.51  11.01 -1082.05   886.42  16   1   100
   6   7 188747008  BOT 8x8 0.772  9.75  11.19 -1181.01   601.06  16   1   100
   7   8 188356208  BOT 8x8 0.753 10.02  11.20  1357.75 -1241.32   8   1    60
   0   9 188488808  ACQ 8x8 0.768  9.97  11.20  -346.98  1127.42   8   1    60
   1  10 188757072  ACQ 8x8 0.582 10.31  11.20 -2383.87 -1490.79   8   1    60
   2  11 188501440  ACQ 8x8 0.589 10.39  11.20  1414.78   616.57   8   1    60
args = acas[26719].call_args.copy()
args['n_guide'] = 4
args['include_ids_guide'] = [188744824, 188747592, 188747008, 188356208]
cat = proseco.get_aca_catalog(**args)
cat.pprint()
slot idx     id    type  sz p_acq  mag  maxmag   yang     zang   dim res halfw
---- --- --------- ---- --- ----- ----- ------ -------- -------- --- --- -----
   0   1         2  FID 8x8 0.000  7.00   8.00  -767.76 -1745.27   1   1    25
   1   2         4  FID 8x8 0.000  7.00   8.00  2145.67   163.39   1   1    25
   2   3         5  FID 8x8 0.000  7.00   8.00 -1820.84   156.93   1   1    25
   3   4 188504240  BOT 8x8 0.978  5.37   6.87   476.61  1572.62  16   1   100
   4   5 188744824  BOT 8x8 0.965  8.32   9.82 -2170.78  2349.37  16   1   100
   5   6 188747592  BOT 8x8 0.848  9.51  11.01 -1082.05   886.42  16   1   100
   6   7 188356208  BOT 8x8 0.753 10.02  11.20  1357.75 -1241.32   8   1    60
   0   8 188747008  ACQ 8x8 0.772  9.75  11.19 -1181.01   601.06  16   1   100
   1   9 188488808  ACQ 8x8 0.768  9.97  11.20  -346.98  1127.42   8   1    60
   2  10 188757072  ACQ 8x8 0.582 10.31  11.20 -2383.87 -1490.79   8   1    60
   7  11 188501440  ACQ 8x8 0.589 10.39  11.20  1414.78   616.57   8   1    60

With this PR the 4 stars requested are used

In [3]: acas = pickle.load(gzip.open('/Users/jean/APR0824_aca_question_2024_093_16_03_57_688.pkl.gz'))
In [4]: args = acas[26719].call_args.copy()
In [5]: args['n_guide'] = 4
In [6]: args['include_ids_guide'] = [188744824, 188747592, 188747008, 188356208]
In [9]: cat = proseco.get_aca_catalog(**args)

In [10]: cat
Out[10]: 
<ACATable length=11>
 slot  idx      id    type  sz   p_acq    mag    maxmag   yang     zang    dim   res  halfw
int64 int64   int64   str3 str3 float64 float64 float64 float64  float64  int64 int64 int64
----- ----- --------- ---- ---- ------- ------- ------- -------- -------- ----- ----- -----
    0     1         2  FID  8x8   0.000    7.00    8.00  -767.76 -1745.27     1     1    25
    1     2         4  FID  8x8   0.000    7.00    8.00  2145.67   163.39     1     1    25
    2     3         5  FID  8x8   0.000    7.00    8.00 -1820.84   156.93     1     1    25
    3     4 188744824  BOT  8x8   0.965    8.32    9.82 -2170.78  2349.37    16     1   100
    4     5 188747592  BOT  8x8   0.848    9.51   11.01 -1082.05   886.42    16     1   100
    5     6 188747008  BOT  8x8   0.772    9.75   11.19 -1181.01   601.06    16     1   100
    6     7 188356208  BOT  8x8   0.753   10.02   11.20  1357.75 -1241.32     8     1    60
    0     8 188504240  ACQ  8x8   0.978    5.37    6.87   476.61  1572.62    16     1   100
    1     9 188488808  ACQ  8x8   0.768    9.97   11.20  -346.98  1127.42     8     1    60
    2    10 188757072  ACQ  8x8   0.582   10.31   11.20 -2383.87 -1490.79     8     1    60
    7    11 188501440  ACQ  8x8   0.589   10.39   11.20  1414.78   616.57     8     1    60

As a regression test, I reselected the catalogs from the loads from
"2023:001" to "2024:175" (using the pickle args) and confirmed no catalog diffs between this PR and current master.

@jeanconn jeanconn marked this pull request as ready for review April 12, 2024 06:29
@jeanconn jeanconn marked this pull request as draft April 12, 2024 06:30
@jeanconn jeanconn marked this pull request as ready for review April 17, 2024 02:41
@jeanconn
Copy link
Contributor Author

Superseded by #397

@jeanconn jeanconn closed this Apr 24, 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.

1 participant