Vertical post-scan identification of readout CRs

[JayVB]

We should limit the search for readout CRs to the actual data pixels. It often happens that a bright object near the top of the detector fills a lot of traps then when the trails are subtracted in the post-scan (which starts with zero electrons), there is enough of an excess to identify the bright object in the image as a readout CR, and to lower the CTE correction for the object. This can be traced to two issues. First is that each column has a different number of traps the model doesn't always accurately account for that. The second is that the zero-background level of the post flash is a very difficult environment for the correction to work in... expecting our model of the CTE tails to work perfect there is asking a lot... so a little over-subtraction shouldn't be surprising. Solution: just probe the first 2048 pixels for trail-oversubtraction.

[pllim]

Are we still talking about PCTECORR or are you referring to ACSREJ? And does this only apply for exposures with post-flash? Please clarify.

[pllim]

Also, tagging both "bug" and "enhancement" is confusing to me. This sounds like it would improve something, so I am removing the "bug" tag.

[JayVB]

I think it's more of a bug than an enhancement; the change should be made for both UVIS and for ACS when the time comes to update the code. It should be a pretty easy change.. instead of searching for readout CRs from j=1 to j=2070 (or j=2068), we will just search for them between j=1 and j=2051 (for UVIS) and j=1 and j=2048 (for ACS). I think the code will actually end up working just on cropped raw images, so this may be a moot point.

[pllim]

@JayVB , are you talking about the PCTECORR step or the ACSREJ step? It may not be as easy as it sounds. If you are talking about ACSREJ, it does not hardcode the value for fullframe, as it also needs to work on some arbitrary subarray. The ACSREJ algorithm also uses some kind of "scrolling buffer" (presumably to avoid having multiple copies of fullframe arrays to save memory), so if ACSREJ is what you are referring to, it will need some thinking. Also, the algorithm should not be changed until #2 is merged (or rejected) to avoid merge conflict.

[JayVB]

I don't know the exact names of the steps in the pipeline, but this is pretty deep within the PCECORR algorithm. Jamie asked me to open the issue as a bug that is present at least in my source code to make sure that it is properly dealt with in the formal implementation.

[pllim]

Okay, maybe Jamie can clarify when he gets the chance, but sounds like this is PCTECORR afterall, and not ACSREJ.

[jamienoss]

@pllim this is PCTECORR and is only currently an issue with wf3cte.

This is the section of code in question, iterating over RAZ_ROWS, set to 2070, and thus includes more than the image.

 /*LOOK FOR AND DOWNSCALE THE CTE MODEL IF WE FIND
    THE TELL-TALE SIGN OF READOUT CRS BEING OVERSUBTRACTED;
    IF WE FIND ANY THEN GO BACK UP AND RERUN THIS COLUMN
    THE WFC3 UVIS MODEL SEARCHES FOR OVERSUBTRACTED TRAILS.
    WHICH ARE  DEFINED AS EITHER:
    - A SINGLE PIXEL VALUE BELOW -10E-
    - TWO CONSECUTIVE PIXELS TOTALING -12 E-
    - THREE TOTALLING -15 E-
    WHEN WE DETECT SUCH AN OVER-SUBTRACTED TAIL, WE ITERATIVELY REDUCE
    THE LOCAL CTE SCALING BY 25% UNTIL THE TRAIL IS
    NO LONGER NEGATIVE  THIS DOES NOT IDENTIFY ALL READOUT-CRS, BUT IT DOES
    DEAL WITH MANY OF THEM. FOR IMAGES THAT HAVE BACKGROUND GREATER THAN 10 OR SO,
    THIS WILL STILL END UP OVERSUBTRACTING CRS A BIT, SINCE WE ALLOW
    THEIR TRAILS TO BE SUBTRACTED DOWN TO -10 RATHER THAN 0.
*/
if (cte->fix_rocr) {
    for (j=10; j< RAZ_ROWS-2; j++){
        if (  (( cte->thresh > pix_modl[j] ) &&
                            ( cte->thresh > (pix_modl[j] - pix_obsd[j]))) ||

                             (((pix_modl[j] + pix_modl[j+1]) < -12.) &&
                             (pix_modl[j] + pix_modl[j+1] - pix_obsd[j] - pix_obsd[j+1] < -12.)) ||

                             (((pix_modl[j] + pix_modl[j+1] + pix_modl[j+2]) < -15.) &&
                             ((pix_modl[j] + pix_modl[j+1] + pix_modl[j+2] -pix_obsd[j] -
                             pix_obsd[j+1] - pix_obsd[j+2]) <-15.))  ){

            jmax=j;

            /*GO DOWNSTREAM AND LOOK FOR THE OFFENDING CR*/
            for (jj=j-10; jj<=j;jj++){
                if ( (pix_modl[jj] - pix_obsd[jj]) >
                           (pix_modl[jmax] - pix_obsd[jmax]) ) {
                    jmax=jj;
                 }
             }
             /* DOWNGRADE THE CR'S SCALING AND ALSO FOR THOSE
                 BETWEEN THE OVERSUBTRACTED PIXEL AND IT*/
             for (jj=jmax; jj<=j;jj++){
                 Pix(pixz_fff.sci.data,i,jj) *= 0.75;
             }
             REDO=1; /*TRUE*/
         } /*end if*/
     } /*end for  j*/
}/*end fix cr*/

Now, when this was written, the design (from what I know) was to not correctly loop over the number of pixels in just the image frame, whether that be a sub (excluding rest of image) or full (excluding post) but to iterate over the full amount and take account for this by using a mask in an attempt to ignore the non-image pixels (it was supposedly easier/better than writing in the different and correct iterator boundaries in the loops, taking into account the different image sizes). This happens here:

/*HORIZONTAL PRE/POST SCAN POPULATION */
for (j=0; j< RAZ_ROWS; j++){
    if(Pix(rz.dq.data,i,j)){
        pix_obsd[j] = Pix(rz.sci.data,i,j); /*starts as input RAZ*/
        ...
    }
}

Here rz.dq.data is the said mask. However, this may not be effective/complete due to a subsequent assignment of pix_modl to rz.sci.data without filtering against the mask here:

for (j=0; j<RAZ_ROWS; j++){
    pix_modl[j] =  Pix(rz.sci.data,i,j);
    ...
}

This latter issue is something that I was already aware of, and needing to open an issue to clarify/log.

[jamienoss]

@pllim Not being 100% confident of this latter point is why I labeled this issue as both a 'bug' and an 'enhancement'.

[pllim]

Thanks for the clarifications! I was worried that the "CR readout" might conflict with ACSREJ work but it appears to be a separate issue, so I am going to slowly back away now.

[sosey]

The full-frame code was adapted for subarray use by placing and rotating the subarray into the correct full frame exposure position before assignment to raz and entering of the cte loops. From my understanding, the correction is dependent on the original pixel location in the full frame image. Only the area from the subarray specified is stored in the onboard buffer, the rest of the array is readout, but discarded. Since the full frame correction was done using full frame data I think it's appropriate to treat the subarrays as such until it's decided that a different correction algorithm applies to them, I think jay has an in-progress program to measure any differences in the cte trailing for subarray vs full-frame readouts. Additionally, putting the subarray into the correct full frame position accounts for the group's user specified subarrays, often used for team calibration proposals which are allowed to cross quadrant boundries. Some of those still have prescan pixels, and only the prescan pixels are used for the additional cte bias subtraction for subarrays, whereas the postscan pixels are used for the full frame images. Putting the subarray into the correct location in the full frame also allows for the correct subtraction of the cte bias reference image as well, neglecting quandrant-crossing subarray locations which don't have prescan pixels from the amp. So as long as the algorithm is changed to correctly add in the original pixel location as the looping parameters throughout the code, pull the correct pixel from the cte table and the flux correction, and take into account quandrant-crossing subarrays for the cte bias subtraction, then changing to a model which removes the masking and full frame insertion is fine.

Check the logic, but the for-loop you highlighted only gets entered for columns which contain flux, the whole cte correction only gets performed for columns which contain the subarray. After the highlighted loop point, I believe you want to track the correct trap correction regardless of whether the science pixel has a zero flux value or not. In fact, the same thing would occur for zero-valued science pixels in a full frame as for subarrays. Most of the logic in simcol is calculating the correction based on reference files and the absolute location of the original pixel in the full frame. When we pop out of simcol we go on with the correction and take into account the subarray mask when updating locations for the final corrected images. I validated this logic using full frame images which were then cropped to a subarray and passing both through the routine to make sure the same correction was returned. You should consider doing the same for validating this updated instrument combined code. @JayVB should comment here if I've misinterpreted the logic here for the correct application of the correction in sim_colreadout.

[jamienoss]

@sosey regarding your last paragraph. You're right that the above-described loop is only entered if the column is determined to have flux in it, and that this is computed using the mask, however, this is for the entire column. Doesn't this mean that the overscan pixels (or any pixels in a subarray column before and after the subarray boundary) will still be accounted for when they might not want to be? It looks like it's the combination of the mask and whether the column has flux that is currently acting as the real mask and that it may not cover all of the necessary logic space.

E.g. any column that doesn't have a pixel that is within the subarray is correctly ignored, however, the column that is a part of the subarray will be determined to have flux in it. The entire cte correction will then be computed on all pixels in this column because it loops over the entire column (nRows) and that the temporary columns used for this computation are initialized with the unmasked data. These pixels aren't outputted to the final data array as the mask is used for this process, however, it seems possible that these pixels still imprint on the others when, as @JayVB mentioned, this may not be desired. If the temp columns used the masked data, then there might not be a problem (I think?).

[sosey]

The column of image pixel data going into this section has zero everywhere except where the subarray exists. When I look at the logic inside this this part of the code and in simcol, the zero flux is inconsequential to the result. Look through the logic carefully yourself and see if I've missed something.

[mdlpstsci]

The section of code in question in this issue is in the routine inverse_cte_blur() which is obsolete in the new/updated version of the CTE code released as CALWF3 V3.6.0 (April 2021). The algorithm author (J. Anderson) significantly updated the WFC3 CTE algorithm which experienced extensive testing.

An updated ACS CTE algorithm was implemented as CALACS V9.2.0 (June 2017). This code should be checked as this issue may be obsolete.

[mdlpstsci]

This issue is obsolete for WFC3. Re-reading and seeing this was just an issue for WFC3.

[mdlpstsci]

Closing as this is obsolete since the implementation of the updated WFC3 CTE code.