DM-26862: Add focal-plane outlier rejection and focal-plane residual fits to fgcm #18
Conversation
Add focal-plane gray extinction fitting
| if ignoreRef: | ||
| obsFlag &= ~obsFlagDict[flagName] | ||
| else: | ||
| obsFlag &= ~obsFlagDict[flagName] |
There was a problem hiding this comment.
Why the if/else here? Is it correct to call this a "reset" if you are performing &=? (I'm likely missing something here since I have made no attempt to dig deeply into all the nuances of your flagging...).
There was a problem hiding this comment.
I've added additional comments on the meanings of the FLAGs. And the reset is done by arr &= ~flag (notice the ~ (not) there), which will set any values in arr that have a bit set to 0. It's not intuitive, but it is provable.
There was a problem hiding this comment.
Ok, cool. I'm also wondering the if/else here has any meaning (the same operation is executed in either case...flagName gets set based on the if/else ignoreRef just above).
There was a problem hiding this comment.
It's not quite the same operation, the ignoreRef is passed on below which leads to different numbers being processed. But point taken!
fgcm/fgcmStars.py
Outdated
| hi,=np.where(objRARad > np.pi) | ||
| objRARad[hi] -= 2*np.pi | ||
| # hi,=np.where(objRARad > np.pi) | ||
| # objRARad[hi] -= 2*np.pi |
There was a problem hiding this comment.
Not the most helpful "FIXME" description 😉 (and I couldn't help but noticed you just deleted in fgcm/fgcmParameters.py).
There was a problem hiding this comment.
I've improved the FIXME, which is helpful for me most of all. Thanks for flagging this!
There was a problem hiding this comment.
It's future-you I'm thinking about (and maybe me if I continue to fail to resist the temptation of looking into the changes in this repo!)
fgcm/fgcmSuperStarFlat.py
Outdated
| @@ -177,7 +177,7 @@ def computeSuperStarFlats(self, doPlots=True, doNotUseSubCCD=False, onlyObsErr=F | |||
| # with x/y, new sub-ccd | |||
|
|
|||
| # we will need the ccd offset signs | |||
| self._computeCCDOffsetSigns(goodObs) | |||
| # self._computeCCDOffsetSigns(goodObs) | |||
There was a problem hiding this comment.
Delete commented out code (and the comment says you need it!)?
| # If we are in the initial cycle, we can do outlier rejection now. | ||
| if self.initialCycle: | ||
| self.fgcmStars.performFocalPlaneOutlierCuts(self.fgcmPars, reset=True, ignoreRef=True) | ||
| self.fgcmStars.performFocalPlaneOutlierCuts(self.fgcmPars, reset=True, ignoreRef=False) |
There was a problem hiding this comment.
I find this puzzling...why are both calls required? I suppose I'm thinking that reset implies that any flags set are being wiped out and then "reset"...so wouldn't the second call just override anything that happened in the first? My naive expectation would be that you only call this once (either ignoring reference stars or not...)
There was a problem hiding this comment.
These two calls reset and set different flags. I've improved the comments to make this clearer, but I want to make sure that if a star has a bad reference magnitude it doesn't necessarily mean that it's a bad star.
fgcm/fgcmStars.py
Outdated
| @@ -1559,6 +1563,72 @@ def performSuperStarOutlierCuts(self, fgcmPars, reset=False): | |||
| # I had considered it might be necessary to flag bad exposures | |||
| # at this point, but I don't think that's the case. | |||
|
|
|||
| def performFocalPlaneOutlierCuts(self, fgcmPars, reset=False, ignoreRef=False): | |||
| """ | |||
| Do focal plane outlier cuts per exposure. | |||
There was a problem hiding this comment.
Short description goes on same line as """.
I see that's pervasive (and I'm not going to insist!), but FYI from the dev guide (https://developer.lsst.io/python/numpydoc.html#basic-format-of-docstrings):
The docstring’s summary sentence occurs on the same line as the opening """.
fgcm/fgcmGray.py
Outdated
| pars[0, 0] = 1.0 | ||
|
|
||
| FGrayGO = 10.**(EGrayGO / (-2.5)) | ||
| FGrayErrGO = (np.log(10.) / 2.5) * np.sqrt(EGrayErr2GO) * FGrayGO |
There was a problem hiding this comment.
Extra () around -2.5 and spaces between * & /.
fgcm/fgcmGray.py
Outdated
| fit = pars.flatten() | ||
| fit[0] = (np.sum(EGrayGO[i1a]/EGrayErr2GO[i1a]) / | ||
| np.sum(1./EGrayErr2GO[i1a])) | ||
| fit[0] = 10.**(fit[0]/(-2.5)) |
|
|
||
| Parameters | ||
| ---------- | ||
| fgcmStars : `fgcmStars` |
There was a problem hiding this comment.
Did you stall out while writing documenting? 😉
Fix the computation of E_gray with reference stars.
Also fix reference star outlier rejection bugs (in that outlier reference stars were not being rejected for all computations).