Add support for dynamic limits via effective t_ccd

[taldcroft]

This adds two top-level attributes t_ccd_eff_acq and t_ccd_eff_guide to capture the effective T_ccd that gets used in downstream processing. It leaves t_ccd_acq and t_ccd_guide as the user-supplied values.

In the acqs and guides and fids objects, t_ccd_acq/guide is always the effective temperature. The idea is that those "low-level" objects don't need to fuss about this detail of how we implement dynamic limits. These objects just do their job of selecting stars for a given CCD temperature. This is all illustrated in the tests.

This also adds a version attribute to ACATable.
Closes #257

[taldcroft]

@jskrist - FYI see this PR. It adds two top-level attributes. John Scott stated that he was not especially interested in seeing these in ORviewer reporting, but they are there. I will include the effective temperature in the sparkles reports, and presumably @jeanconn will include them in starcheck reporting.

[taldcroft]

With apologies, I tossed in the entirely-unrelated 0bf3901, because it falls in perfectly to the new set_attrs_from_kwargs method. No unit test, but it works:

In [2]: aca = get_aca_catalog(20603, t_ccd=-8.5)

In [3]: aca.version
Out[3]: '4.4-r535-6b4125c'

[jeanconn]

Still not quite working the way I expect

In [3]: cat = proseco.get_aca_catalog(att=Quat((10, 20, 30)), dither_acq=(8,8), dither_guide=(8,8),
   ...: focus_offset=0, sim_offset=0, man_angle=90, t_ccd_acq=-7.5, t_ccd_guide=-5, date='2018:001',
   ...: raise_exc=True)

    194     def t_ccd(self):
    195         if self.t_ccd_acq != self.t_ccd_guide:
--> 196             raise ValueError('t_ccd attribute is ambiguous: acq and guide t_ccd are different')
    197         return self.t_ccd_guide
    198 

ValueError: t_ccd attribute is ambiguous: acq and guide t_ccd are different

[taldcroft]

Yck, I'll investigate. (BTW, more of the traceback would help in the future).

[jeanconn]

Gotcha. I'll plan to come back around to this with more (and better debug info) if the issue isn't reproducible for you or if that was just a bad plug-in-the-args try on my part. (I don't think there was anything else screwed up in my workspace at the time).

[taldcroft]

This is interesting because the new code reveals an ambiguity that was swept under the rug previously, namely which t_ccd to use for scaling the dark current. Before this code was just using t_ccd_guide for t_ccd. I guess that by definition t_ccd_guide > t_ccd_acq, so using the guide value is conservative.

~/git/proseco/proseco/core.py in set_attrs_from_kwargs(self, **kwargs)
    612         if self.dark is None:
    613             from mica.archive.aca_dark import get_dark_cal_image
--> 614             self.log(f'Getting dark cal image at date={self.date} t_ccd={self.t_ccd:.1f}')
    615             self.dark = get_dark_cal_image(date=self.date, select='before',
    616                                            t_ccd_ref=self.t_ccd, aca_image=True)

So perhaps just reverting to the old behavior is right.

[jeanconn]

Ah. I was wondering about that (with the attribute structure I couldn't remember if there were two copies of the dark map or not). As you saw from my first broken review, I was trying to confirm that at least something reasonable was done with the dark map.

Sounds like at some point we should figure out how do separate out the dark map scaling so that it is easier to have the "right" scaled dark map for acq and guide temperatures, but that the more conservative one should be OK for now.

[taldcroft]

The other mystery being why my tests were passing.

[taldcroft]

Duh, because I passed in the dark current explicitly.

[jeanconn]

I think maybe your tests were passing because you were passing t_ccd from STD_INFO and t_ccd_acq and t_ccd_guide?

[jeanconn]

Ah. That makes even more sense. And is somewhat related to my comment on PR #274.

[taldcroft]

Your exact example didn't work for other reasons, but this now runs:

In [3]: cat = proseco.get_aca_catalog(att=Quat((10, 20, 30)), dither_acq=(8,8), dither_guide=(8,8),
   ...: focus_offset=0, sim_offset=0, man_angle=90, t_ccd_acq=-7.5, t_ccd_guide=-5, date='2018:001',
   ...: raise_exc=True, detector='ACIS-S', n_guide=5, n_acq=8)

[jeanconn]

Great! And "Your exact example didn't work for other reasons," yeah, when poking around at console generally I just keep adding keywords until the warnings go away...

[jeanconn]

So I'm a little confused about how this ended up? I agree that if there can be only one temperature for the dark current that it should be the warmer one and the max guide temperature should be warmer than the acq temperature... but where is the code that sets t_ccd to be t_ccd_guide (I must be missing it)?

And the t_ccd_eff_acq seems a bit ambiguous if it ends up being the "checking" temperature for the catalog but the imposters are made with the dark current from the t_ccd_eff_guide.

But not a show-stopper. Otherwise I have not been able to make this code break since fixes.

[taldcroft]

@jeanconn - look here and at the tests below Hopefully at least the API now makes sense. The implementation is not that tricky either but there are some obfuscations that must be traversed...