The function used to interpolate over a defects, interpolateOverDefects
in meas_algorithms
, requires a Psf input argument. However, the current implementation makes no use of this Psf (as evidenced in the function definition which does not associate it with a variable name). There are several functions throughout the codebase that make use of this function for interpolation. Some are aware of the fact that the Psf argument is not used, e.g. the documentation for interpolateOnePlane
states:
Note that the interpolation code in meas_algorithms currently
doesn’t use the input PSF (though it’s a required argument),
so it’s not important to set the input PSF parameters exactly.
However, there are a host of calls to the interpolation function that are not necessarily aware that the Psf they are feeding it is not in fact being used. An example is in
ip_isr
’s isrTask.py
which defines fwhm as a config parameter. This fwhm is then passed to several different functions whose purpose is to perform interpolation over some defect type (e.g. saturationInterpolation
, maskAndInterpDefect
, maskAndInterpNan
). These all feed into the interpolateOverDefects
function which make no use of the fwhm being passed. In another example, obs_subaru
’s isr.py sets a config parameter:
fwhmForBadColumnInterpolation = pexConfig.Field(
dtype = float,
doc = "FWHM of PSF used when interpolating over bad columns (arcsec)",
default = 1.0,
)
which certainly implies that it is being used to some effect.
My original instinct was to remove the unused parameter from interpolateOverDefects
and adapt any calls to it in the rest of the codebase. However, given the above, I think we should be asking the question of wether the interpolation code should be making use of the Psf (estimated or not), and perhaps there should be two versions: one requiring and using the Psf and the other not (i.e. the function as is).
As it currently stands, I think it is very misleading to have so many tasks using this function and perhaps making some erroneous assumptions about the use of the Psf they are feeding into it.
I think @rhl may have some thoughts/opinions on the original intentions of including this placeholder in the function. Was/is there a plan to make use of the Psf in the interpolation that has fallen through the cracks?