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

Ignore mag clipping chandra_aca warnings #416

Merged
merged 1 commit into from
Jul 6, 2023
Merged

Ignore mag clipping chandra_aca warnings #416

merged 1 commit into from
Jul 6, 2023

Conversation

jeanconn
Copy link
Contributor

@jeanconn jeanconn commented Jul 5, 2023

Description

Ignore mag clipping chandra_aca star_probs warnings from the acquisition model.

Fixes #414

Interface impacts

Testing

Unit tests

  • No unit tests

Functional tests

On fido/linux, with CHANDRA_MODELS_REPO_DIR set to /home/jeanconn/git/chandra_models and that repo checked out to have a version of the recent acquisition model in it, I confirmed that master gives these warnings

Writing plot file jul0323t/ccd_temperature.png
/fido.real/conda/envs/ska3-matlab-2023.4rc6/lib/python3.10/site-packages/chandra_aca/star_probs.py:349: UserWarning: 
Model grid-* computed between mag <= 5.0 <= 10.75, clipping input mag(s) outside that range.
  warnings.warn(
Checking star catalog for obsid 44375

and they aren't present with this PR.

Writing plot file jul0323t/ccd_temperature.png
Checking star catalog for obsid 44375
Checking star catalog for obsid 44374

@jeanconn
Copy link
Contributor Author

jeanconn commented Jul 5, 2023

I'm a little surprised that the mags are clipped at 10.75 from the acquisition model and don't quite remember what that means for various temperatures, but that's neither here nor there for this PR. There is the question about if starcheck should be checking anything else when the new acquisition model is promoted. @taldcroft ?

@jeanconn jeanconn requested a review from taldcroft July 5, 2023 20:27
@jeanconn
Copy link
Contributor Author

jeanconn commented Jul 5, 2023

I was seeing this warning for every catalog, but it looks like there aren't that any fainter-than-10.75 in the actual catalogs of the week I was using in testing, so likely these come up via some other method (probabilities of the not-selected candidates). From a starcheck perspective we probably want to warn with a CAUTION if the mag of an acq star is fainter than 10.75 or not include it in the acquisition probability.

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.

LGTM. Per discussion we may want to warn on acq stars fainter than 10.75, but it is not urgent. This is pretty rare, 10 stars total (out of 10000 acqs) this year:

obsid      starcat_date         id      mag   halfw
int64         str21           int64   float64 int64
----- --------------------- --------- ------- -----
26880 2023:071:23:56:07.591 261248792   10.78    80
26880 2023:071:23:56:07.591 190844704   10.79    80
26836 2023:084:01:38:41.457  35657496   10.80    60
26836 2023:084:01:38:41.457  35659096   10.92    60
25341 2023:093:15:19:23.839 189012272   10.80    80
25317 2023:097:03:03:14.248 189150896   10.87    60
44659 2023:097:06:17:58.943 189138488   10.88    60
25565 2023:155:17:19:35.346 261621792   10.84    60
25780 2023:167:03:39:53.004 331755840   10.80    60
25780 2023:167:03:39:53.004 331754064   10.83    80

@jeanconn
Copy link
Contributor Author

jeanconn commented Jul 6, 2023

Thanks for doing that sanity check on the super faint stars! Though I suppose if more fields are passable for guide with the dynamic background bonus, maybe we'll find ourselves on the P2=2.0 edge more frequently? Will see!

@jeanconn jeanconn merged commit 7ab19df into master Jul 6, 2023
@jeanconn jeanconn deleted the no-clip-warns branch July 11, 2023 15:43
@javierggt javierggt mentioned this pull request Aug 9, 2023
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.

Hide console warnings about mag outside of grid model range
2 participants