Possible updates to combiner class

[SaOgaz]

Okay so, @eteq recommended I run this stuff by you guys before I started on anything. I've looked through the current stuff in the Combiner class and it looks like there's two things going on.

1. the combinations are being done with numpy masked array arithmetic calls. If these were changed to use the NDData arithmetic it would take care of the uncertainty arrays automatically. Are you guys okay with me changing these calls?

2. in addition to that, the "use the provided uncertainty" arrays when calculating the final uncertainty needs to be hooked into the actual average_combine,sum_combine, median_combine methods one way or another. I'm not sure if this also involves some tweaks to the combine function.

To get what I need out of Combiner I need to do 2 wether or not I do 1. Thoughts?

[SaOgaz]

In reference to #562.

[MSeifert04]

I have a few questions:

• How would you use NDData arithmetic for median_combine?
• Is it useful to use the uncertainty of the inputs? I always thought that in case of combination you're not interested in the propagated uncertainty but the uncertainty of the combined-image (different concept but I can understand why one could want the propagated uncertainty).
• How slow/fast would it be if we replaced the numpy functions with NDData arithmetic operations? combine is quite slow and memory expensive already, if it's a significant slowdown (say ~2-3 times slower or more) then it's probably worth providing it as alternative method/function instead of replacing the original.

[SaOgaz]

Well, median_combine would the obvious exception to using NDData arithmetic.

Using the uncertainties is how a lot of the pipelines for HST data work, and how the IRAF imcombine
task is most frequently run for calibration. Uncertainties in this case being the HST Error extensions.

As far as the speed question, I'm not really sure. Any idea about this @eteq?

[MSeifert04]

I thought imcombine also just uses the standard deviation:

Sigma of Combined Pixels
The sigma parameter produces optional output sigma image(s). The sigma is the standard deviation, corrected for a finite population, of the input pixel values (excluding rejected pixels) about the output combined pixel values.

[MSeifert04]

I thought about this some more and I think using arithmetic operations with uncertainty propagation isn't correct and the currently implemented way is the "correct one". Because you expect the combination to give a closer representation of the "real value" and then the standard deviation represents how "trustworthy" that combined value is. You don't expect the individual pixels to have a well-defined standard deviation (in most cases it's just an approximation like readout noise, etc.)

However, the more I think about it, we probably need to use ddof=1 when calculating the standard deviation ...

[SaOgaz]

Ahh, you're right, I was conflating imcombine and mscombine (a version of imcombine written specifically for HST data).

[MSeifert04]

@SaraOgaz No problem 😄

I just wondered: Is there some documentation for mscombine regarding the sigma/uncertainty output? I only found http://stsdas.stsci.edu/cgi-bin/gethelp.cgi?mscombine but I couldn't find anything relating to uncertainty propagation there.

[eteq]

Is it useful to use the uncertainty of the inputs? I always thought that in case of combination you're not interested in the propagated uncertainty but the uncertainty of the combined-image (different concept but I can understand why one could want the propagated uncertainty).

I thought about this some more and I think using arithmetic operations with uncertainty propagation isn't correct and the currently implemented way is the "correct one".

@MSeifert04 - I think it very much is useful for at least some combiner use cases. Consider the case of combining 2 or 3 exposures (which is probably ~50% of cases): if you have uncertainties of the individual exposures, using that information is much better information than the stddev of 2 or 3 exposures. Of course sometimes you can't trust the per-image uncertainties, but the combiner machinery should accept the use case where you do.

[eteq]

(Oh and whether or not imcombine or mscombine support this is not critical to our decision here: we want to support IRAF cases, but not necessarily follow all the IRAF conventions...)

[SaOgaz]

@MSeifert04 You can find a little more detail in the gcombine help: http://stsdas.stsci.edu/cgi-bin/gethelp.cgi?gcombine. Mscombine was a re-write/wrapper around gcombine.

[SaOgaz]

@eteq, agreed. @MSeifert04, my main concern here is that the Instrument teams at STScI still use this type of image combination for creating some reference files.

[eteq]

Oh, and to clarify: I see @MSeifert04's point in #569 (comment) that the current implementation is sometimes more correct. So this needs to be an option, as it depends on the situation which is more correct. My inclination is to use the presence/absence of uncertainty as a way to determine which should be used, but it could also be an explicit option to combiners.

[eteq]

(Might also be good to get opinions from @crawfordsm and/or @mwcraig on this as it has some significant implications for some of the science use cases...)

(this = should the combiners support using the uncertainties in individual frames, and how should that be exposed to users)

[MSeifert04]

Ah, thank you for the clarification @eteq. I was thinking too much about bias and flat fields and which of the two approaches is the correct one - because I understood the original issue to propose rewriting the current implementation with the propagate-uncertainty implementation.

Would it make more sense to add a sum and mean/average function unrelated to the Combiner class or are the combine-features like clipping and scaling useful in this context?