DM-25711: Compare the gains produced by FULLCOVARIANCE in the PTC task (DM-20070) and with the pre-existing options EXPAPPROXIMATION and POLYNOMIAL #43
Conversation
| @@ -543,9 +542,12 @@ def fitFullModel(self, pInit=None, nSigma=5): | |||
| covariances calculation, and the extra "1" is the gain. | |||
| If "b" is 0, then "c" is 0, and len(pInit) will have r^2 fewer entries. | |||
|
|
|||
| nSigma : `int` | |||
| nSigma : `int`, optional | |||
There was a problem hiding this comment.
This should be float, I think.
| covariance /= mu | ||
| model /= mu | ||
| weights *= mu | ||
|
|
||
| return mu, covariance, model, weights | ||
| return mu, covariance, model, weights, mask |
There was a problem hiding this comment.
The mask isn't in the "Returns" docstring block.
python/lsst/cp/pipe/plotPtc.py
Outdated
| meanVecFinal, varVecFinal, varVecModel, wc = fit.getNormalizedFitData(0, 0) | ||
| meanVecFinalCov01, varVecFinalCov01, varVecModelCov01, wcCov01 = fit.getNormalizedFitData(0, 1) | ||
| meanVecFinalCov10, varVecFinalCov10, varVecModelCov10, wcCov10 = fit.getNormalizedFitData(1, 0) | ||
| meanVecOriginal, varVecOriginal, varVecModelOriginal, wc, varMask = fit.getFitData(0, 0) |
There was a problem hiding this comment.
Can wc be renamed something more descriptive? And below in ptc.py.
I'm also not clear what the difference between Original and Final is. Original is the raw measured mean/variances, and Final is the same thing, but masked based on the weights? If I'm following the code correctly, the only time the weight is non-zero is when the mean*g > maxElectrons? No, that's not true, because setMaxMuElectrons is never called. It's the points excluded via the tophat iteratively-reweighted least squares in fitFullModel.
There was a problem hiding this comment.
Yes, "original" is "unmasked". The mask is based on the weights in getFitData , which in turn come from self.sqrtW. And this one is set in fitFullModel as you say.
plazas
force-pushed
the
tickets/DM-25711
branch
from
97fa5e3
to
30dd041
Compare
| def getNormalizedFitData(self, i, j, divideByMu=False): | ||
| """Get measured signal and covariance, cov model and wigths | ||
| def getFitData(self, i, j, divideByMu=False, unitsElectrons=False, returnMasked=False): | ||
| """Get measured signal and covariance, cov model, weigths, and mask. |
| for amp in self.ampNames: | ||
| self.assertAlmostEqual(localDataset.gain[amp], 0.75, places=2) | ||
| for v1, v2 in zip(varStandard[amp], localDataset.finalVars[amp][0]): | ||
| self.assertAlmostEqual(v1/v2, 1.0, places=4) | ||
| v2 *= (0.75**2) # convert to electrons | ||
| self.assertAlmostEqual(v1/v2, 1.0, places=1) |
There was a problem hiding this comment.
One digit isn't a very strong test. We'll probably want to revisit that in the future to see if there's an underlying difference to be concerned about.
Fix bugs in plotting and change max iter in fit
Test PTC fit with mock images
plazas
force-pushed
the
tickets/DM-25711
branch
from
1e3a23b
to
6ec5c08
Compare
No description provided.