# Include INTERP+CR pixels in coadds

XMLWordPrintable

## Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
1
• Team:
External

## Description

I think we agreed at the 2018-12-11 Monday Meeting that it's better to included interpolated CR pixels in the coadd (since the interpolation is pretty good) rather than leave them out and get lots more pixels masked with INEXACT_PSF.

Paul Price, if you're in a position to do this quickly before the next HSC release, please steal it. Otherwise I'll try to get to it later.

## Attachments

1. include-interp.png
392 kB
2. Screen Shot 2017-12-13 at 9.46.52 PM.png
250 kB

## Activity

Hide
Robert Lupton added a comment -

CR for sure as the interpolation is decent (it should be better!). I'm less convinced that we should include INTERP

Show
Robert Lupton added a comment - CR for sure as the interpolation is decent (it should be better!). I'm less convinced that we should include INTERP
Hide
Jim Bosch added a comment - - edited

The current configuration actually has:

 badMaskPlanes = ['BAD', 'EDGE', 'SAT', 'INTERP', 'NO_DATA'] 

so the reason we're not propagating the interpolated CR pixels is actually because they're also masked with INTERP.

I don't think our low-level code can currently handle an expression like "include pixels with INTERP only if they also have CR", so I think the appropriate thing to do here is to remove INTERP from the list and rely on the (existing) rejection of BAD, SAT, and NO_DATA to remove pixels for which we don't expect interpolation to be good.

Show
Jim Bosch added a comment - - edited The current configuration actually has: badMaskPlanes = ['BAD', 'EDGE', 'SAT', 'INTERP', 'NO_DATA'] so the reason we're not propagating the interpolated CR pixels is actually because they're also masked with INTERP. I don't think our low-level code can currently handle an expression like "include pixels with INTERP only if they also have CR", so I think the appropriate thing to do here is to remove INTERP from the list and rely on the (existing) rejection of BAD, SAT, and NO_DATA to remove pixels for which we don't expect interpolation to be good.
Hide
Robert Lupton added a comment -

That sounds reasonable, but we do need to revisit that code (was that part of Pim's work on Statistics?) at some point.

Show
Robert Lupton added a comment - That sounds reasonable, but we do need to revisit that code (was that part of Pim's work on Statistics ?) at some point.
Hide
Jim Bosch added a comment -

• CRs have essentially the same pixel values, but are masked differently as expected (now INTERP+CR; was INEXACT_PSF).
• Saturated/bleed pixels are being handled differently as well. I'm not sure what's going on here, but I suspect it's an interaction with the known no-good-pixels bug in WarpCompare.

Given the latter, I think I should hold merging and do another test once the fix for that is merged (though I'd be interested to hear Yusra AlSayyad's thoughts on that).

Show
Jim Bosch added a comment - Here's a shot of coadds before (left) and after (right) removing INTRP from the coaddition badMaskPlanes configuration. Two things jump out: CRs have essentially the same pixel values, but are masked differently as expected (now INTERP+CR; was INEXACT_PSF). Saturated/bleed pixels are being handled differently as well. I'm not sure what's going on here, but I suspect it's an interaction with the known no-good-pixels bug in WarpCompare. Given the latter, I think I should hold merging and do another test once the fix for that is merged (though I'd be interested to hear Yusra AlSayyad 's thoughts on that).
Hide

The no-good-pixels bug fix went in yesterday: see DM-12697.

WarpCompare does not run by default; these are SafeClip coadds. To use warpCompare you need to specify that in a config file. Instructions here: https://paper.dropbox.com/doc/CompareWarp-Evaluation-OCKPjQtffrEP6fkJPAt9A

Show
Yusra AlSayyad added a comment - - edited The no-good-pixels bug fix went in yesterday: see DM-12697 . WarpCompare does not run by default; these are SafeClip coadds. To use warpCompare you need to specify that in a config file. Instructions here: https://paper.dropbox.com/doc/CompareWarp-Evaluation-OCKPjQtffrEP6fkJPAt9A
Hide

Note that if you are making changes to obs_subaru/config. You now need to do that in 3 places instead of 2:

Show
Hide
Jim Bosch added a comment -

Thanks for the corrections and the tip on config/compareWarpAssembleCoadd.py - I had missed that one. I think I've found how this affects SafeClip's logic, and I think the change is harmless; it's just affecting which epochs contribute to areas that are are masked SAT on every epoch, and the resulting area on the coadd is always masked SAT.

I'll run a pair of coadds with and without these configs with warpCompare to make sure the same is true there.

Show
Jim Bosch added a comment - Thanks for the corrections and the tip on config/compareWarpAssembleCoadd.py - I had missed that one. I think I've found how this affects SafeClip's logic, and I think the change is harmless; it's just affecting which epochs contribute to areas that are are masked SAT on every epoch, and the resulting area on the coadd is always masked SAT. I'll run a pair of coadds with and without these configs with warpCompare to make sure the same is true there.
Hide

When testing WarpCompare, use master pipe_tasks/obs_subaru if possible

Show
Hide
Jim Bosch added a comment -

The WarpCompare results look quite similar to the SafeClip ones, and that made me think I had misdiagnosed the behavior of the new configs around bright stars.

I had - the problem is that we have some pixels masked with INTRP in the single-epoch images with no additional bit set to indicate why they were interpolated. I think I'd call that a bug, and I've opened DM-12985 to keep it separate from the coadd configuration.

Because the config change proposed by this ticket does make the bright stars on the coadd look a bit worse (because of those INTRP-only pixels), I'm inclined to block this merge on DM-12985. If that takes too long to sort out, however, we may want to press forward with this one on the principle that avoiding overzealous INEXACT_PSF masking outweighs any damage done to saturated stars.

Show
Jim Bosch added a comment - The WarpCompare results look quite similar to the SafeClip ones, and that made me think I had misdiagnosed the behavior of the new configs around bright stars. I had - the problem is that we have some pixels masked with INTRP in the single-epoch images with no additional bit set to indicate why they were interpolated. I think I'd call that a bug, and I've opened DM-12985 to keep it separate from the coadd configuration. Because the config change proposed by this ticket does make the bright stars on the coadd look a bit worse (because of those INTRP-only pixels), I'm inclined to block this merge on DM-12985 . If that takes too long to sort out, however, we may want to press forward with this one on the principle that avoiding overzealous INEXACT_PSF masking outweighs any damage done to saturated stars.
Hide
Jim Bosch added a comment - - edited

Yusra AlSayyad, since you've already looked at most of this config-only change, could you do the formal review?

I think the most important question is whether there is anywhere else I need to modify these configuration options.

Note that final merge of this is blocked on DM-12985, but that shouldn't actually require any changes to this ticket.

Show
Jim Bosch added a comment - - edited Yusra AlSayyad , since you've already looked at most of this config-only change, could you do the formal review? I think the most important question is whether there is anywhere else I need to modify these configuration options. Note that final merge of this is blocked on DM-12985 , but that shouldn't actually require any changes to this ticket.
Hide

Looks good.

1) No other obs_packages have overrides, but double check that the default badMaskPlanes for all cameras is desirable:

2) A search for INTRP in github won't turn up your commit message spelled INTERP. The commit message is clear regardless.

Show
Hide

Here's an N_Image (visits contributing to each pixel for a few configs left to right:

1) PRE DM-12968 WarpCompare HSC /datasets/hsc/repo/rerun/private/yusra/psfMatching/DM-12697/deepCoadd/HSC-G/9813/5,8_nImage.fits
2) POST DM-12968 WarpCompare HSC /datasets/hsc/repo/rerun/private/yusra/psfMatching/w_2017_48/warpCompare_DM-12968/deepCoadd/HSC-G/9813/5,8_nImage.fits
3) POST DM-12968 SafeClip HSC /datasets/hsc/repo/rerun/private/yusra/psfMatching/w_2017_48/safeClipOverride/deepCoadd/HSC-G/9813/5,8_nImage.fits
4) POST DM-12968 SafeClip ALL OTHER CAMERAS (see my comment #1 above): /datasets/hsc/repo/rerun/private/yusra/psfMatching/w_2017_48/safeClip/deepCoadd/HSC-G/9813/5,8_nImage.fits

Show
Yusra AlSayyad added a comment - Here's an N_Image (visits contributing to each pixel for a few configs left to right: 1) PRE DM-12968 WarpCompare HSC /datasets/hsc/repo/rerun/private/yusra/psfMatching/ DM-12697 /deepCoadd/HSC-G/9813/5,8_nImage.fits 2) POST DM-12968 WarpCompare HSC /datasets/hsc/repo/rerun/private/yusra/psfMatching/w_2017_48/warpCompare_ DM-12968 /deepCoadd/HSC-G/9813/5,8_nImage.fits 3) POST DM-12968 SafeClip HSC /datasets/hsc/repo/rerun/private/yusra/psfMatching/w_2017_48/safeClipOverride/deepCoadd/HSC-G/9813/5,8_nImage.fits 4) POST DM-12968 SafeClip ALL OTHER CAMERAS (see my comment #1 above): /datasets/hsc/repo/rerun/private/yusra/psfMatching/w_2017_48/safeClip/deepCoadd/HSC-G/9813/5,8_nImage.fits
Hide
Jim Bosch added a comment -

I hadn't realized the defaults for other cameras were in such bad shape - thanks for bringing that to my attention.

Yusra AlSayyad, do you mind taking another look, since the changes themselves are now fairly different from what they were before?

Show
Hide

OK to merge Jim Bosch

Show

## People

• Assignee:
Jim Bosch
Reporter:
Jim Bosch
Reviewers:
Watchers:
Jim Bosch, Paul Price, Robert Lupton, Yusra AlSayyad