sot / xija

Thermal modeling framework for Chandra X-ray Observatory
https://sot.github.io/xija
BSD 3-Clause "New" or "Revised" License
9 stars 5 forks source link

Expand the upper end of the supported pitch range from 170 to 180 degrees #59

Closed matthewdahmer closed 5 years ago

matthewdahmer commented 5 years ago

This expands the upper end of the supported pitch range from 170 degrees (169.999 degrees) to 180 degrees (179.999 degrees) to reflect the expanded allowable pitch range being explored.

matthewdahmer commented 5 years ago

@taldcroft Per your suggestion, I added a test to ensure the new pitch range is being used, and corrected the version tag.

jeanconn commented 5 years ago

Plan out of TWG is for ACIS to test models from this branch and for @jeanconn to functionally check starcheck before merging this.

jeanconn commented 5 years ago

WRT starcheck testing: I set up a test environment with xija 4.14 pip-installed, and ran the starcheck regression test set (11 loads run with ACA thermal model) and MAY2019A. No thermal-model-related diffs and no errors out of starcheck with xija 4.14.

matthewdahmer commented 5 years ago

I checked the production DPA, DEA, and ACISFP models, which do have definition past 170, and found only very minor differences in prediction accuracy. The differences I observed do not warrant a refit to support Xija 4.14. I am waiting for agreement from ACIS before posting the results to the TWiki.

jzuhone commented 5 years ago

@matthewdahmer I saw your plots and they look good, I will try to look at the DEA, FP temp, and PSMC models tomorrow (at home today with sick kid). @Gregg140 will have to comment on his tests of the DPA model.

Gregg140 commented 5 years ago

I, too, ran comparisons between the old xija code and the new on the 1DPAMZT model time slices. There were very small though discernable differences. The update looks good to me.

Gregg140 commented 5 years ago

You had not yet received John Zuhone's comments/approval before merging it, though he was invited to approve.

The Focal Plane model is just as affected as 1DPAMZT.

jzuhone commented 5 years ago

I don't anticipate disagreeing with @matthewdahmer, but I do want to take a look at it before we promote it. I can do that tonight.

taldcroft commented 5 years ago

@Gregg140 - oops, sorry, I misread that @jzuhone had delegated to you. We'll hold on doing anything with the new xija.

jzuhone commented 5 years ago

Ok, this looks good to me after some analysis.