Fix bug in component param set attribute

[taldcroft]

Description

This fixes a 7+ year old bug when trying to do something like:

mdl.comp['solarheat__pf0tank2t'].P_60 = 5.0
mdl.calc()

Note that a work-around right now is:

mdl.comp['solarheat__pf0tank2t'].pars_dict['P_60'] = 5.0

Testing

• Passes unit tests on MacOS
• Functional testing: New unit testing

[jskrist]

This change looks reasonable, and in my testing it doesn't seem to cause any issues. However, I was not able to validate that any behavior has actually changed. I wrote a simple test which changed the tau parameter of the aca model's heatsink__aca0 component, saving the results out to variables before and after the change and then plotting the results. From the description of the issue, and the location of the fix, I expected that without the fix, no difference would be observed in the predicted output, however a change is observed with and without the fix.

Without the fix (xija version 4.18.2):

With the fix (xija version 4.18.3.dev1+g8248b87):

script I used to generate the plots:

import xija
import numpy as np
import matplotlib.pyplot as plt
from Ska.Matplotlib import plot_cxctime

# create model input parameters
model_spec_file = '<PATH_TO_THERMAL_MODELS>\\Thermal_Models\\chandra_models-3.30\\chandra_models\\xija\\aca\\aca_spec.json'
model_start_stop_times = [707375709.184, 707963167.343];
pitchval = np.array([100.5, 56.5, 72.75])
model_times = np.array([707375709.1840, 707375719.1840, 707962267.3430])
# create and initialize the model
aca_model = xija.ThermalModel("model_aca", start=model_start_stop_times[0], stop=model_start_stop_times[1], model_spec=model_spec_file)
aca_model.comp["eclipse"].set_data(False)
aca_model.comp["aacccdpt"].set_data(-9.58980000000000032401,None)
aca_model.comp["aca0"].set_data(-12.93389999999999950830,None)
aca_model.comp["pitch"].set_data(pitchval,model_times)
aca_model.make()
# run the prediction
aca_model.calc()
# extract the temperature prediction component
aca1 = aca_model.comp['aacccdpt']
times1 = aca1.times
temps1 = aca1.mvals
# plot the prediction
plt.figure(1, figsize=(6, 4))
plot_cxctime(times1, temps1, 'b')
# resize the axes for a little more room
x0, x1 = plt.xlim()
dx = (x1 - x0) * 0.03
plt.xlim(x0 - dx, x1 + dx)
# modify the tau paramter and rerun the propogation
aca_model.comp['heatsink__aca0'].tau = aca_model.comp['heatsink__aca0'].tau * 0.8
aca_model.calc()
# extract the temperature prediction component
aca2 = aca_model.comp['aacccdpt']
times2 = aca2.times
temps2 = aca2.mvals
# plot the prediction from the modified tau
plot_cxctime(times2, temps2, 'g')
# add titles and layout
plt.title('tau vs 0.8*tau', fontsize=12)
plt.ylabel('predicted temp')
plt.tight_layout()
# save plot to file named after the xija version in use
plt.savefig(f'{xija.__version__}_plot.png')

[taldcroft]

@jskrist - thanks for the detailed testing! This threw me for a loop for a bit since the new test case I included definitely fails without the patch.

It turns out that the heatsink component uses direct attribute access in the call to update() which is used to update based on current parameters. So in order to get tau it does self.tau. The intended design is that tau is not a direct object attribute but rather gets found and maintained in the pars_dict attribute. However, if you set that object attribute directly as in your test script with the buggy version of xija, it does not get put into pars_dict and is just an normal object attribute.

In contrast the test case I used is the solar heat component, and that accesses parameter values with self.parvals, which maps into pars_dict. So setting self.P60 = 5 was updating that direct object attribute but not self.parvals, hence the problem.

So I learned even a little bit more and feel even better that this is the right change.