Fix issues related to large p_acq change for small T_ccd change

[taldcroft]

Description

As discussed here and here, proseco can occasionally end up generating very different p_acq values for the same star (diff by up to 0.3) when the acquisition T_ccd changes by a tiny amount (< 0.02 C).

The operational impact is that a schedule which passes sparkles during load development can fail final review due to temperature propagation changes. This has happened twice.

The mechanism we identified is the temperature-dependent transformation from yag/zag to row/col. If the row/col of an acq candidate crosses a row/col boundary then box comparisons to imposter locations (fixed in row/col by definition) can change. There are two manifestations we found:

1. An imposter discretely goes in or out of the effective search box, changing the probability of the acq star being the brightest substantially. For a bright imposter this can change p_acq from 1 to 0 or vice versa.
2. The row/col of the 2x2 binning used for imposter detection crosses a pixel boundary, thereby changing the binning and brightness of imposters.

This PR fixes both issues as follows:

1. If a spoiler or imposter star is within 4 pixels of the effective search box edge, add a positive magnitude offset to reflect the fraction of the star / imposter light that is actually within the box. This offset is empirically determined from the ACA star PSF.
2. Require the CCD region box used for imposter detection to have even row/col boundaries. This ensures consistency of sampling even if T_ccd changes.

Interface impacts

Testing

Unit tests

• No unit tests
• Mac
• Linux
• Windows

Independent check of unit tests by [REVIEWER NAME]

• [PLATFORM]:

Functional tests

No functional testing.

[jeanconn]

For this one, I ran a year of approved-schedule-pickles with the current release and this new code and the P2 distribution is just about what I would expect - tiny changes with some better and some worse.

[jeanconn]

This is a lightweight new function but seems like it should have some unit tests someplace in this PR.

[taldcroft]

This is not really a public function and it is just 3 lines of very straightforward code. If these lines had been inlined in the original function there wouldn't be a question of unit testing.

[taldcroft]

What would be important for unit/functional tests is verification that the patch actually fixes items (1) and (2). I believe that (2) is done from the flight catalog in question, so it might be good to adapt this to a unit test. Verifying (1) is trickier and I need to think about it.

[jeanconn]

For (1) I was thinking about just explicitly testing with synthetic dark map(s).

[taldcroft]

Related to the in-person discussion yesterday with @jeanconn, I found the source of yet-another potential discontinuity here. Basically there will be a T_ccd value at which a new search hit appears (the 2x2 binned value exceeds the threshold). This adds new values to rows, cols, and vals. Then the r2x2 and c2x2 centroids change, potentially enough to change the pixel boundary and substantially change the counts/mag of the imposter.

With the current algorithm relying on 2x2 search hits and PEA-ish behavior, I don't see an easy way around this.

    for idx in range(n_hits):
        # Get row and col index vals for each merged region of search hits
        rows, cols = np.where(dc_labeled == idx + 1)
        vals = dc2x2[rows, cols]

        # Centroid row, col in 2x2 binned coords.  Since we are using edge-based
        # coordinates, we need to at 0.5 pixels to coords for FM centroid calc.
        # A single pixel at coord (0, 0) has FM centroid (0.5, 0.5).
        rows = rows + 0.5
        cols = cols + 0.5
        vals_sum = np.sum(vals)
        r2x2 = np.sum(rows * vals) / vals_sum
        c2x2 = np.sum(cols * vals) / vals_sum

        # Integer centroid row/col (center of readout image 8x8 box)
        c_row = int(np.round(box_r0 + 2 * r2x2))
        c_col = int(np.round(box_c0 + 2 * c2x2))

[jeanconn]

Yes, I was worried that search hits could change both based on the dark current scaling and on the way that maxmag may be modified with t_ccd.

You've made the effect of the hits more smooth by the new interpolation in position space - it does seem like the remaining piece is to figure out how to make the "hits" themselves just a bit more stable with changing t_ccd. Or maybe the complexity for that just isn't worth it. Hmph.

[taldcroft]

@jeanconn - somehow we both managed to miss a crucial point that would seem to invalidate the premise of this PR: Within proseco the yag/zag to row/col transforms alway use the same default ACA temperature of +20 C.

That could mean that the issue is mostly (or entirely) related to new search hits crossing the threshold and bumping the imposter centroid around. This seems less easy to fix.

[jeanconn]

Ah, well I think that's a subtlety that would have come up in testing eventually. Though I suppose it means that this fix had to happen for me to understand what is going on as I was convinced it was the plate scale moving the imposter instead of the shape of the imposter changing. Also it seems like there's no driver to have a fixed t_aca in proseco for these conversions except the issue that chandra_aca isn't supporting the transforms we want sot/chandra_aca#152 ? Though I suppose then we only need this PR if we eventually extend to actually using the differing plate scale by temperature.