JP-3463: Dark current noise floor #236
Conversation
…2/stcal into dark-current-noise-floor
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #236 +/- ##
==========================================
- Coverage 85.94% 85.80% -0.14%
==========================================
Files 35 35
Lines 6523 6621 +98
==========================================
+ Hits 5606 5681 +75
- Misses 917 940 +23 ☔ View full report in Codecov by Sentry. |
|
@mwregan2 Still waiting for review comments/suggestions to be addressed. |
…2/stcal into dark-current-noise-floor
|
The errors in CI test "downstream" for jwst are all coming from the API change to "ramp_fit_data", because this PR adds more arguments to that function call, so the unit tests that are over in jwst/ramp_fitting/tests/test_ramp_fit.py don't match. That'll be fixed by a PR over in jwst. So I think the CI errors here are expected and OK. Should probably move all the unit tests out of jwst and into stcal, to avoid these kinds of issues. |
|
@mwregan2 I'm still confused as to why there seem to be some changes to the jump code included here. Is that a mistake? |
|
@kmacdonald-stsci Can you review again to see if you agree with the way |
| if image_info is None or integ_info is None: | ||
| return None, None, None | ||
|
|
||
| return image_info, integ_info, opt_info | ||
|
|
||
| # Call ramp fitting for multi-processor (multiple data slices) case | ||
| image_info, integ_info, opt_info = ols_ramp_fit_multiprocessing( | ||
|
|
||
| else: |
There was a problem hiding this comment.
This else is unnecessary.
| rnoise_slice = readnoise_2d[start_row:start_row + rslices[k], :].copy() | ||
| gain_slice = gain_2d[start_row:start_row + rslices[k], :].copy() | ||
| slices.insert( | ||
| k, (ramp_slice, buffsize, save_opt, rnoise_slice, gain_slice, weighting, avg_dark_current)) |
There was a problem hiding this comment.
The avg_dark_current is not needed as a part of this tuple. I am surprised this doesn't cause problems with the multiprocessing, since this tuple is a list of parameters to be used by ols_ramp_fit_single.
| @@ -640,10 +640,10 @@ def test_one_group_fit(): | |||
|
|
|||
| ramp_data, gain2d, rnoise2d = setup_inputs(dims, gain, rnoise, group_time, frame_time) | |||
| ramp_data.data[0, 0, 50, 50] = 10.0 | |||
|
|
|||
| avg_dark_current = 0.0 | |||
There was a problem hiding this comment.
Unused, so can be removed.
| @@ -665,10 +665,10 @@ def test_two_groups_unc(): | |||
|
|
|||
| ramp_data.data[0, 0, 50, 50] = 10.0 | |||
| ramp_data.data[0, 1, 50, 50] = 10.0 + deltaDN | |||
|
|
|||
| avg_dark_current = 0.0 | |||
There was a problem hiding this comment.
Unused, so can be removed.
| @@ -705,10 +705,10 @@ def test_five_groups_unc(): | |||
| ramp_data.data[0, 4, 50, 50] = 60.0 | |||
|
|
|||
| # check = np.median(np.diff(ramp_data.data[0,:,50,50])) / grouptime | |||
|
|
|||
| avg_dark_current = 0.0 | |||
There was a problem hiding this comment.
Unused, so can be removed.
| @@ -752,10 +752,10 @@ def test_oneCR_10_groups_combination(): | |||
| ramp_data.data[0, 9, 50, 50] = 180.0 | |||
|
|
|||
| ramp_data.groupdq[0, 5, 50, 50] = JUMP_DET | |||
|
|
|||
| avg_dark_current = 0.0 | |||
There was a problem hiding this comment.
Unused, so can be removed.
| @@ -806,10 +806,10 @@ def test_oneCR_10_groups_combination_noisy2ndSegment(): | |||
| ramp_data.data[0, 9, 50, 50] = 180.0 | |||
|
|
|||
| ramp_data.groupdq[0, 5, 50, 50] = JUMP_DET | |||
|
|
|||
| avg_dark_current = 0.0 | |||
There was a problem hiding this comment.
Unused, so can be removed.
|
Superseded by #243 |
Resolves JP-3463
Closes [spacetelescope/jwst#8071]
This PR addresses the issue described in JP-3463 that the Poisson Noise from the dark current in the pixels is not included in the rate uncertainty. This change adds the dark rate to the estimated source rate to determine the total Poisson noise.
Checklist
CHANGES.rst(either inBug FixesorChanges to API)