search

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

create a generic solar heating class which is SIM-Z dependent #128

Closed jzuhone closed 2 years ago

jzuhone commented 2 years ago

Description

The AcisPsmcSolarHeat class provides separate sets of pitch bins for various ranges of SIM-Z position. This PR refactors that class into a new class, SimZDepSolarHeat, and then subclasses AcisPsmcSolarHeat and HrcCeaSolarHeat, which supply their own SIM-Z ranges.

Interface impacts

The AcisPsmcSolarHeat class should be completely unaffected, but will test.

Testing

Unit tests

Independent check of unit tests by [REVIEWER NAME]

Functional tests

PSMC model (which is the only one that uses this functionality) has identical outputs before and after this change

In-development ACIS FP model which uses one of the new classes ran as expected.

jzuhone commented 2 years ago

@taldcroft @matthewdahmer @drxre check this out, still need to test it

jzuhone commented 2 years ago

Tests pass

jzuhone commented 2 years ago

@taldcroft I agree re: testing--I have a working model for the ACIS FP using one of the new classes with fewer dP than P--if you want I can show the results somehow.

taldcroft commented 2 years ago

Yeah, just any sort of statement that you ran a model that has different P vs dP and it was fine. Doesn't need to be super detailed, just a sanity check. We'd like to get this into 2022.9 so merging this today would be good if possible.

jzuhone commented 2 years ago

This is ready to go.

taldcroft commented 2 years ago

@jzuhone - FYI my 👍 was just acknowledging that this is ready for review so the merge was a little premature, but OK. I just changed the master branch protection rules to require a review for merge. 😺