Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DM-42024: Allow for low gains and allow initial outlier cutoff to have units of electrons. #222

Merged
merged 3 commits into from
Dec 6, 2023
Merged

DM-42024: Allow for low gains and allow initial outlier cutoff to have units of electrons. #222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b399ff4
Change default bounds on initial gain fit.
erykoff Dec 5, 2023
c425650
Add option to scale initial signal cutoff by the gain.
erykoff Dec 5, 2023
c0e6461
Add correct variance to test ptc datasets.
erykoff Dec 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 26 additions & 5 deletions python/lsst/cp/pipe/ptc/cpSolvePtcTask.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,17 @@ class PhotonTransferCurveSolveConfig(pipeBase.PipelineTaskConfig,
maxSignalInitialPtcOutlierFit = pexConfig.Field(
dtype=float,
doc="Maximum signal considered for intial outlier fit. This should be below "
"the PTC turnoff to ensure accurate outlier rejection.",
default=30_000.,
"the PTC turnoff to ensure accurate outlier rejection. If "
"scaleMaxSignalInitialPtcOutlierFit=True then the units are electrons; "
"otherwise ADU.",
default=50_0000.,
)
scaleMaxSignalInitialPtcOutlierFit = pexConfig.Field(
dtype=bool,
doc="Scale maxSignalInitialPtcOutlierFit by approximate gain? If yes then "
"maxSignalInitialPtcOutlierFit is assumed to have units of electrons, "
"otherwise ADU.",
default=True,
)
minVarPivotSearch = pexConfig.Field(
dtype=float,
Expand Down Expand Up @@ -1005,8 +1014,8 @@ def errFunc(p, x, y):
if self.config.binSize > 1:
bounds = self._boundsForAstier(parsIniPtc)
else:
bounds = self._boundsForAstier(parsIniPtc, lowers=[-1e-4, 0.5, -2000],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How necessary are these bounds for the convergence of this initial fit? In any case, I'm just wondering if even the new values (min=0.2, and max=3.0) will give us trouble in the future. Or if this code is used for other cameras with different settings (I think DECam has gains of 4, so I'm not even sure at this point how this has worked in the past for DECam...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think anybody has ever run this on DECam. I'll change this to a bigger range.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did!

uppers=[1e-4, 2.5, 2000])
bounds = self._boundsForAstier(parsIniPtc, lowers=[-1e-4, 0.1, -2000],
uppers=[1e-4, 10.0, 2000])
if ptcFitType == 'POLYNOMIAL':
ptcFunc = funcPolynomial
parsIniPtc = self._initialParsForPolynomial(self.config.polynomialFitDegree + 1)
Expand All @@ -1022,6 +1031,18 @@ def errFunc(p, x, y):
# This algorithm was initially developed by Seth Digel for
# the EO Testing pipeline.

if self.config.scaleMaxSignalInitialPtcOutlierFit:
approxGain = np.nanmedian(meanVecOriginal/varVecOriginal)
maxADUInitialPtcOutlierFit = self.config.maxSignalInitialPtcOutlierFit/approxGain
self.log.info(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were concerned in another ticket about too much info in the logs, especially per amp. Will it be OK in this case? Should we change the level to "debug"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did think about this, and could make it debug. But it's not that much. The difference is that this is a log that is done per amp in a PTC fit (16 messages per hundreds of flat pairs) and not a log per amp per ISR (16 messages times hundreds of flat pairs).

"Using approximate gain %.3f and ADU signal cutoff of %.1f for amplifier %s",
approxGain,
maxADUInitialPtcOutlierFit,
ampName,
)
else:
maxADUInitialPtcOutlierFit = self.config.maxSignalInitialPtcOutlierFit

if maxIterationsPtcOutliers == 0:
# We are not doing any outlier rejection here, but we do want
# an initial fit.
Expand All @@ -1034,7 +1055,7 @@ def errFunc(p, x, y):
pars = res.x
newMask = mask.copy()
else:
newMask = (mask & (meanVecOriginal <= self.config.maxSignalInitialPtcOutlierFit))
newMask = (mask & (meanVecOriginal <= maxADUInitialPtcOutlierFit))

count = 0
lastMask = mask.copy()
Expand Down
2 changes: 1 addition & 1 deletion tests/test_linearity.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def _create_ptc(self, amp_names, exp_times, means, ccobcurr=None, photo_charges=
inputExpIdPair=exp_id_pairs[i],
rawExpTime=exp_times[i],
rawMean=raw_mean,
rawVar=1.0,
rawVar=raw_mean,
kspValue=1.0,
expIdMask=exp_id_mask,
photoCharge=photo_charges[i],
Expand Down