Make ACA penalty limit optional

[taldcroft]

Description

Make the ACA penalty limit from the chandra_models aca_spec.json file be optional. This also makes the ACA planning limit optional for proseco, though it is still needed in other tools.

If not provided there is no penalty limit and the effective CCD temperature is the same as the actual (predicted) temperature. In this case the t_ccd_penalty_limit attribute of an ACA catalog is None.

This PR also transitions to using ska_helpers.chandra_models to get the chandra_models data. This is helpful to enable testing using the CHANDRA_MODELS_REPO_DIR and CHANDRA_MODELS_DEFAULT_VERSION env vars.

Interface impacts

CHANDRA_MODELS_REPO_DIR and CHANDRA_MODELS_DEFAULT_VERSION env vars now work with proseco for selection of the aca thermal model for limits. Those vars already worked to define the acquisition model used within proseco via chandra_aca.

Testing

Unit tests

• Mac

Independent check of unit tests by Jean

• Linux

Functional tests

chandra_models update

Starting from chandra_models version 3.49, I made a new branch no-penalty-limit and edited chandra_models/xija/aca/aca_spec.json to remove the penalty limit.

➜  chandra_models git:(no-penalty-limit) git diff 3.49                         
diff --git a/chandra_models/xija/aca/aca_spec.json b/chandra_models/xija/aca/aca_spec.json
index 87665ca..7912ffe 100644
--- a/chandra_models/xija/aca/aca_spec.json
+++ b/chandra_models/xija/aca/aca_spec.json
@@ -273,7 +273,6 @@
     },
     "limits": {
         "aacccdpt": {
-            "planning.penalty.high": -4.0,
             "planning.warning.high": -4.5,
             "unit": "degC"
         }

Simple test

$ export CHANDRA_MODELS_REPO_DIR=/Users/aldcroft/tmp/chandra_models
$ export CHANDRA_MODELS_DEFAULT_VERSION=no-penalty-limit    
$ ipython
In [1]: from proseco.tests.test_common import mod_std_info
In [2]: from proseco import get_aca_catalog
In [3]: aca = get_aca_catalog(**mod_std_info(t_ccd=-2.0))
In [4]: aca.t_ccd_acq
Out[4]: -2.0

In [5]: aca.t_ccd_eff_acq
Out[5]: -2.0

In [6]: print(aca.t_ccd_penalty_limit)
None

In [7]: import proseco.characteristics as ACA

In [8]: ACA.chandra_models_version
Out[8]: 'no-penalty-limit'

In [9]: ACA.aca_t_ccd_planning_limit
Out[9]: -4.5

In [10]: print(ACA.aca_t_ccd_penalty_limit)
None

[jeanconn]

I'd assumed the plan was to update get_xija_model_spec file to use chandra_models.get_data to have a unified approach via that strategy.

[taldcroft]

Yes, but that is out of scope for this PR. Also, using the direct interface in ska_helpers makes more sense here since there is no real use of xija itself.

[jeanconn]

Yes -- though for me I was bringing it up because starcheck was getting the ACA model version and the limits from proseco here via characteristics.py, but getting the aca model spec with get_xija_model_spec. That leads to a disconnect in that application due to the interface change here (in scope). It probably makes sense for starcheck to either join proseco in using ska_helpers.chandra_models.get_data to get the aca model or to stop using proseco.characteristics as a convenient source of limit reading. Or update get_xija_model_spec to use ska_helpers.chandra_models.get_data.

[jskrist]

I should mention that get_xija_model_spec() does not work on FOT Windows machines. it returns an error about not being able to read from remote repository when executing the line repo = git.Repo.clone_from(repo_path, repo_path_local). I have confirmed that this occurs from the python environment directly, and is not related to the pyexec interface. I think we don't run into this issue now only because MATLAB provides all the necessary inputs, so this code doesn't run when called from MATLAB. Except it did in my testing of removing this parameter (before I started using this PR).

[jeanconn]

Though James I was working under the impression that part of the problem with get_xija_model_spec was that right now the windows environment had a broken git .

[jskrist]

That could be the root of the issue. I'm not sure exactly what or how the git is broken in the python environment.

[jeanconn]

We had previously seen what we guessed was a conda bug making a broken git. It was a non-issue because the windows environment wasn't using git for anything but will hope it is fixed upstream or will work around it for the next testing release (if that is coming into play).

[jeanconn]

I removed the WIP and am marking this approved.