DM-23699: Update fgcmcal default config format #30
Conversation
All list fields which depended on getting the band order correct have been deprecated and replaced with dict mappings to avoid many possible index errors. This is supported with 3.2.0 version of fgcm. Additional FutureWarnings from numpy 1.17 have also been fixed.
Also update logic so that ccds with valid interpolated zeropoints are output.
python/lsst/fgcmcal/fgcmFitCycle.py
Outdated
| ) | ||
| fitBands = pexConfig.ListField( | ||
| doc=("Bands to use in atmospheric fit. Other bands will have atmosphere constrained " | ||
| "from observations in the other bands on the same night."), |
There was a problem hiding this comment.
Stutter on “other bands”...replace second with “those listed here”/“the bands used in the fit”?
State that this must be a subset of config.bands & validate?
Unrelated: what is the meaning of the default of ("NO_DATA",) for bands above?
There was a problem hiding this comment.
All these fields are validated immediately by the fgcm code, and that's why I haven't put in redundant validations in fgcmcal. However, some basic validations would not be a bad idea. As for the NO_DATA, this was cargo-culted from somewhere...
python/lsst/fgcmcal/fgcmFitCycle.py
Outdated
| "It will be removed after v20. Use expGrayPhotometricCutDict instead."), | ||
| ) | ||
| expGrayPhotometricCutDict = pexConfig.DictField( | ||
| doc=("Per-band specification on maximum (negative) exposure gray for a visit to be " |
| @@ -83,7 +83,8 @@ def setDefaults(self): | |||
|
|
|||
| self.fgcmBuildStars.checkAllCcds = False | |||
| self.fgcmBuildStars.densityCutMaxPerPixel = 10000 | |||
| self.fgcmFitCycle.useRepeatabilityForExpGrayCuts = [True] | |||
| self.fgcmFitCycle.useRepeatabilityForExpGrayCutsDict = {b: True for | |||
| b in self.fgcmFitCycle.bands} | |||
There was a problem hiding this comment.
b -> band would be clearer
python/lsst/fgcmcal/fgcmFitCycle.py
Outdated
| "It will be removed after v20. Use ccdGraySubCcdDict instead."), | ||
| ) | ||
| ccdGraySubCcdDict = pexConfig.DictField( | ||
| doc="Per-band specification on whether to compute ccd gray on sub-ccd scale.", |
python/lsst/fgcmcal/fgcmFitCycle.py
Outdated
| ) | ||
| expGrayPhotometricCutDict = pexConfig.DictField( | ||
| doc=("Per-band specification on maximum (negative) exposure gray for a visit to be " | ||
| "considered photometric. Must have one entry per band."), |
There was a problem hiding this comment.
Should this get checked in a validate on this config Task? And should you also check the validate in the tests (i.e. include some configs that don’t comply & assert that they fail)?
python/lsst/fgcmcal/fgcmFitCycle.py
Outdated
| ) | ||
| expVarGrayPhotometricCutDict = pexConfig.DictField( | ||
| doc=("Per-band specification on maximum exposure variance to be considered possibly " | ||
| "photometric."), |
There was a problem hiding this comment.
Any guidance for another user on how to select a reasonable value here (and for all the other empty defaulted threshold configs)?
There was a problem hiding this comment.
I have added a bunch of defaults to setDefaults
| "calibration observations."), | ||
| keytype=str, | ||
| itemtype=float, | ||
| default={}, |
There was a problem hiding this comment.
Is it required that this be set & have any entry for each band?
python/lsst/fgcmcal/fgcmFitCycle.py
Outdated
| colorSplitBands = pexConfig.ListField( | ||
| doc="Band names to use to split stars by color. Must have 2 entries.", | ||
| dtype=str, | ||
| default=None, |
| i, b in enumerate(self.config.bands)} | ||
| updatedHighCutDict = {b: float(fgcmFitCycle.updatedHighCut[i]) for | ||
| i, b in enumerate(self.config.bands)} | ||
|
|
| deprecated=("This field is no longer used, and has been deprecated by DM-23699. " | ||
| "It will be removed after v20. Use superStarSubCcdDict instead."), | ||
| ) | ||
| superStarSubCcdDict = pexConfig.DictField( |
There was a problem hiding this comment.
You now explicitly include (append) “Dict” in the name for DictFields, but don’t append “List” for ListFields (just food for thought from an OCD-leaning personality 😉 )
All list fields which depended on getting the band order correct have been
deprecated and replaced with dict mappings to avoid many possible index
errors. This is supported with 3.2.0 version of fgcm. Additional
FutureWarnings from numpy 1.17 have also been fixed.