# Look into source of NaN/Infs values in calexp images

XMLWordPrintable

#### Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
5
• Sprint:
DRP S21a (Dec Jan)
• Team:
Data Release Production
• Urgent?:
No

#### Description

It seems some of our calexps coming out of single frame processing have non-finite values, including NaNs and infs. Neither are marked with the UNMASKEDNAN mask nor are they interpolated and replaced with finite values, as would be expected.

To get an idea of the frequency of this occurrence, considering just the COSMOS visits included in the HSC RC2 dataset:

 COSMOS_G nVisits: 17   COSMOS_R nVisits: 16 There are 2187 NaN pixels in HSC-R 23704 102 There are 949 NaN pixels in HSC-R 23706 102   COSMOS_I nVisits: 33 There are 5526 NaN pixels in HSC-I 30494 100   COSMOS_Z nVisits: 31 There are 4320 NaN pixels in HSC-Z 1166 100 There are 1899 NaN pixels in HSC-Z 1168 102 There are 1464 NaN pixels in HSC-Z 1176 102 There are 643 NaN pixels in HSC-Z 1192 102 There are 6 NaN pixels in HSC-Z 1194 101 There are 4892 NaN pixels in HSC-Z 17950 100   COSMOS_Y nVisits: 52 There are 689 NaN pixels in HSC-Y 11736 101 

So of (17 + 16 + 33 + 31 + 52)*103 = 15347 CCDs, 10 of them have NaN pixels (I wasn't actually looking for Inf when I ran the script, and it turns out they can creep in too, so there could be a handful more with non-finite values).

It is likely of note that they are all outer corner CCDs (all with odd nQuarter rotations...but I'm really hoping this is not a rotated CCD issue...)

#### Activity

Hide
Lauren MacArthur added a comment - - edited

For a bit more context, see discussion at:

But this is the gist, using visit HSC-I 30494 100 as an example:

The mess at top right-of-center has 5526 NaN and 128 inf pixels.

So, not only are non-finite values creeping in, they are not getting the (most unfortunately named) UNMASKEDNAN bit set. The only mask plane that includes the mess is NO_DATA.

I have looked at the calib images (bias, dark, flat), and they all look "fine", so this is either present at "raw", or introduced by some other method in ip_isr.

Show
Lauren MacArthur added a comment - - edited For a bit more context, see discussion at: https://lsstc.slack.com/archives/C4JQP6FRS/p1615848617018600?thread_ts=1615848582.018500&cid=C4JQP6FRS But this is the gist, using visit HSC-I 30494 100 as an example: The mess at top right-of-center has 5526 NaN and 128 inf pixels. Here are the mask planes: So, not only are non-finite values creeping in, they are not getting the (most unfortunately named) UNMASKEDNAN bit set. The only mask plane that includes the mess is NO_DATA . I have looked at the calib images (bias, dark, flat), and they all look "fine", so this is either present at "raw", or introduced by some other method in ip_isr .
Hide
Lauren MacArthur added a comment -

Ok, I have isolated where this is coming in and it is not related to rotated CCDs (phew!).  The non-finite values are getting into the image during the brighter-fatter (BF) correction in isrTask.  Given that the CCDs affected all seemed to be the outer corner, and thus most-vignetted, CCDs, my suspicion was that it was related to a failure to skip over this region (and, indeed, the non-finite pixels all seemed to be located in a vignetted region).  In obs_subaru, the vignetted pixels are marked on the flats with the NO_DATA mask plane in the HSCFlatCombineTask in calib.py here, so I tried adding the "NO_DATA" to the list passed to the context manager in the BF application path here. (I opted not to add it to the maskToInterpolate config list as I am not sure we want to interpolate over the entire vignetted region). With this fix, the non-finite numbers no longer end up in the calibrated exposures. I have run this fix on three of the visits listed above and, not surprisingly, this change does come with some (very minor) differences around the edges. To demonstrate, here are some comparisons between runs pre and post this fix for the HSC-I visit 30494 (same as in plots above). This is scatter plot of a direct comparison between the trace radii between the two:

and they sky distribution of the above looks like:

DO NOTE THE SCALES! The very small differences all appear around the edges, as expected (there is one exception in ccd 66 which, as it turns out, has 24 pixels marked as NO_DATA!). Here are the individual trace distributions:
preFIx:

postFix:

I'd be hard pressed to say either one is clearly "better". To isolate to the regions where there are differences, I remade these plots only including the outer edge CCDs (as indicated by the filled in CCDs in the upper right camera panel...colour-coded by nQuarter in case anyone is curious!):
preFix:

postFix:

Again, I don't see any clear winner or loser and am inclined to say that this fix should be implemented (but see below for some further pondering about the vignette mask setting outside of HSC below).

The rest of the plots can be perused (for some finite amount of time) here for the postFix and direct comparisons to preFix and here for preFix only

Show
Hide
Lauren MacArthur added a comment - - edited

Ok, as for the vignetting mask setting, I think we're currently living in a strange and inconsistent state.  As noted above, obs_subaru assigns a NO_DATA plane to the vignetted region of the calibration FLAT (and thus gets propagated via mask OR-ing when the FLAT is applied).  On the other hand, the only other place I can find in the codebase where a similar masking is attached to the FLAT is in the CpFlatMeasureTask in cp_pipe, but this function uses the BAD mask plane for the vignette masking (also in the default function parameter setting here and, as a side note, no polygon is passed in, so unless the FLAT has a getValidPolygon() set, this will raise with a RuntimeError). I would think these mask names & definitions should be kept consistent. The HSC setting was deliberate, but I'm not sure if the Rubin/LSST pipelines want to follow that convention?

Now, in isrTask, there is a doVignette config said to "Apply vignetting parameters", but I am very puzzled by this option. It fires only after all the other ISR operations have fired (so none of them would know about any effect this operation has). If True, it calls the VignetteTask in ip_isr, but that task returns None unless the config parameter doWriteVignettePolygon is True, in which case it returns a polygon. In otherwords, if doWriteVignettePolygon is False, the call to this function is a no-op, so why make it at all? Also, the function gets an exposure passed in as a required parameter, but as far as I can tell, it is not used (nor updated) in the function. If doWriteVignettePolygon is indeed True, then the validPolygon is attached to the exposure (but it doesn't seem that any mask bits are set...but that might be as intended).

This all seems to be a bit of a mess as one can have a validPolygon representing the vignetted region attached to an exposure, but this does not imply a mask bit will be set and, even if a mask bit is set, it seems it will be a different one for at least obs_subaru (NO_DATA vs. BAD), so one really has to be careful with default "mask planes to skip/interpolate/whatever else" lists.

As usual, it's entirely possible I'm missing some very important points...so all this may be cleared up for me and other potential unexpected trawlers of this part of the codebase with a few doc updates?

Show
Hide
Lauren MacArthur added a comment - - edited

I really need some expert eyes on this one...and I'm afraid you may be the sole qualifier (although I know Merlin Fisher-Levine played a large role in getting the BF code implemented, so he may also be interested in weighing in here?)  Would you mind giving this a look?  Do let me know if you think this is the right way to go and, if so, if you would like me to do any more validations.

Jenkins + ci_hsc is happy.

Show
Lauren MacArthur added a comment - - edited I really need some expert eyes on this one...and I'm afraid you may be the sole qualifier (although I know Merlin Fisher-Levine  played a large role in getting the BF code implemented, so he may also be interested in weighing in here?)  Would you mind giving this a look?  Do let me know if you think this is the right way to go and, if so, if you would like me to do any more validations. Jenkins + ci_hsc is happy .
Hide
Christopher Waters added a comment -

I have no strong feelings about NO_DATA vs BAD in cp_pipe.  I likely didn't check what value it had elsewhere.  Can you please file a ticket on making this consistent?

The IsrTask doVignette is inherited from previous versions of ip_isr and/or obs_subaru.  I don't know if it's used downstream anywhere other than in the flat normalization step.  It may be that the original logic was:

2. Mask flat using attached polygon.
3. Use masked flat to ensure pixels outside of polygon are not used.

The usage of the polygon in cp_pipe is consistent with what I'd expect.  doVignette there defaults to False, so for cameras that are not vignetted (such as LATISS), this isn't run.  I agree that the usage in ip_isr is odd at best.  Perhaps we need another ticket to clean that up?

Show
Christopher Waters added a comment - I have no strong feelings about NO_DATA vs BAD in cp_pipe .  I likely didn't check what value it had elsewhere.  Can you please file a ticket on making this consistent? The IsrTask doVignette is inherited from previous versions of ip_isr and/or obs_subaru .  I don't know if it's used downstream anywhere other than in the flat normalization step.  It may be that the original logic was: Add polygon to exposure. Mask flat using attached polygon. Use masked flat to ensure pixels outside of polygon are not used. The usage of the polygon in cp_pipe is consistent with what I'd expect.  doVignette there defaults to False , so for cameras that are not vignetted (such as LATISS), this isn't run.  I agree that the usage in ip_isr is odd at best.  Perhaps we need another ticket to clean that up?
Hide
Lauren MacArthur added a comment -

Thanks for the comments, Chris.  I've created the two spin-off tickets (linked as "relates to" here) and have started progress on one of them (but will leave the other unassigned for now!)  I also updated the branch here in accordance with the comments on the PR and, indeed, I would appreciate you taking another peek to make sure all is still a-ok.  I reran the processing done above and my pipe_analysis scripts on the new branch and things look much the same.  I'll spare you a posting of numerous additional plots, but those posted above are now in this directory (i.e. plotsOld) and the latest run can be found here (so if you want to blink a given plot, you can flip the URL btw plots/ and plotsOld/).

Show
Lauren MacArthur added a comment - Thanks for the comments, Chris.  I've created the two spin-off tickets (linked as "relates to" here) and have started progress on one of them (but will leave the other unassigned for now!)  I also updated the branch here in accordance with the comments on the PR and, indeed, I would appreciate you taking another peek to make sure all is still a-ok.  I reran the processing done above and my pipe_analysis  scripts on the new branch and things look much the same.  I'll spare you a posting of numerous additional plots, but those posted above are now in this directory  (i.e. plotsOld) and the latest run can be found here  (so if you want to blink a given plot, you can flip the URL btw plots/ and plotsOld/).
Hide
Christopher Waters added a comment -

I looked over a subset of the plots, and couldn't find any significant differences.  I'm still happy with the changes here.

Show
Christopher Waters added a comment - I looked over a subset of the plots, and couldn't find any significant differences.  I'm still happy with the changes here.
Hide
Lauren MacArthur added a comment -

Thanks again, Chris. Jenkins on the final branch was again happy, so merged and done.

Show
Lauren MacArthur added a comment - Thanks again, Chris. Jenkins on the final branch was again happy , so merged and done.

#### People

Assignee:
Lauren MacArthur
Reporter:
Lauren MacArthur
Reviewers:
Christopher Waters
Watchers:
Christopher Waters, Jim Bosch, Lauren MacArthur, Merlin Fisher-Levine