8000 JP-3447: Interpolate errors for replaced pixels by melanieclarke · Pull Request #8504 · spacetelescope/jwst · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

JP-3447: Interpolate errors for replaced pixels #8504

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Jun 11, 2024
Merged

JP-3447: Interpolate errors for replaced pixels #8504

merged 8 commits into from
Jun 11, 2024

Conversation

melanieclarke
Copy link
Collaborator
@melanieclarke melanieclarke commented May 23, 2024 •
edited by hbushouse
Loading

Resolves JP-3447

Closes #8029

This PR updates the pixel replace method to provide estimated errors, corresponding to the estimated fluxes in replaced pixels. Errors are estimated following the same interpolation scheme as is used on the data (either the minimum gradient method, or profile scaling). Variance components are also similarly updated.

Checklist for PR authors (skip items if you don't have permissions or they are not applicable)

  • added entry in CHANGES.rst within the relevant release section
  • updated or added relevant tests
  • updated relevant documentation
  • added relevant milestone
  • added relevant label(s)
  • ran regression tests, post a link to the Jenkins job below.
    How to run regression tests on a PR
  • All comments are resolved
  • Make sure the JIRA ticket is resolved properly

@melanieclarke melanieclarke added this to the Build 11.0 milestone May 23, 2024
melanieclarke
melanieclarke commented May 23, 2024
View reviewed changes
jwst/pixel_replace/pixel_replace.py
# the DO_NOT_USE flag
if ((indq[yindx[ii], xindx[ii]] & self.DO_NOT_USE)
and not (indq[yindx[ii], xindx[ii]] & self.NON_SCIENCE)):
newdq[yindx[ii], xindx[ii]] -= self.DO_NOT_USE
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I modified this section to preserve any original flags in the replaced pixel, instead of replacing them with FLUX_ESTIMATED only, to bring this algorithm in line with what fit_profile does.

@stscijgbot-jp stscijgbot-jp mentioned this pull request May 23, 2024
Closed
@melanieclarke
Copy link
Collaborator Author
melanieclarke commented May 23, 2024 •
edited
Loading

Regression tests started here:
https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1468/

But we should re-run them once #8409 is resolved. I will leave this PR at draft until that's done.

Edited to add:

Results from this run look right, so far. Only MIRI LRS is affected, since it is the only one that has pixel_replace on by default.

  • There are differences in the errors and variances in products after pixel_replace. Mostly, the new values replace NaNs, but the errors for the miri_lrs_slit cal file were not NaN for DO_NOT_USE pixels before pixel replacement, so they just have new values replacing the old values.
  • There are slight differences in the flux for the s2d file and x1d file. This is expected, since the read noise variance was updated, and it's used to weight resampling by default.

@codecov Codecov
Copy link
codecov bot commented May 23, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 81.35593% with 22 lines in your changes missing coverage. Please review.

Project coverage is 58.56%. Comparing base (b7e0b10) to head (afd75d6).
Report is 317 commits behind head on master.

Files with missing lines Patch % Lines
jwst/pixel_replace/pixel_replace.py 81.35% 22 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #8504      +/-   ##
==========================================
+ Coverage   58.02%   58.56%   +0.54%     
==========================================
  Files         388      388              
  Lines       38977    39030      +53     
==========================================
+ Hits        22617    22859     +242     
+ Misses      16360    16171     -189

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@melanieclarke
Copy link
Collaborator Author

Merged in changes from #8409. New regression tests started here:

https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1486/

@melanieclarke
Copy link
Collaborator Author

Regression tests look good to me:

  • As before, for steps using the fit_profile method, there are changes to the error and variance for replaced pixels in products downstream of pixel_replace.
  • There are also small changed to flux for s2d products and the x1d products derived from them, because of changes to the read noise variances used for weighting.
  • For steps using the mingrad algorithm, there are changes to the DQ image to preserve existing flags, and to the variance components, but the error is unchanged.

I think this is ready for review now; I will take it out of draft.

@melanieclarke melanieclarke marked this pull request as ready for review May 30, 2024 18:38
@melanieclarke melanieclarke requested a review from a team as a code owner May 30, 2024 18:38
@melanieclarke melanieclarke requested review from drlaw1558 and nden May 30, 2024 18:45
@drlaw1558
Copy link
Collaborator

Just retested with the merged-in changed from #8409 , and results look good to me.

hbushouse
hbushouse approved these changes Jun 11, 2024
View reviewed changes
Copy link
Collaborator
@hbushouse hbushouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hbushouse hbushouse merged commit cdadd8d into spacetelescope:master Jun 11, 2024
28 checks passed
@melanieclarke melanieclarke deleted the jp-3447 branch June 27, 2024 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drlaw1558 drlaw1558 drlaw1558 approved these changes

@hbushouse hbushouse hbushouse approved these changes

@nden nden Awaiting requested review from nden

Assignees
No one assigned
Labels
documentation MIRI NIRSPEC pipeline pixel replacement spec2 pipeline spec3 pipeline testing tso3 pipeline
Projects
None yet
Milestone
Build 11.0
Development

Successfully merging this pull request may close these issues.

Errors need to be interpolated in pixel_replace fit_profile code
3 participants
@melanieclarke @drlaw1558 @hbushouse
0