Add eclipse support to the SimZDepSolarHeat class and fix crashes in xija_gui_fit

[jzuhone]

Description

The SimZDepSolarHeat class is used to model pitch-based solar heating for the ACIS FP, ACIS PSMC, and HRC CEA thermal models. It has a separate set of pitch bins for a specified set of SIM-Z ranges.

Unlike the other pitch-based solar heating classes, it does not have support for eclipses. This PR adds that support by adding the appropriate code to the dvals property and eclipse_comp=None to the __init__ method.

In order for the above models to use this functionality, they will have to include the eclipse component in the SimZDepSolarHeat setup. Until that is done, the models should operate in the same way without any changes, since eclipse_comp will be None by default.

While I was carrying out this work, I was confronted again with the fact that xija_gui_fit sometimes tends to crash with the following message when working with plots and trying to zoom in, pan, or otherwise manipulate them:

RuntimeError: wrapped C/C++ object of type FigureCanvasQTAgg has been deleted

I made some small edits to try to reduce the frequency of these crashes, but I determined that preventing them entirely is very difficult.

Interface impacts

None on the existing thermal models until they implement eclipses. Hopefully xija_gui_fit will crash less.

Testing

Unit tests

• No unit tests
• Mac
• Linux
• Windows

Independent check of unit tests by [REVIEWER NAME]

• [PLATFORM]:

Functional tests

• Loaded the ACIS FP, ACIS PSMC, and HRC CEA models in xija_gui_fit, models loaded OK without having eclipse component defined.
• Ensured that model outputs did not change with the addition of the option for the eclipse component without actually enabled it.
• Enabled the eclipse component for each model and verified that the temperature changed as a result. This was a bit harder to interpret. In the PSMC model case, the temperature temporarily decreased after an eclipse as expected. But for the CEA model it increased, and for the ACIS FP model it depended on the details of the radzone passage the eclipse was in. I believe that what is happening here is that the existing model fits have adapted to the telemetry in the absence of the inclusion of eclipses in slightly different ways in order to find a bit fit (waves hands wildly).

On the xija_gui_fit side, I opened the HRC CEA model, opened many plots, zoomed in and out on many plots, panned plots, and closed and re-opened plots, all without crashing (but I don't think the problem is entirely solved).

[jeanconn]

Sorry for the delay on review on this one @jzuhone . Quick question on the eclipse support - should SimZDepSolarHeat just extend SolarHeat? I didn't look carefully to see if there are reasons it cannot.

[jzuhone]

@jeanconn it cannot in this case--the dvals method for both of these classes is very different and that's where the eclipse masking gets applied. In theory we could refactor SimZDepSolarHeat to inherit from SolarHeat as it currently doesn't, but that's a separate PR.

[taldcroft]

I believe that what is happening here is that the existing model fits have adapted to the telemetry in the absence of the inclusion of eclipses in slightly different ways in order to find a bit fit (waves hands wildly).

That is quite plausible. For these models it is entirely possible that a solar heat input is actually negative, in which case the eclipse will cause an increase in the heat input. Basically without eclipses there is a degeneracy in the solar heat input and the heat sink. The eclipse breaks that degeneracy by uniquely defining "zero" solar heat input. Here the hands are waving only a bit. 😄

This looks fine then (noting that I did not review the GUI fit changes), but please do one quick sanity check and confirm that you can get the eclipse data__time plot and it looks reasonable.

[taldcroft]

Hmm... I did a quick 1-param fit of the CEA model with one plot open (the CEA temp vs. time plot) as a sanity check and it crashed in a way that seems related to the PR changes:

*********************************
  FIT FINISHED
*********************************

Traceback (most recent call last):
  File "/Users/aldcroft/miniconda3-arm/envs/ska3/lib/python3.12/site-packages/matplotlib/backends/backend_qt.py", line 506, in _draw_idle
    self.draw()
  File "/Users/aldcroft/miniconda3-arm/envs/ska3/lib/python3.12/site-packages/matplotlib/backends/backend_agg.py", line 385, in draw
    super().draw()
  File "/Users/aldcroft/miniconda3-arm/envs/ska3/lib/python3.12/site-packages/matplotlib/backends/backend_qt.py", line 475, in draw
    self.update()
RuntimeError: wrapped C/C++ object of type FigureCanvasQTAgg has been deleted

I've never seen an error like wrapped C/C++ object before, and doing the fit in current master worked fine for me.

[taldcroft]

@jzuhone - maybe for this PR it would be best to drop the xija_gui_fit changes and tackle that in a separate PR.

[jzuhone]

@taldcroft this error you saw is the already existing error that I cannot seem to get rid of, and should appear also on the master branch. It seems to be worse on the CEA model for some reason, but it also seems to go pretty deep.

[taldcroft]

I rebuilt the compiled bits and tried again. This time I didn't get any crashes after a few basic tests. Not sure if rebuilding had anything to do with it, but at least it seems basically functional for me.