-
Notifications
You must be signed in to change notification settings - Fork 7
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.
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
DM-45928: add parallel CTI and add serial/parallel turnoff calculations to DeferredChargeCalib #280
Conversation
1f0f97f
to
d6cc862
Compare
28fab7f
to
6fbf70c
Compare
67580da
to
fbdd3ee
Compare
7b3b1c5
to
3825f88
Compare
5b92870
to
3b40c4d
Compare
@@ -37,153 +38,231 @@ class CpCtiSolveTaskTestCase(lsst.utils.tests.TestCase): | |||
def setUp(self): | |||
self.camera = IsrMock().getCamera() | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where did these data come from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the originals were just empirical values I had during testing, and suspect the same of the updated values.
self.assertEqual( | ||
calib.signals[ampName][-1] - calib.signals[ampName][-2], | ||
calib.parallelCtiTurnoffSamplingErr[ampName], | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where did the expectations come from above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added SerialOscanMeansD
, which is roughly 20*SerialOscanMeansC
. The code should have correctly calculated that the serial turnoff is at the signal level associated with C and the serial overscan turnoff sampling error is roughly (signal D - signal B) / 2.
I also set all the parallel overscan means to the same thing at all signal levels (which is similar to never sampling the turnoff, which is common in real data). In that case it shoul set the parallel turnoff to the last value and the parallel turnoff sampling error should be the spacing between the last value and the second-to-last value.
I can add a comment to clarify this.
@@ -37,153 +38,231 @@ class CpCtiSolveTaskTestCase(lsst.utils.tests.TestCase): | |||
def setUp(self): | |||
self.camera = IsrMock().getCamera() | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the originals were just empirical values I had during testing, and suspect the same of the updated values.
3b40c4d
to
3d21fb2
Compare
3d21fb2
to
aadd571
Compare
aadd571
to
9e11bcd
Compare
No description provided.