Bug in imcombine avsigclip task
jhaislip wrote on Oct 13, 2008
Hi Everyone,
So I noticed in the source code for the avsigclip routine (icaclip.gx) that on line 403 the 'final line sigma' is set equal to the variable 's'. The algorithm then loops through each pixel in the line and performs the necessary rejection relative to this line sigma. After each pixel's rejection is complete, it then checks to see if too many have been rejected. If so, it determines which pixel to add back based on the pixel's residual. This involves taking the two pixels that were rejected last (one from the set of pixels less than the median and one from the set of pixels greater than median) and comparing the residuals from each. It stores the residual from the low side as the variable 'r' and the residual from the high side as variable 's' (line 489 and 490). This is overwriting the line sigma! When the algorithm moves on to the next pixel it will be rejecting not with respect to the line sigma, but with respect to the last reinstated pixel's residual. This isn't the intended behavior is it?
If it is not, then either the variable name for the line sigma should be changed or the variable name for the high-side residual should be changed. If it is the intended behavior, could you explain why? I'm trying to write a fits library that provides IRAF-like combining and an explanation of why you replace the line sigma with a residual would be great!
Thanks!
Joshua Haislip
So I noticed in the source code for the avsigclip routine (icaclip.gx) that on line 403 the 'final line sigma' is set equal to the variable 's'. The algorithm then loops through each pixel in the line and performs the necessary rejection relative to this line sigma. After each pixel's rejection is complete, it then checks to see if too many have been rejected. If so, it determines which pixel to add back based on the pixel's residual. This involves taking the two pixels that were rejected last (one from the set of pixels less than the median and one from the set of pixels greater than median) and comparing the residuals from each. It stores the residual from the low side as the variable 'r' and the residual from the high side as variable 's' (line 489 and 490). This is overwriting the line sigma! When the algorithm moves on to the next pixel it will be rejecting not with respect to the line sigma, but with respect to the last reinstated pixel's residual. This isn't the intended behavior is it?
If it is not, then either the variable name for the line sigma should be changed or the variable name for the high-side residual should be changed. If it is the intended behavior, could you explain why? I'm trying to write a fits library that provides IRAF-like combining and an explanation of why you replace the line sigma with a residual would be great!
Thanks!
Joshua Haislip
Francisco Valdes wrote on Oct 13, 2008
Hello Joshua,
I believe you are correct about this being a bug. I have made a change for the next release. You are free to make an appropriate change but what I did was replace "s = sqrt (s..." with "sig = sqrt (s..." along with adding the variable declaration and change where s is used as the line sigma.
Thanks for identifying this problem.
Yours,
Frank Valdes
I believe you are correct about this being a bug. I have made a change for the next release. You are free to make an appropriate change but what I did was replace "s = sqrt (s..." with "sig = sqrt (s..." along with adding the variable declaration and change where s is used as the line sigma.
Thanks for identifying this problem.
Yours,
Frank Valdes
Last post on Oct 13, 2008