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

Update a few tests to use old dyn_bgd_n_faint=0 default #214

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jeanconn
Copy link
Contributor

@jeanconn jeanconn commented Oct 31, 2024

Description

Update a few tests to use old dyn_bgd_n_faint=0 default.

This PR is intended to go with sot/proseco#403 , though explicitly adding dyn_bgd_n_faint=0 to a few tests does not require that version of proseco.

Interface impacts

Testing

Tested with and without sot/proseco#403 in PYTHONPATH . Requires sot/proseco#401 or proseco >= 5.15.0.

Unit tests

  • Mac
(ska3-flight-latest) flame:sparkles jean$ python -c "import proseco; print(proseco.__version__)"
5.15.1.dev4+g09710b0
(ska3-flight-latest) flame:sparkles jean$ git rev-parse HEAD
61a57818bf7c93c38843a13f679ebbf4fef39129
(ska3-flight-latest) flame:sparkles jean$ pytest
=========================================================================== test session starts ===========================================================================
platform darwin -- Python 3.11.8, pytest-8.0.2, pluggy-1.4.0
PyQt5 5.15.9 -- Qt runtime 5.15.8 -- Qt compiled 5.15.8
rootdir: /Users/jean/git
configfile: pytest.ini
plugins: astropy-0.11.0, qt-4.4.0, cov-5.0.0, timeout-2.2.0, remotedata-0.4.1, anyio-4.3.0, filter-subpackage-0.2.0, doctestplus-1.2.1, astropy-header-0.2.2, hypothesis-6.112.0, arraydiff-0.6.1, mock-3.14.0
collected 103 items                                                                                                                                                       

sparkles/tests/test_checks.py ............................................................................                                                          [ 73%]
sparkles/tests/test_find_er_catalog.py .....                                                                                                                        [ 78%]
sparkles/tests/test_review.py ..................                                                                                                                    [ 96%]
sparkles/tests/test_yoshi.py ....                                                                                                                                   [100%]

========================================================================== 103 passed in 26.68s

Independent check of unit tests by [REVIEWER NAME]

  • [PLATFORM]:

Functional tests

No functional testing.

@jeanconn
Copy link
Contributor Author

For this I suppose the question is if there's enough coverage for dyn_bgd_n_faint=0 and dyn_bgd_n_faint=2.

@taldcroft
Copy link
Member

In general there seems to be pretty good coverage for dyn_bgd_n_faint having different values for single-catalog review.

It's good news that some review tests failed with the new default. I guess the question is whether the roll optimization is really doing the right thing in this case, i.e. using the bonus in evaluation. There is no obvious reason it wouldn't be working, but I'm not sure if that is tested.

@jeanconn
Copy link
Contributor Author

jeanconn commented Nov 1, 2024

I looked a bit at the roll optimization question, and it looked to me like the kwargs were getting through correctly, but because that code had a separate area to recalculate n_stars (aka guide_count) for the rolled catalogs, and that guide_count did not use the new bonus method, the bonus wasn't getting into the n_stars value. If n_stars is just supposed to be guide_count, I think it probably makes sense to just update those values to use the bonus per #215 .

@taldcroft
Copy link
Member

Something is wonky, tests are failing for me. ???

(ska3) ➜  sparkles git:(proseco-n-faint-default) git rev-parse HEAD                                    
61a57818bf7c93c38843a13f679ebbf4fef39129
(ska3) ➜  sparkles git:(proseco-n-faint-default) python -c "import proseco; print(proseco.__version__)"
5.13.2
(ska3) ➜  sparkles git:(proseco-n-faint-default) pytest --log-level=CRITICAL
================================================= test session starts ==================================================
platform darwin -- Python 3.11.8, pytest-7.4.4, pluggy-1.4.0
rootdir: /Users/aldcroft/git
configfile: pytest.ini
plugins: cov-5.0.0, timeout-2.2.0, anyio-4.3.0
collected 103 items                                                                                                    

sparkles/tests/test_checks.py ............................................................................       [ 73%]
sparkles/tests/test_find_er_catalog.py .....                                                                     [ 78%]
sparkles/tests/test_review.py .F.......FF.......                                                                 [ 96%]
sparkles/tests/test_yoshi.py ....                                                                                [100%]

======================================================= FAILURES =======================================================
_________________________________________________ test_review_catalog __________________________________________________

proseco_agasc_1p7 = None
tmpdir = local('/private/var/folders/0p/mj5zm2311gl283lksrknl7bc0000gp/T/pytest-of-aldcroft/pytest-187/test_review_catalog0')

    def test_review_catalog(proseco_agasc_1p7, tmpdir):
        aca = get_aca_catalog(**KWARGS_48464)
        acar = aca.get_review_table()
        acar.run_aca_review()
>       assert acar.messages == [
            {
                "category": "warning",
                "text": "Guide star imposter offset 2.6, limit 2.5 arcsec",
                "idx": 4,
            },
            {"category": "warning", "text": "P2: 3.33 less than 4.0 for ER"},
            {
                "category": "critical",
                "text": "ER count of 9th (8.9 for -9.9C) mag guide stars 1.91 < 3.0",
            },
            {"category": "critical", "text": "ER count of guide stars 5.00 < 6.0"},
            {"category": "caution", "text": "ER with 5 guides but 8 were requested"},
        ]
E       AssertionError: assert [{'category':...e requested'}] == [{'category':...e requested'}]
E         At index 3 diff: {'category': 'caution', 'text': 'ER with 6 guides but 8 were requested'} != {'category': 'critical', 'text': 'ER count of guide stars 5.00 < 6.0'}
E         Right contains one more item: {'category': 'caution', 'text': 'ER with 5 guides but 8 were requested'}
E         Use -v to get more diff

sparkles/tests/test_review.py:72: AssertionError
_____________________________________________ test_run_aca_review_function _____________________________________________

proseco_agasc_1p7 = None
tmpdir = local('/private/var/folders/0p/mj5zm2311gl283lksrknl7bc0000gp/T/pytest-of-aldcroft/pytest-187/test_run_aca_review_function0')

    def test_run_aca_review_function(proseco_agasc_1p7, tmpdir):
        aca = get_aca_catalog(**KWARGS_48464)
        acar = aca.get_review_table()
        acars = [acar]
    
        exc = run_aca_review(load_name="test_load", report_dir=tmpdir, acars=acars)
    
        assert exc is None
>       assert acar.messages == [
            {
                "category": "warning",
                "text": "Guide star imposter offset 2.6, limit 2.5 arcsec",
                "idx": 4,
            },
            {"category": "warning", "text": "P2: 3.33 less than 4.0 for ER"},
            {
                "category": "critical",
                "text": "ER count of 9th (8.9 for -9.9C) mag guide stars 1.91 < 3.0",
            },
            {"category": "critical", "text": "ER count of guide stars 5.00 < 6.0"},
            {"category": "caution", "text": "ER with 5 guides but 8 were requested"},
        ]
E       AssertionError: assert [{'category':...e requested'}] == [{'category':...e requested'}]
E         At index 3 diff: {'category': 'caution', 'text': 'ER with 6 guides but 8 were requested'} != {'category': 'critical', 'text': 'ER count of guide stars 5.00 < 6.0'}
E         Right contains one more item: {'category': 'caution', 'text': 'ER with 5 guides but 8 were requested'}
E         Use -v to get more diff

sparkles/tests/test_review.py:341: AssertionError
_________________________________________ test_run_aca_review_dyn_bgd_n_faint __________________________________________

proseco_agasc_1p7 = None
tmpdir = local('/private/var/folders/0p/mj5zm2311gl283lksrknl7bc0000gp/T/pytest-of-aldcroft/pytest-187/test_run_aca_review_dyn_bgd_n_0')

    def test_run_aca_review_dyn_bgd_n_faint(proseco_agasc_1p7, tmpdir):
        aca = get_aca_catalog(**KWARGS_48464)
        acar = aca.get_review_table()
        acars = [acar]
    
        exc = run_aca_review(
            load_name="test_load",
            report_dir=tmpdir,
            acars=acars,
            dyn_bgd_n_faint=2,
        )
    
        assert exc is None
        assert acar.dyn_bgd_n_faint == 2
>       assert np.isclose(acar.guide_count, 5, atol=0.1)
E       assert False
E        +  where False = <function isclose at 0x10303ba30>(6.000437968971055, 5, atol=0.1)
E        +    where <function isclose at 0x10303ba30> = np.isclose
E        +    and   6.000437968971055 = <ACAReviewTable length=9>\n idx   slot    id    type  sz   p_acq  ...   row     col    dim   res  halfw\nint64 int64  in....601 ...  -113.3   -67.5     8     1    60\n    9     7 85336992  ACQ  8x8   0.476 ...   141.9   282.9     8     1    60.guide_count

sparkles/tests/test_review.py:377: AssertionError
=============================================== short test summary info ================================================
FAILED sparkles/tests/test_review.py::test_review_catalog - AssertionError: assert [{'category':...e requested'}] == [{'category':...e requested'}]
FAILED sparkles/tests/test_review.py::test_run_aca_review_function - AssertionError: assert [{'category':...e requested'}] == [{'category':...e requested'}]
FAILED sparkles/tests/test_review.py::test_run_aca_review_dyn_bgd_n_faint - assert False
============================================ 3 failed, 100 passed in 21.63s ============================================

@taldcroft
Copy link
Member

It's worth noting that I'm seeing these same failures on master.

@jeanconn
Copy link
Contributor Author

jeanconn commented Nov 4, 2024

I think you need a proseco with at least sot/proseco#401 to have tests passing before this PR.

@jeanconn
Copy link
Contributor Author

jeanconn commented Nov 4, 2024

I updated the testing section.

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