DM-26615: Remove afw.filter dependency from DcrModel #167
Conversation
isullivan
force-pushed
the
tickets/DM-26615
branch
from
7dadcb9
to
edd774d
Compare
|
|
||
| @classmethod | ||
| def fromDataRef(cls, dataRef, datasetType="dcrCoadd", numSubfilters=None, **kwargs): | ||
| def fromDataRef(cls, dataRef, effectiveWavelength, bandwidth, datasetType="dcrCoadd", numSubfilters=None, |
There was a problem hiding this comment.
Why do effectiveWavelength and bandwidth have defaults above (in init) but not here?
python/lsst/ip/diffim/dcrModel.py
Outdated
| Dictionary of spatially relevant retrieved coadd patches, | ||
| indexed by their sequential patch number. | ||
| effectiveWavelength : `float` | ||
| The effective wavelengths of the current filter. |
There was a problem hiding this comment.
I presume "wavelengths" vs wavelength is a typo (also above on L131).
The units should be mentioned either explicitly or with reference to the definition on L283 (the latter is probably preferable in case it changes).
python/lsst/ip/diffim/dcrModel.py
Outdated
| Returns | ||
| ------- | ||
| bandwidth : `float` | ||
| The bandwidth of the current filter. |
There was a problem hiding this comment.
"In the same units as effectiveWavelength"?
| dtype=int, | ||
| default=3, | ||
| ) | ||
| effectiveWavelength = pexConfig.Field( | ||
| doc="Effective wavelength of the filter. Used only if ``coaddName``='dcr'", |
There was a problem hiding this comment.
Should the unit=nanometer be mentioned here?
| refShift = [(-0.5575567724366292, -0.2704095599533037), | ||
| (0.001961910992342903, 0.000951507567181944), | ||
| (0.40402552599550073, 0.19594841296051665)] | ||
| refShift = [(-0.587550080368824, -0.28495575189083666), |
There was a problem hiding this comment.
Do you understand why these numbers changed, and/or does it matter?
There was a problem hiding this comment.
The minimum and maximum wavelengths were slightly asymmetric before, which was slightly more accurate but not supportable by the new method. As a result of the small shift in wavelengths, the results of this test changed.
isullivan
force-pushed
the
tickets/DM-26615
branch
from
edd774d
to
6da1b6f
Compare
The new Filter will not contain wavelength information, so that must be removed. The long-term solution is to use the actual transmission curves, but for now we will have to simply pass that in as a config option.