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

Add grid-local-quadratic-2023-05 acq probability model #106

Merged
merged 1 commit into from
Jun 27, 2023

Conversation

taldcroft
Copy link
Member

@taldcroft taldcroft commented Apr 27, 2023

Description

This adds the new acquisition probability model that was developed in sot/aca_stats#16.

Closes #99

NOTE: this can be merged and promoted at any point without any impact to operations. Only when sot/chandra_aca#146 is addressed and promoted to flight will this new file be used operationally.

Functional testing

In a ska3-dev environment with the current master of chandra_aca etc:

In [1]: os.environ["THERMAL_MODELS_DIR_FOR_MATLAB_TOOLS_SW"]
Out[1]: '/Users/aldcroft/git/chandra_models'

In [2]: from ska_helpers.paths import chandra_models_repo_path
   ...: from chandra_aca.star_probs import acq_success_prob
   ...: chandra_models_repo_path()
   ...:
Out[2]: PosixPath('/Users/aldcroft/git/chandra_models')

In [3]: acq_success_prob(mag=9.75, halfwidth=140, t_ccd=0)  # Flight model
   ...: 
Out[3]: 0.08876371372991076

In [4]: acq_success_prob(  # New model
   ...:     mag=9.75, halfwidth=140, t_ccd=0, model="grid-local-quadratic-2023-05"
   ...: )
   ...: 
Out[4]: 0.40033623318863376

These values match expectation based on this plot from https://github.com/sot/aca_stats/blob/master/fit_acq_model-2023-05-local-quadratic.ipynb, where the x-axis ticks are at -15, -10, -5, and 0 C respectively and the y-axis goes from 0.0 to 1.0.

image

Copy link

@jeanconn jeanconn left a comment

Choose a reason for hiding this comment

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

I'm a little concerned this is a model fit with agasc_file agasc1p8rc1.h5 , and I don't know where that is or what was in it at the time of the fit. Overall that's probably fine but I think requires that we archive that file someplace.

@taldcroft
Copy link
Member Author

I'm a little concerned this is a model fit with agasc_file agasc1p8rc1.h5

The mags in rc1 will be identical to the final AGASC 1.8 since no more changes to mag estimation algorithm are planned. Any small changes in final star correlations will not impact the probabilities at a level that is observable. So I don't understand what value there will be in archiving that file since it will never get used again.

@jeanconn
Copy link

OK, if the rc1 file is the latest-draft-mags that are expected for 1.8 that sounds fine.

@taldcroft taldcroft merged commit 671973b into master Jun 27, 2023
@taldcroft taldcroft deleted the add-grid-local-quadratic-2023-05 branch June 27, 2023 13:55
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.

Add "local-quadratic" acquisition probability model to chandra_models
2 participants