DM-25984: Investigate why the afw means of flat images are NANs for several amps of BOT data after DM-25934 #44
Conversation
python/lsst/cp/pipe/ptc.py
Outdated
| for ampNumber, amp in enumerate(detector): | ||
| ampName = amp.getName() | ||
| # covAstier: (i, j, var (cov[0,0]), cov, npix) | ||
| doRealSpace = self.config.covAstierRealSpace | ||
| muDiff, varDiff, covAstier = self.measureMeanVarCov(exp1, exp2, region=amp.getBBox(), | ||
| covAstierRealSpace=doRealSpace) | ||
| if np.isnan(muDiff) and np.isnan(varDiff) and np.isnan(covAstier): |
There was a problem hiding this comment.
Are you OK with some but not all of these being NaN? Or do you want ors here?
There was a problem hiding this comment.
I'll put ors to report any NaN in the covariances too.
|
Where are these NaNs coming from? There should be none in the input frames (because we interpolate them all), so it's because there are labelled as defects. We addressed this problem on PFS; please check with @PaulPrice |
|
It happened in saturated flats. Apparently all of the pixels in the amp are saturated so with no good pixels MEANCLIP returned a NaN.
…________________________________
From: Robert Lupton <notifications@github.com>
Sent: Tuesday, July 21, 2020 6:18 AM
To: lsst/cp_pipe <cp_pipe@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Subject: Re: [lsst/cp_pipe] DM-25984: Investigate why the afw means of flat images are NANs for several amps of BOT data after DM-25934 (#44)
Where are these NaNs coming from? There should be none in the input frames (because we interpolate them all), so it's because there are labelled as defects. We addressed this problem on PFS; please check with @PaulPrice<https://github.com/PaulPrice>
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub<#44 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACH3T6SDMJCZGOGZIXRCZHDR4WIRVANCNFSM4PC2Q6XQ>.
|
tests/test_ptc.py
Outdated
| self.flatExp1.image.array[20:30] = np.nan | ||
| self.flatExp2.image.array[20:30] = np.nan |
There was a problem hiding this comment.
This is weird, and perhaps not what you mean. This is a sensor-wide 10-pixel slice. Did you mean [20,30] instead?
There was a problem hiding this comment.
Changed it to [20:30, :], thanks for the suggestion.
tests/test_ptc.py
Outdated
| @@ -281,6 +281,44 @@ def test_meanVarMeasurement(self): | |||
| self.assertLess(self.flatWidth - np.sqrt(varDiff), 1) | |||
| self.assertLess(self.flatMean - mu, 1) | |||
|
|
|||
| def test_meanVarMeasurementSomeNan(self): | |||
There was a problem hiding this comment.
Suggest either test_meanVarMeasurementSomeNans or test_meanVarMeasurementWithNans
There was a problem hiding this comment.
(I prefer the latter)
tests/test_ptc.py
Outdated
| self.assertTrue(mu is np.nan) | ||
| self.assertTrue(varDiff is np.nan) | ||
| self.assertTrue(covDiff is np.nan) |
There was a problem hiding this comment.
These lines don't actually work. You need self.assertTrue(np.isnan(mu)) etc here.
tests/test_ptc.py
Outdated
| expectedMu1 = np.nanmean(self.flatExp1.image.array) | ||
| expectedMu2 = np.nanmean(self.flatExp2.image.array) | ||
| expectedMu = 0.5*(expectedMu1 + expectedMu2) | ||
|
|
||
| # Now the variance of the difference. First, create the diff image. | ||
| im1 = self.flatExp1.maskedImage | ||
| im2 = self.flatExp2.maskedImage | ||
|
|
||
| temp = im2.clone() | ||
| temp *= expectedMu1 | ||
| diffIm = im1.clone() | ||
| diffIm *= expectedMu2 | ||
| diffIm -= temp | ||
| diffIm /= expectedMu | ||
|
|
||
| expectedVar = 0.5*np.nanvar(diffIm.image.array) | ||
|
|
||
| self.assertLess(np.sqrt(expectedVar) - np.sqrt(varDiff), 1) | ||
| self.assertLess(expectedMu - mu, 1) |
There was a problem hiding this comment.
Could you just put a comment in the code for why these conditions are being tested for? Clearly it works, but would be nice to for the next person not to have to think it all through.
tests/test_ptc.py
Outdated
| self.assertTrue(mu is np.nan) | ||
| self.assertTrue(varDiff is np.nan) | ||
| self.assertTrue(covDiff is np.nan) |
There was a problem hiding this comment.
I think these should all be changed to self.assertTrue(np.isnan(mu)) etc because if those functions above returned float('nan') instead then this would fail, and that would be annoying and hard to debug.
No description provided.