acisops / acis_thermal_check

Thermal model prediction and validation for Chandra ACIS Operations
https://cxc.cfa.harvard.edu/acis/acis_thermal_check
0 stars 4 forks source link

Use chandra limits #71

Closed jzuhone closed 7 months ago

jzuhone commented 8 months ago

Description

This version of acis_thermal_check hands violations checking and plotting of limits over completely to the chandra_limits package. A review of the most important changes is as follows:

Practical effects of these changes:

Interface impacts

The table reporting violations on the webpages has changed slightly, but is human-read. Plots show limit lines as a function of time instead of every limit line across the whole plot; also human-read.

Testing

Unit tests

All tests which do not depend on violations checking pass without any answer updates as expected. Some answer changes are necessary for the FP and a couple of other models. All of these answer changes were hand-checked and they were found to be correct based on the code changes.

Independent check of unit tests by [REVIEWER NAME]

Functional tests

Ran the code in this PR at weekly load reviews for several weeks and results were checked for consistency with flight code; any differences are explained by changes to the violations checking changes.

jazan12 commented 7 months ago

Thanks

On Thu, Mar 28, 2024 at 8:47 AM Jean Connelly @.***> wrote:

@.**** commented on this pull request.

In acis_thermal_check/main.py https://github.com/acisops/acis_thermal_check/pull/71#discussion_r1542905880 :

@@ -1393,6 +1286,23 @@ def write_index_rst(self, outdir, context):

Render the template and write it to a file

with open(outfile, "w") as fout: fout.write(template.render(**context))

  • if self.msid == "fptemp":
  • For the ACIS FP limit we write out an obsid table

  • context["acis_obs"] = self.acis_and_ecs_obs
  • template_path = (
  • TASK_DATA / "acis_thermal_check/templates/obsid_template.rst"

In passing - I think it is imported from utils here https://github.com/acisops/acis_thermal_check/blob/4f7c496bb51f327b16a75066896d5c5e5dfbe55d/acis_thermal_check/main.py#L25

— Reply to this email directly, view it on GitHub https://github.com/acisops/acis_thermal_check/pull/71#discussion_r1542905880, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACN5TGO43PQIXA35QFQCBG3Y2QGMDAVCNFSM6AAAAABEQX7YRSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSNRWGEYDMMBTGY . You are receiving this because you commented.Message ID: @.***>

jazan12 commented 7 months ago

Good, so long as the syntax was as intended

On Thu, Mar 28, 2024 at 9:08 AM John ZuHone @.***> wrote:

@.**** commented on this pull request.

On acis_thermal_check/tests/bep_pcb/answers/JUL2919A_viol.json https://github.com/acisops/acis_thermal_check/pull/71#discussion_r1542937148 :

It's not so much "official" vs "informal" as what limits are stored in the ODB (related to engineering telemetry) vs the ones related to DEA HKP telemetry

— Reply to this email directly, view it on GitHub https://github.com/acisops/acis_thermal_check/pull/71#discussion_r1542937148, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACN5TGNJQS6CFRF5TDSDKLDY2QI5NAVCNFSM6AAAAABEQX7YRSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSNRWGE3TCNZTGA . You are receiving this because you commented.Message ID: @.***>

Gregg140 commented 7 months ago

Looks good to me.