search

spacetelescope / stcal

https://stcal.readthedocs.io/en/latest/
Other
10 stars 32 forks source link

JP-3463: Dark current noise floor #236

Closed mwregan2 closed 8 months ago

mwregan2 commented 10 months ago

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

codecov[bot] commented 10 months ago

Codecov Report

Attention: 28 lines in your changes are missing coverage. Please review.

Comparison is base (dfa9a50) 85.94% compared to head (23e3562) 85.80%.

Files Patch % Lines
tests/test_jump.py 25.00% 12 Missing :warning:
tests/test_ramp_fitting_gls_fit.py 58.33% 10 Missing :warning:
src/stcal/ramp_fitting/ols_fit.py 50.00% 4 Missing :warning:
src/stcal/ramp_fitting/ramp_fit.py 50.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

hbushouse commented 10 months ago

@mwregan2 Still waiting for review comments/suggestions to be addressed.

hbushouse commented 8 months ago

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.

hbushouse commented 8 months ago

@mwregan2 I'm still confused as to why there seem to be some changes to the jump code included here. Is that a mistake?

hbushouse commented 8 months ago

@kmacdonald-stsci Can you review again to see if you agree with the way avg_dark_current has now been implemented within the ramp_data class?

hbushouse commented 8 months ago

Superseded by #243