-
Notifications
You must be signed in to change notification settings - Fork 32
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
JP-3827: Remove unused error from ramp data #334
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Removed all references to the unused ramp error array in dark, jump, and ramp fitting steps. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -278,6 +278,8 @@ | |
Bias of the best-fit count rate from using cvec plus the observed | ||
resultants to estimate the covariance matrix. | ||
""" | ||
raise NotImplementedError('Bias calculations are not implemented.') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I noticed in passing that this method currently has a code error: it calls a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good. I think it can be removed. The calculation is in my repo fit-ramp, but I do not think it is necessary to include in the jwst pipeline. |
||
|
||
alpha = countrates[np.newaxis, :] * self.alpha_phnoise[:, np.newaxis] | ||
alpha += sig**2 * self.alpha_readnoise[:, np.newaxis] | ||
beta = countrates[np.newaxis, :] * self.beta_phnoise[:, np.newaxis] | ||
|
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.
The gls_fit algorithm is unused and only partially implemented. I noticed in passing that the set_arrays call had incorrect arguments here, so I added the average dark current. Changes in this file should have no impact to any current data processing.
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.
Sounds good. The gls_fit algorithm should be superseded by the likelihood-based one--it should be the identical calculation, but with orders of magnitude better performance. I think we can consider permanently removing gls_fit.