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

Check and plot the ACIS FP safety planning limit, support the new ACIS FP model #50

Merged
merged 9 commits into from
May 19, 2022

Conversation

jzuhone
Copy link
Member

@jzuhone jzuhone commented May 17, 2022

Description

This PR does three things:

  1. Add support for plotting the ACIS safety planning and caution limits on one of the model plots (adjusts the y-axis limit for this plot so they can be seen).
  2. Add support for checking violations of the ACIS safety planning limit. A test has been added for this functionality.
  3. Support the new ACIS FP thermal model which can incorporate the Earth phase in its Earth solid angle calculation (Adjust earthshine effect based on Earth phase sot/xija#117)

Interface impacts

The ACIS FP thermal model web pages will now show lines for the planning and caution safety limits on one of the plots, and any violations of the planning limit will now be reported.

Testing

Unit tests

  • Mac
  • Linux

Functional tests

  • Checked that the web pages are produced as expected
  • Artificially lowered limits to produce a violation, checked that it was in the expected location

Copy link

@jazan12 jazan12 left a comment

Choose a reason for hiding this comment

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

Looks good.

@Gregg140
Copy link
Contributor

Question:
In acisfp_check.py you have (line 50):

        "planning.warning.high": "planning_hi",

In MAY0922A_viol.json you have (line 3,4,5):
"limits": {
"planning_hi": -97.0
},

And back in acisfp_check.py you have (lines 283 to 295):

---------------------------------------------------------------

    # Planning - Collect any -84 C violations. These are load killers
    # ---------------------------------------------------------------

    hi_viols = self._make_prediction_viols(
        times, temp, load_start, self.limits["planning_hi"].value,
        "planning", "max")
    viols = {"hi":
                 {"name": f"Planning High ({self.limits['planning_hi'].value} C)",
                  "type": "Max",
                  "values": hi_viols}
             }

How/where did self.limits["planning_hi"].value get set to 84?

@jzuhone
Copy link
Member Author

jzuhone commented May 18, 2022

Hi @Gregg140,

How/where did self.limits["planning_hi"].value get set to 84?

This is taken from the limits directory in the JSON file. In the test JSON file MAY0922A_viol.json we override it to fake a violation and test the violation reporting functionality.

@Gregg140
Copy link
Contributor

Ok thanks.

Copy link
Contributor

@Gregg140 Gregg140 left a comment

Choose a reason for hiding this comment

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

Looks good

@jzuhone jzuhone merged commit 384a17b into acisops:master May 19, 2022
@javierggt javierggt mentioned this pull request May 31, 2022
@javierggt javierggt mentioned this pull request Aug 10, 2022
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.

3 participants