# Variance underestimated in patch overlaps in templates

XMLWordPrintable

#### Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s: None
• Labels:
None
• Story Points:
8
• Team:
Data Release Production
• Urgent?:
No

#### Description

See notebook for examples from DP0.2:

VarianceUnderestimateOverlaps.html

Currently, the code that mosaics templates from coadds does so by coadding them. By default the variance plane is computing per-pixel by taking the inverse of the sum of inverse variances. This assumes inputs are independent, but the same calexps go into neighboring patches. (It's just counting the same calexps twice) Therefore the variance planes in those overlap regions are underestimated.

A potential $\sqrt{2}$, or $\sqrt{4}$ underestimate sounds scary, but the variance in the science image >> variance of the template. So in the case where you have 4 coadds overlapping, this ends up being a 7% effect for 10 visits and 0.7% effect for 100 visits in practice on the variance places of the image differences...

... which is also why we haven't noticed.  Not catastrophic for DP0.2, but we should post in the known issues.

#### Attachments

1. DM-34628.html
2.38 MB
2. VarianceUnderestimateOverlaps.html
4.87 MB

#### Activity

Hide
Ian Sullivan added a comment -

Adding a link to DM-33001, since the variance plane underestimation is a much more serious effect when convolving the science image.

Show
Ian Sullivan added a comment - Adding a link to DM-33001 , since the variance plane underestimation is a much more serious effect when convolving the science image.
Hide

Morning Jim Bosch, would you be up for reviewing https://github.com/lsst/afw/pull/666/files? It’s the implementation of your “just average the variances when mosaicking coadds” suggestion. I’m feeling a little uneasy about a few aspects of the interface:

1) I couldn’t think of a better method name than “setCalcErrorMosaicMode” at the time. I’m sure there’s something better to call the new option to ask for (var1 + var2)/2 instead of (var1 + var2)/4 (In the case where weights=1 of course).
2) You can only EITHER setCalcErrorMosaicMode OR setCalcErrorFromInputVariance. If you set both, it raises
3) Interface of getting a stack with mean variance seems reasonable in test_stacker.py, but in test_statistics.py getting a mean of the variances from statsMosaic.getError(afwMath.MEAN) from seems a little icky.

Show
Yusra AlSayyad added a comment - Morning Jim Bosch , would you be up for reviewing https://github.com/lsst/afw/pull/666/files? It’s the implementation of your “just average the variances when mosaicking coadds” suggestion. I’m feeling a little uneasy about a few aspects of the interface: 1) I couldn’t think of a better method name than “setCalcErrorMosaicMode” at the time. I’m sure there’s something better to call the new option to ask for (var1 + var2)/2 instead of (var1 + var2)/4 (In the case where weights=1 of course). 2) You can only EITHER setCalcErrorMosaicMode OR setCalcErrorFromInputVariance. If you set both, it raises 3) Interface of getting a stack with mean variance seems reasonable in test_stacker.py, but in test_statistics.py getting a mean of the variances from statsMosaic.getError(afwMath.MEAN) from seems a little icky.
Hide
Show
Hide
Jim Bosch added a comment -

Left a few comments on the PR, but mostly I'm not really bothered by how you've resolved these questions already.  In particular:

1) I couldn’t think of a better method name than “setCalcErrorMosaicMode” at the time.

I don't have a better name that's similarly concise, but (as noted on the PR) I do think we need some words in docs explaining it.

2) You can only EITHER setCalcErrorMosaicMode OR setCalcErrorFromInputVariance. If you set both, it raises.

Using an enum interface would be better if we were writing this from scratch (one enum value for each of these options, and another for whatever happens when you don't set either), but retrofitting that in now would mean deprecating the existing method and I don't really think that's worth it for this crufty old class.

3) Interface of getting a stack with mean variance seems reasonable in test_stacker.py, but in test_statistics.py getting a mean of the variances from statsMosaic.getError(afwMath.MEAN) from seems a little icky.

I'm actually not familiar enough with how one uses Statistics normally to have much of an opinion on this - I have somehow managed to avoid working on any code that uses it over the past decade.  And I basically regard its entire interface as "a little icky" anyway .

Show
Jim Bosch added a comment - Left a few comments on the PR, but mostly I'm not really bothered by how you've resolved these questions already.  In particular: 1) I couldn’t think of a better method name than “setCalcErrorMosaicMode” at the time. I don't have a better name that's similarly concise, but (as noted on the PR) I do think we need some words in docs explaining it. 2) You can only EITHER setCalcErrorMosaicMode OR setCalcErrorFromInputVariance. If you set both, it raises. Using an enum interface would be better if we were writing this from scratch (one enum value for each of these options, and another for whatever happens when you don't set either), but retrofitting that in now would mean deprecating the existing method and I don't really think that's worth it for this crufty old class. 3) Interface of getting a stack with mean variance seems reasonable in test_stacker.py, but in test_statistics.py getting a mean of the variances from statsMosaic.getError(afwMath.MEAN) from seems a little icky. I'm actually not familiar enough with how one uses Statistics normally to have much of an opinion on this - I have somehow managed to avoid working on any code that uses it over the past decade.  And I basically regard its entire interface as "a little icky" anyway .
Hide

Thank you for the reassurance

Show

#### People

Assignee:
Reporter:
Reviewers:
Jim Bosch
Watchers:
Eric Bellm, Ian Sullivan, Jim Bosch, Kenneth Herner, Nima Sedaghat, Yusra AlSayyad