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

Work with optional penalty limit in model spec #429

Merged
merged 4 commits into from
Aug 14, 2023
Merged

Conversation

jeanconn
Copy link
Contributor

@jeanconn jeanconn commented Aug 11, 2023

Description

Work with optional penalty limit in model spec

Interface impacts

This removes the model_spec kwarg to starcheck.calc_ccd_temps.get_ccd_temps and sets that method to just use the model spec from chandra_models. It is not expected that any external code is using get_ccd_temps and this kwarg was not used from starcheck.

Testing

Tested with sot/proseco#384 as starcheck uses get_effective_t_ccd and it won't matter if starcheck supplies a penalty limit or not if the proseco in the path is overriding None with the limit from the get_xija_model_spec version.

Unit tests

  • Linux

Independent check of unit tests by [REVIEWER NAME]

  • [PLATFORM]:

Functional tests

On Linux in https://icxc.cfa.harvard.edu/aspect/test_review_outputs/starcheck-pr429/

# Make some flight output
starcheck -dir /data/mpcrit1/mplogs/2023/AUG0723/oflsa -out aug0723a_flight

# Run the test code with the previous version of the penalty limit to see the line works and penalty actually applied.
export PYTHONPATH=/home/jeanconn/git/proseco 
export CHANDRA_MODELS_REPO_DIR=/home/jeanconn/git/chandra_models
export CHANDRA_MODELS_DEFAULT_VERSION=3.48
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2023/AUG0723/oflsa -out aug0723a_test_3.48

# Run the test code with the current version of the penalty limit to confirm no errors
export CHANDRA_MODELS_DEFAULT_VERSION=3.49
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2023/AUG0723/oflsa -out aug0723a_test_3.49

# Run the test code with the penalty removed to confirm no errors
export CHANDRA_MODELS_DEFAULT_VERSION=remove-penalty
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2023/AUG0723/oflsa -out aug0723a_test_no-penalty

# Diff the flight output with the 3.49 test output to confirm reasonable diffs 
diff aug0723a_flight.txt aug0723a_test_3.49.txt > diff.txt

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.

I know this is a WIP but I couldn't help looking at it and then I had an idea.

starcheck/calc_ccd_temps.py Outdated Show resolved Hide resolved
@jeanconn jeanconn changed the title WIP: Work with optional penalty limit in model spec Work with optional penalty limit in model spec Aug 13, 2023
@jeanconn jeanconn requested a review from taldcroft August 13, 2023 19:44
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.

I reviewed the code and the test outputs. Looks good!

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