DM-31880: Add support for new fgcm reference star options. #84
Conversation
python/lsst/fgcmcal/fgcmFitCycle.py
Outdated
| dtype=bool, | ||
| default=True, | ||
| ) | ||
| useExposureReferenceOffset = pexConfig.Field( | ||
| doc=("Use per-exposure (visit) reference offsets for final zeropoints? " |
There was a problem hiding this comment.
It may be too long to try to define in a config doc, but it's not obvious what is meant by "reference offsets".
python/lsst/fgcmcal/fgcmFitCycle.py
Outdated
| @@ -329,10 +329,16 @@ class FgcmFitCycleConfig(pipeBase.PipelineTaskConfig, | |||
| default=4.0, | |||
| ) | |||
| applyRefStarColorCuts = pexConfig.Field( | |||
| doc="Apply color cuts to reference stars?", | |||
| doc="Apply starColorCuts to reference stars?", | |||
There was a problem hiding this comment.
Urghhh...I totally agree this is now a most confusing config name (but I will not make you change it here!) Maybe just stick in an "(i.e. the color cuts specifically defined for the observed star sample)" and also state that this is in addition to any color cuts specified in refStarColorCuts in the doc?
python/lsst/fgcmcal/fgcmFitCycle.py
Outdated
| @@ -659,7 +665,12 @@ class FgcmFitCycleConfig(pipeBase.PipelineTaskConfig, | |||
| default=None, | |||
| ) | |||
| starColorCuts = pexConfig.ListField( | |||
| doc="Encoded star-color cuts (to be cleaned up)", | |||
| doc="Encoded star-color cuts (using calibrated colors)", | |||
There was a problem hiding this comment.
Again, may be difficult to concisely describe, but what does "calibrated colors" mean here? Also, for this config and refStarColorCuts below, I have no idea what the format is if not "NO_DATA" (I mean, yeah I do, 'cuz I saw the test update, but you know what I mean!)
| @@ -1563,6 +1577,9 @@ def _makeParSchema(self, parInfo, pars, parSuperStarFlat, | |||
| parSchema.addField('compNGoodStarPerExp', type='ArrayI', | |||
| doc='Computed number of good stars per exposure', | |||
| size=pars['COMPNGOODSTARPEREXP'].size) | |||
| parSchema.addField('compExpRefOffset', type='ArrayD', | |||
| doc='Computed per-visit median offset between standard stars and ref stars.', | |||
There was a problem hiding this comment.
This will be computed only using those objects used in the fit, right?
There was a problem hiding this comment.
Yes. Does this need an expanded doc?
There was a problem hiding this comment.
Up to you...I was just making sure (so, I guess adding it would preclude others having the same question, but I don't think it's crucial)!
| @@ -92,25 +92,27 @@ def test_fgcmcalPipeline(self): | |||
| nOkZp = 27 | |||
| nBadZp = 1093 | |||
| nStdStars = 235 | |||
| nPlots = 43 | |||
| nPlots = 48 | |||
There was a problem hiding this comment.
Can you squash the commit adding an interim change?
This PR adds support for the new fgcm 3.9.0 which includes...
This PR does not change the default configuration or behavior of fgcmcal.