Update how the HRC 24v state is set

[matthewdahmer]

Description

This update how the HRC 24v state is set. With a pending update to kadi, the 24v state will be available, allowing acts_thermal_check to properly set this state to generate an accurate thermal prediction.

Interface impacts

No interface impacts

Testing

Unit tests

• No unit tests
• Mac
• Linux
• Windows

Independent check of unit tests by JC

• Linux on kady (I just ran the cea tests, with ska3-prime release candidate and with HRC state updates for 24V, 15V (from SCS-134), HRC_I and HRC_S sot/kadi#281 in PYTHONPATH)

Functional tests

For functional testing - Dan ran the HRC test weeks. Jean put some plots comparing those to the FOT runs in sot/kadi#281.

[taldcroft]

The 24V input is not needed for an accurate thermal prediction. I confirmed this by running xija_gui_fit cea --stop 2022:265 --days 5, which covers one pathfinder HRC observation. I then changed the 224pcast_off__P parameter from -10 to +10 and observed no change in model prediction output. Changing 215pcast_off__P by even a little made a huge change as expected.

That 24V dependence should not have been in the HRC model since is basically impossible to calibrate:

• Pre-HRC-B anomaly it was always on, so it is degenerate with the 15V or 5V.
• Post-HRC-B anomaly it is only on for about 90 seconds to swap instruments. This is so short that it does not have any measurable thermal impact. Remember the default time step in xija is 5 minutes, so it isn't obvious this is even getting seen at all. In my test case I suspect it fell within the 5 minute sampling bin.

Formally speaking this PR looks correct, and indeed the current code looks incorrect since it brings in a spurious heating component for the whole HRC observation. Another option that will be equivalent and removes the kadi update dependence is just to set the 24V state to "OFF" always.

[matthewdahmer]

@taldcroft as I noted in sot/kadi#281 I agree that, in reality, the 24v is not on long enough to affect temperature. That being said, if it is left on by mistake it definitely would affect temperature. Thinking out loud, I wonder if we should still keep this in the model in future model fits just to give us an alternative way of catching a case where the 24v was not turned off as expected.

[taldcroft]

@matthewdahmer - the 24v ON/OFF commanding is done by the HRCDET ATS, so not getting turned off seems pretty unlikely. Nevertheless there should be a direct load review check of the 24V commanding timing.

Right now the model does depend on the 24v status in a way that might turn up an error in load commanding, but we wouldn't want to rely on that. The 224past_off__P is degenerate with 2imst_on__P and 2spst_on__P. I just did an experiment of freezing 224past_off_P at -5.0 and fitting those two for 50 days pre-anomaly. The other two just came back to the same values modulo the 5.0 offset. So basically that parameter is meaningless and we shouldn't rely on it in any way.

[matthewdahmer]

@taldcroft Sounds good, we can remove the 24v element of the model during the next hrc model update, unless you think we should do it sooner?

[taldcroft]

@matthewdahmer @jzuhone - this update looks fine to me with the appropriate testing.

I think what is needed is to run this on at least one (and better yet all) the HRC test loads in a Ska3 environment with the kadi from sot/kadi#281 in the path. Then verify that the thermal model predictions from cea_check match reasonably well the predictions from MCC. In particular the temperatures during the HRC observations should be pretty close. Maybe this needs to be a team effort with @matthewdahmer doing the MCC side and @jzuhone or @drxre doing the acis_thermal_check / cea_check side.

[jeanconn]

I think this also needs an update here (add 'hrc_24v' to the list)

acis_thermal_check/acis_thermal_check/state_builder.py

Line 137 in 63b7f64

self.state_keys += ["hrc_15v", "hrc_i", "hrc_s"]

to get the 24v state. And review and update of the test and regression outputs.

[jzuhone]

@jeanconn thanks for catching that I fixed it and pushed the change here (ping @matthewdahmer)

[jeanconn]

Thanks! With that change, cea_check runs for me but I noted the cea-related tests still weren't passing and I didn't dig in to see which were trivial (new hrc_24v column in test data?) and which required some knowledge of what was reasonable for updates to regression test outputs.

[matthewdahmer]

@jzuhone thank you for adding the hrc_24v to the state_keys list. I emailed links to the MCC-produced HRC thermal predictions for all versions of the test loads.

[jzuhone]

Once we are certain nothing will change here or on the kadi side in these PRs, I will update the answers as part of this PR.

[jzuhone]

Answers have been updated. I don't know if @jeanconn wants to verify that they pass for her, but otherwise I am good with this.

[jzuhone]

And we can ignore the ruff failures--I will handle those separately.