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

Set plot ymin relative to planning not penalty limit #411

Merged
merged 1 commit into from
Jul 19, 2023

Conversation

jeanconn
Copy link
Contributor

@jeanconn jeanconn commented Jun 16, 2023

Description

Set ACA t_ccd model temperature plot ymin relative to planning not penalty limit. This will be useful if the penalty limit is removed.

Interface impacts

Testing

Unit tests

  • OSX

Functional tests

I set a local version of the ACA thermal model chandra_model to have planning limit -15 and penalty limit 0C and ran on the JUL1723A schedule.

ccd_temperature_mock_15

This is the JUL1723A output with the current thermal model limits

ccd_temperature_jul1723

As expected, this PR sets the Y minimum to the minimum of the data or the planning limit - 1. I needed to move the planning limit lower to see that the plot change, as most schedules have a data range in which the y min on the plot is driven by the y min of the model data.

@jeanconn jeanconn changed the title WIP: Set plot ymin relative to planning not penalty limit Set plot ymin relative to planning not penalty limit Jul 13, 2023
@@ -520,7 +520,7 @@ def make_check_plots(outdir, states, times, temps, tstart, tstop, char):
logger.info('Making temperature check plots')
for fig_id, msid in enumerate(('aca',)):
temp_ymax = max(char.aca_t_ccd_planning_limit, np.max(temps))
temp_ymin = min(char.aca_t_ccd_penalty_limit, np.min(temps))
temp_ymin = min(char.aca_t_ccd_planning_limit - 1, np.min(temps))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could also just auto scale the plot minimum to the data, but I figured there would be no harm in explicitly setting the default min to max range as 1 degree .

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.

@jeanconn jeanconn merged commit f72318e into master Jul 19, 2023
@jeanconn jeanconn deleted the penalty-plot-range branch July 19, 2023 15:45
@javierggt javierggt mentioned this pull request Sep 6, 2023
@javierggt javierggt mentioned this pull request Sep 18, 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.

2 participants