DM-23762: Check bad amps in LSSTCam are being tracked in the defects file #34

plazas · 2020-04-29T14:38:10Z

No description provided.

mfisherlevine · 2020-04-30T17:03:42Z

python/lsst/cp/pipe/ptc.py

@@ -393,7 +393,6 @@ def runDataRef(self, dataRef, visitPairs):
        # Produce coefficients for Polynomial ans Squared linearizers.
        dataset = self.fitPtcAndNonLinearity(dataset, self.config.ptcFitType,
                                             tableArray=lookupTableArray)
-


I don't care, but others really seem to - please don't randomly delete lines for no reason.

mfisherlevine · 2020-04-30T17:03:51Z

python/lsst/cp/pipe/ptc.py

@@ -414,12 +413,10 @@ def runDataRef(self, dataRef, visitPairs):
        for linType, dataType in [("LOOKUPTABLE", 'linearizeLut'),
                                  ("LINEARIZEPOLYNOMIAL", 'linearizePolynomial'),
                                  ("LINEARIZESQUARED", 'linearizeSquared')]:
-


mfisherlevine · 2020-04-30T17:03:56Z

python/lsst/cp/pipe/ptc.py

            if linType == "LOOKUPTABLE":
                tableArray = lookupTableArray
            else:
                tableArray = None
-


mfisherlevine · 2020-04-30T17:24:59Z

python/lsst/cp/pipe/ptc.py

+                dataset.nonLinearity[ampName] = [np.nan for _ in range(lenRawTimes)]
+                dataset.nonLinearityError[ampName] = [np.nan for _ in range(lenRawTimes)]
+                dataset.nonLinearityResiduals[ampName] = [np.nan for _ in range(lenRawTimes)]
+                dataset.fractionalNonLinearityResiduals[ampName] = [np.nan for _ in range(lenRawTimes)]
                dataset.coefficientLinearizeSquared[ampName] = np.nan
+                dataset.ptcFitPars[ampName] = [np.nan for _ in range(lenRawTimes)]
+                dataset.ptcFitParsError[ampName] = [np.nan for _ in range(lenRawTimes)]
+                dataset.ptcFitReducedChiSquared[ampName] = np.nan
+                dataset.coefficientsLinearizePolynomial[ampName] = [np.nan for _ in range(lenNonLinPars)]
+                tableArray[i, :] = [np.nan for _ in range(self.config.maxAduForLookupTableLinearizer)]


For all these for _ in range(len...) list-comps why not just do either
[np.nan for _ in dataset.rawExpTimes] and dispense with the initial len() and the range(), or use
np.full_like(dataset.rawExpTimes, np.nan, dtype=float) to explictly make a matching nan-array?

Also, if we're not plotting these, I'm not quite sure the shape of any of these matters rather than just having a simple nan actually.

mfisherlevine · 2020-04-30T17:26:28Z

python/lsst/cp/pipe/ptc.py

+                dataset.nonLinearity[ampName] = [np.nan for _ in range(lenRawTimes)]
+                dataset.nonLinearityError[ampName] = [np.nan for _ in range(lenRawTimes)]


Shouldn't these two be the length of lenNonLinPars rather than the number of inputs?

mfisherlevine · 2020-04-30T17:31:08Z

python/lsst/cp/pipe/ptc.py

+            if len(meanVecFinal):  # Empty if the whole amp is bad, for example.
+                minMeanVecFinal = np.min(meanVecFinal)
+                maxMeanVecFinal = np.max(meanVecFinal)
+                meanVecFit = np.linspace(minMeanVecFinal, maxMeanVecFinal, 100*len(meanVecFinal))
+                minMeanVecOriginal = np.min(meanVecOriginal)
+                maxMeanVecOriginal = np.max(meanVecOriginal)
+                deltaXlim = maxMeanVecOriginal - minMeanVecOriginal
+
+                a.plot(meanVecFit, ptcFunc(pars, meanVecFit), color='red')
+                a.plot(meanVecFinal, pars[0] + pars[1]*meanVecFinal, color='green', linestyle='--')
+                a.scatter(meanVecFinal, varVecFinal, c='blue', marker='o', s=markerSize)
+                a.scatter(meanVecOutliers, varVecOutliers, c='magenta', marker='s', s=markerSize)
+                a.set_xlabel(r'Mean signal ($\mu$, DN)', fontsize=labelFontSize)
+                a.set_xticks(meanVecOriginal)
+                a.set_ylabel(r'Variance (DN$^2$)', fontsize=labelFontSize)
+                a.tick_params(labelsize=11)
+                a.text(0.03, 0.8, stringLegend, transform=a.transAxes, fontsize=legendFontSize)
+                a.set_xscale('linear', fontsize=labelFontSize)
+                a.set_yscale('linear', fontsize=labelFontSize)
+                a.set_title(amp, fontsize=titleFontSize)
+                a.set_xlim([minMeanVecOriginal - 0.2*deltaXlim, maxMeanVecOriginal + 0.2*deltaXlim])
+
+                # Same, but in log-scale
+                a2.plot(meanVecFit, ptcFunc(pars, meanVecFit), color='red')
+                a2.scatter(meanVecFinal, varVecFinal, c='blue', marker='o', s=markerSize)
+                a2.scatter(meanVecOutliers, varVecOutliers, c='magenta', marker='s', s=markerSize)
+                a2.set_xlabel(r'Mean Signal ($\mu$, DN)', fontsize=labelFontSize)
+                a2.set_ylabel(r'Variance (DN$^2$)', fontsize=labelFontSize)
+                a2.tick_params(labelsize=11)
+                a2.text(0.03, 0.8, stringLegend, transform=a2.transAxes, fontsize=legendFontSize)
+                a2.set_xscale('log')
+                a2.set_yscale('log')
+                a2.set_title(amp, fontsize=titleFontSize)
+                a2.set_xlim([minMeanVecOriginal, maxMeanVecOriginal])
+            else:
+                a.set_xlabel(r'Mean signal ($\mu$, DN)', fontsize=labelFontSize)
+                a.set_ylabel(r'Variance (DN$^2$)', fontsize=labelFontSize)
+                a.tick_params(labelsize=11)
+                a.text(0.03, 0.8, stringLegend, transform=a.transAxes, fontsize=legendFontSize)
+                a.set_xscale('linear', fontsize=labelFontSize)
+                a.set_yscale('linear', fontsize=labelFontSize)
+                a.set_title(f"{amp} (BAD)", fontsize=titleFontSize)
+
+                a2.set_xlabel(r'Mean Signal ($\mu$, DN)', fontsize=labelFontSize)
+                a2.set_ylabel(r'Variance (DN$^2$)', fontsize=labelFontSize)
+                a2.tick_params(labelsize=11)
+                a2.text(0.03, 0.8, stringLegend, transform=a2.transAxes, fontsize=legendFontSize)
+                a2.set_xscale('log')
+                a2.set_yscale('log')
+                a2.set_title(f"{amp} (BAD)", fontsize=titleFontSize)


I think pulling out the elements in common and calling those together outside of the if/else would help keep things together and easier to understand & maintain.

mfisherlevine · 2020-04-30T17:31:47Z

python/lsst/cp/pipe/ptc.py

+            if len(meanVecFinal):
+                pars, parsErr = dataset.nonLinearity[amp], dataset.nonLinearityError[amp]
+                k0, k0Error = pars[0], parsErr[0]
+                k1, k1Error = pars[1], parsErr[1]
+                k2, k2Error = pars[2], parsErr[2]
+                stringLegend = (f"k0: {k0:.4}+/-{k0Error:.2e} DN\n k1: {k1:.4}+/-{k1Error:.2e} DN/t"
+                                f"\n k2: {k2:.2e}+/-{k2Error:.2e} DN/t^2 \n")
+                a.scatter(timeVecFinal, meanVecFinal)
+                a.plot(timeVecFinal, self.funcPolynomial(pars, timeVecFinal), color='red')
+                a.set_xlabel('Time (sec)', fontsize=labelFontSize)
+                a.set_xticks(timeVecFinal)
+                a.set_ylabel(r'Mean signal ($\mu$, DN)', fontsize=labelFontSize)
+                a.tick_params(labelsize=labelFontSize)
+                a.text(0.03, 0.75, stringLegend, transform=a.transAxes, fontsize=legendFontSize)
+                a.set_xscale('linear', fontsize=labelFontSize)
+                a.set_yscale('linear', fontsize=labelFontSize)
+                a.set_title(amp, fontsize=titleFontSize)
+            else:
+                a.set_xlabel('Time (sec)', fontsize=labelFontSize)
+                a.set_ylabel(r'Mean signal ($\mu$, DN)', fontsize=labelFontSize)
+                a.tick_params(labelsize=labelFontSize)
+                a.text(0.03, 0.75, stringLegend, transform=a.transAxes, fontsize=legendFontSize)
+                a.set_xscale('linear', fontsize=labelFontSize)
+                a.set_yscale('linear', fontsize=labelFontSize)
+                a.set_title(f"{amp} (BAD)", fontsize=titleFontSize)
+


mfisherlevine · 2020-04-30T17:32:38Z

python/lsst/cp/pipe/ptc.py

+            if len(meanVecFinal):
+                a.scatter(meanVecFinal, linRes)
+                a.axhline(y=0, color='k')
+                a.axvline(x=timeVecFinal[self.config.linResidualTimeIndex], color='g', linestyle='--')
+                a.set_xlabel(r'Mean signal ($\mu$, DN)', fontsize=labelFontSize)
+                a.set_xticks(meanVecFinal)
+                a.set_ylabel('LR (%)', fontsize=labelFontSize)
+                a.tick_params(labelsize=labelFontSize)
+                a.set_xscale('linear', fontsize=labelFontSize)
+                a.set_yscale('linear', fontsize=labelFontSize)
+                a.set_title(amp, fontsize=titleFontSize)
+            else:
+                a.axhline(y=0, color='k')
+                a.set_xlabel(r'Mean signal ($\mu$, DN)', fontsize=labelFontSize)
+                a.set_xticks(meanVecFinal)
+                a.set_ylabel('LR (%)', fontsize=labelFontSize)
+                a.tick_params(labelsize=labelFontSize)
+                a.set_xscale('linear', fontsize=labelFontSize)
+                a.set_yscale('linear', fontsize=labelFontSize)
+                a.set_title(f"{amp} (BAD)", fontsize=titleFontSize)


python/lsst/cp/pipe/ptc.py

mfisherlevine · 2020-05-01T13:29:01Z

python/lsst/cp/pipe/ptc.py

+                dataset.ptcFitPars[ampName] = np.nan
+                dataset.ptcFitParsError[ampName] = np.nan
+                dataset.ptcFitReducedChiSquared[ampName] = np.nan
+                dataset.coefficientsLinearizePolynomial[ampName] = [np.nan for _ in range(lenNonLinPars)]


[np.nan] * lenNonLinPars

Also though, is this any more necessary than the other nan-lists which you removed?

mfisherlevine · 2020-05-01T13:32:59Z

python/lsst/cp/pipe/ptc.py

+                dataset.ptcFitParsError[ampName] = np.nan
+                dataset.ptcFitReducedChiSquared[ampName] = np.nan
+                dataset.coefficientsLinearizePolynomial[ampName] = [np.nan for _ in range(lenNonLinPars)]
+                tableArray[i, :] = [np.nan for _ in range(self.config.maxAduForLookupTableLinearizer)]


[np.nan] * self.config.maxAduForLookupTableLinearizer

mfisherlevine · 2020-05-01T13:34:11Z

python/lsst/cp/pipe/ptc.py

@@ -968,6 +968,7 @@ def errFunc(p, x, y):
                msg = (f"\nSERIOUS: Not enough data points ({len(meanVecFinal)}) compared to the number of"
                       f"parameters of the PTC model({len(parsIniPtc)}). Setting {ampName} to BAD.")
                self.log.warn(msg)
+                lenNonLinPars = self.config.polynomialFitDegreeNonLinearity - 1


If anything, shouldn't this be +1 not -1? If I fit an x^2 i.e. 2nd order poly, I'd expect 3 coeffs, not 1, right?

Handle bad amps when plotting and persisting

f816009

plazas requested a review from mfisherlevine April 29, 2020 14:38

mfisherlevine reviewed Apr 30, 2020

View reviewed changes

Address comments by reviewer

a2f23f7

mfisherlevine reviewed May 1, 2020

View reviewed changes

Replace list-comps

7c429ec

mfisherlevine approved these changes May 1, 2020

View reviewed changes

plazas merged commit 7d47663 into master May 1, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DM-23762: Check bad amps in LSSTCam are being tracked in the defects file #34

DM-23762: Check bad amps in LSSTCam are being tracked in the defects file #34

plazas commented Apr 29, 2020

mfisherlevine Apr 30, 2020

mfisherlevine Apr 30, 2020

mfisherlevine Apr 30, 2020

mfisherlevine Apr 30, 2020

mfisherlevine Apr 30, 2020

mfisherlevine Apr 30, 2020

mfisherlevine Apr 30, 2020

mfisherlevine Apr 30, 2020

mfisherlevine Apr 30, 2020

mfisherlevine May 1, 2020

mfisherlevine May 1, 2020

mfisherlevine May 1, 2020

mfisherlevine May 1, 2020

		dataset.nonLinearity[ampName] = [np.nan for _ in range(lenRawTimes)]
		dataset.nonLinearityError[ampName] = [np.nan for _ in range(lenRawTimes)]

DM-23762: Check bad amps in LSSTCam are being tracked in the defects file #34

DM-23762: Check bad amps in LSSTCam are being tracked in the defects file #34

Conversation

plazas commented Apr 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment