DM-31896: Clarify PTC code #112

plazas · 2021-11-17T21:03:10Z

No description provided.

Change fitting functions names in tests

Fix typos

Fix bugs Clarify docstrings

Fix typo

czwa

I'm hoping we can still simplify some more by removing the fft/direct choice, and keeping functions that are only called by one task as task methods.

python/lsst/cp/pipe/ptc/cpExtractPtcTask.py

czwa · 2022-01-20T23:32:36Z

python/lsst/cp/pipe/ptc/cpExtractPtcTask.py

+    The number of partially-filled
+    `~lsst.ip.isr.PhotonTransferCurveDataset` objects will be less
+    than the number of input exposures because the task combines
+    input flats in pairs. However, it is required at this mioment


czwa · 2022-01-20T23:46:26Z

python/lsst/cp/pipe/ptc/cpSolvePtcTask.py

+    before this task), produced a list of
+    `~lsst.ip.isr.PhotonTransferCurveDataset` objects. Each dataset
+    contains the mean signal and covariances of the
+    difference image of to flat-field images taken at


Typo: "of the flat-field images"?

czwa · 2022-01-21T00:30:15Z

python/lsst/cp/pipe/ptc/plotPtcGen2.py

@@ -35,11 +35,9 @@
 from lsst.utils.timer import timeMethod

 from lsst.cp.pipe.utils import (funcAstier, funcPolynomial, NonexistentDatasetTaskDataIdContainer,
-                                calculateWeightedReducedChi2)
+                                calculateWeightedReducedChi2, getFitDataFromCovariances)


Can we drop this file entirely, as gen2 is now deprecated?

czwa · 2022-01-21T23:08:24Z

python/lsst/cp/pipe/utils.py

+    return cov, var, muVals
+
+
+def symmetrize(inputArray):


This duplicates/is duplicated by BrighterFatterKernelSolveTask._tileArray(). I've added DM-33353 to unify these methods.

czwa · 2022-01-21T23:15:23Z

python/lsst/cp/pipe/utils.py

+    return mu, covariance, covarianceModel, weights, maskFromWeights
+
+
+def makeCovArray(inputTuple, maxRangeFromTuple=8):


Is this ever called outside of PhotonTransferCurveExtractTask? I have the same question with computeCovDirect above.

I think it's only called by the extract class, so I moved it there.

czwa · 2022-01-21T23:16:47Z

python/lsst/cp/pipe/utils.py

+        return tupleVec
+
+
+def computeCovDirect(diffImage, weightImage, maxRange):


Is there a reason to keep both the direct and FFT convolutions? If they give the same result, we should just implement one. I see we use the FFT by default, so maybe keeping that one? However, we tend to use lags of 8, which suggests the direct method should be faster (it's also simpler).

I kept the FFT version.

czwa · 2022-01-21T23:17:59Z

python/lsst/cp/pipe/ptc/cpSolvePtcTask.py

+                c = params[lenParams:2*lenParams].reshape((matrixSide, matrixSide))
+                noise = params[2*lenParams:3*lenParams].reshape((matrixSide, matrixSide))
+                gain = params[-1]
+                if key == 'fullModel':


Can't this just be fitResults[key]['a'] = a, and avoid this if/else?

czwa

Looks good. A few minor typos are all I could see.

czwa · 2022-02-01T18:12:30Z

python/lsst/cp/pipe/ptc/cpExtractPtcTask.py

                if (muDiff <= minMeanSignalDict[ampName]) or (muDiff >= maxMeanSignalDict[ampName]):
                    expIdMask = False

                if covAstier is not None:
+                    # Turn the tuples with teh measured information


czwa · 2022-02-01T18:13:51Z

python/lsst/cp/pipe/ptc/cpExtractPtcTask.py

-            c = CovFastFourierTransform(diffIm.image.array, w, fftShape, maxRangeCov)
-            covDiffAstier = c.reportCovFastFourierTransform(maxRangeCov)
+
+        # Calculate covariances via FFT (default).


Not the default anymore, as we only have the one option.

Fix typos

plazas force-pushed the tickets/DM-31896 branch from c565377 to 8f5cf3c Compare November 17, 2021 21:56

plazas force-pushed the tickets/DM-31896 branch 2 times, most recently from dd8dded to c3e3858 Compare December 17, 2021 22:58

plazas requested a review from czwa January 9, 2022 04:56

plazas force-pushed the tickets/DM-31896 branch from c3e3858 to 0eb934c Compare January 11, 2022 02:39

plazas force-pushed the tickets/DM-31896 branch from eb88feb to 2485b0a Compare January 20, 2022 16:47

plazas added 15 commits January 20, 2022 20:19

Add more comments to cpExtractPtcTask to clarify

ec126f5

Add clarifications to docstrings in solve task

1d02d14

Wrap fitting functions for clarity

9cae7f5

Change fitting functions names in tests

Remove one-liner chi2() method

3241d16

Delete unused functions

595fc1e

Fix typos

Get rid of FitParameters

b3712b4

Eliminate parseData function

cbc6758

Pass bool for b=0 in fullCov fit

8c978bf

Delete and rename functions

a55d105

Delete astierCovPtcUtils

824a751

Delete astierCovPtcFit

ca25e9a

Fix bugs Clarify docstrings

Delete unused params from imsim ptc yaml file

8987480

Set EXPAPPROXIMATION as default fit

d4362f4

Move warning line into log message

7313a77

Don't do fits if amp is BAD

4b8ae8b

Fix typo

czwa reviewed Jan 21, 2022

View reviewed changes

plazas added 5 commits January 26, 2022 14:06

Fix typos in docstrings

201d949

Delete gen2 plotting script

43a44d6

Delete unused gen2 util functions

a794b4f

Delete covariance method in real space

312e0ae

Make makeCovArray part of PTC extract task

ddbe5ec

plazas force-pushed the tickets/DM-31896 branch from 5c4077f to 4529b55 Compare January 26, 2022 21:55

plazas requested a review from czwa January 26, 2022 21:58

czwa approved these changes Feb 1, 2022

View reviewed changes

Simplify conditional to store fit results

14b38e8

Fix typos

plazas force-pushed the tickets/DM-31896 branch from 4529b55 to 14b38e8 Compare February 1, 2022 19:20

plazas merged commit 72c1dcc into main Feb 1, 2022

plazas deleted the tickets/DM-31896 branch February 1, 2022 19:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DM-31896: Clarify PTC code #112

DM-31896: Clarify PTC code #112

plazas commented Nov 17, 2021

czwa left a comment

czwa Jan 20, 2022

czwa Jan 20, 2022

czwa Jan 21, 2022

czwa Jan 21, 2022

czwa Jan 21, 2022

plazas Jan 26, 2022

czwa Jan 21, 2022

plazas Jan 26, 2022

czwa Jan 21, 2022

czwa left a comment

czwa Feb 1, 2022

czwa Feb 1, 2022

		return mu, covariance, covarianceModel, weights, maskFromWeights


		def makeCovArray(inputTuple, maxRangeFromTuple=8):

		return tupleVec


		def computeCovDirect(diffImage, weightImage, maxRange):

DM-31896: Clarify PTC code #112

DM-31896: Clarify PTC code #112

Conversation

plazas commented Nov 17, 2021

czwa left a 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

czwa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment