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

Apply dynamic background bonus to rolled "n_stars" #215

Merged
merged 2 commits into from
Nov 26, 2024
Merged

Conversation

jeanconn
Copy link
Contributor

@jeanconn jeanconn commented Nov 1, 2024

Description

Apply dynamic background bonus to rolled "n_stars".

The catalogs in roll optimization were correctly carrying forward any dynamic background options (dyn_bgd_n_faint, dyn_bgd_dt_ccd) but these options were not applied to the optimized "n_stars". This PR fixes that for consistency.

Interface impacts

n_stars in the roll optimization table should now match the "final" catalog guide_count used in operations, if the same input kwargs for proseco are used.

Testing

Unit tests

  • Mac with a recent ska3-flight-latest
(ska3-flight-latest) flame:sparkles jean$ git rev-parse HEAD
ab4628e8d2f629d6c42d9d1af60f0064c9e7261c
(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 104 items                                                                                               

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

============================================== 104 passed in 28.53s

Independent check of unit tests by [REVIEWER NAME]

  • [PLATFORM]:

Functional tests

New unit test added.

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.

Awesome! The only nit I could find is that np.allclose() does not require np.array() casting, you can just pass in lists and it works just fine. No need to fix that but remember for the future.

@jeanconn jeanconn changed the base branch from proseco-n-faint-default to master November 26, 2024 13:21
@jeanconn jeanconn merged commit 97424df into master Nov 26, 2024
2 checks passed
@jeanconn jeanconn deleted the rolled-dyn-bgd branch November 26, 2024 13:22
@javierggt javierggt mentioned this pull request Dec 9, 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