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

Updated 'Hack' to only overwrite final prediction mvals instead of all mvals + PEP8 #121

Closed jskrist closed 2 years ago

jskrist commented 2 years ago

Description

There was a "Hack" in the model.py code to make sure that calculated data was returned for all entries of the mvals. This code essentially copies the second-to-last value (which is computed by the calc function) into the last value as well. This logic works fine for predicted values, and results in the last two data points being the same exact value. However, for non-predicted values, this results in overwritten input data. Most of the time, the input states aren't changing significantly in the last two data point, so this doesn't cause any issues. However, when the last data point of an input state does change significantly, then the returned mvals will no longer match the given inputs.

The fix presented in this PR is to only overwrite the last values for components that are predictions, and to leave non-predicted values alone.

Interface impacts

This does not change the API, and should have minimal impacts on the user community in most cases.

Testing

Unit tests

I was unable to run the unit test on Windows due to not having the Ska data available locally.

Independent check of unit tests by @taldcroft

Functional tests

This was tested using the FOT MATLAB Tools running MCC and evaluating all the thermal models for a given week. The thermal predictions were checked against the values without this fix, and the mvals from non-predicted components were compared to input states.

In particular, the final value of the fep_count component of the ACIS DPA model was compared to the provided inputs from the MAY0222A schedule.

taldcroft commented 2 years ago

Thanks @jskrist !