Skip to content
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

Update refpix step for NIR detectors to use convolution kernel #8482

Open
stscijgbot-jp opened this issue May 13, 2024 · 11 comments
Open

Update refpix step for NIR detectors to use convolution kernel #8482

stscijgbot-jp opened this issue May 13, 2024 · 11 comments

Comments

@stscijgbot-jp
Copy link
Collaborator

stscijgbot-jp commented May 13, 2024

Issue JP-3622 was created on JIRA by David Law:

As part of an ongoing multi-instrument discussion of ways in which to improve 1/f noise in NIR data (see https://outerspace.stsci.edu/display/JWSTCC/JWST+Pipeline+1-over-f+Noise+Removal]) there was general agreement on a desired improvement to the refpix step which should allow reference pixels to mitigate 1/f noise as much as possible without the need for using science pixels.  In brief, instead of the side-reference pixel subtraction using a simple median smoothed signal as a function of row, it should instead use an optimized convolution kernel to do the smoothing prior to subtraction.  Algorithm and example implementation provided by Timothy Brandt (in collaboration with Bernie Rauscher).

The impact of this change on rate files is quite modest, improving the total noise by a few percent.  Likewise, there is not a large impact on the visible residual 1/f striping.  Nonetheless, this change allows us to maximize the gains that can be realized without science-specific solutions.

Details:

  • This new method requires convolution of the side reference pixels by a realspace kernel.  These kernels should be provided as CRDS reference files to allow for future flexibility in changing the kernels if necessary.
  • The new method should be added as a new (and default) option, but the simple-median kernel should also be kept as a user-selectable option as well.
  • This method is only relevant to non-IRS2 readout modes, as IRS2 readout patterns already implement something similar on board.

A notebook is attached that demonstrates the use of this approach in a custom post-hook step appended to the refpix step (passing in the kernels for convenience as global variables- these would come from reference files instead).

This ticket will require multiple steps to implement: defining a new CRDS reference file format, implementing the necessary code changes, creating and delivering the new reference files for each NIR instrument, and potentially updating parameter reference files.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by David Law on JIRA:

Just adding a note that, since this change would involve creation of new reference files, it's probably too late to work this with a target build of 11.1.  We'd perhaps be looking more at 11.2 instead.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Maria Pena-Guerrero on JIRA:

The attached notebook uses a file containing the Fourier coefficients, can that file please be attached here as well?

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Maria Pena-Guerrero on JIRA:

I am assuming all instruments can/will provide their coefficients in the same format, is this correct? 

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Timothy Brandt on JIRA:

I have attached the reference file provided by Bernie Rauscher.  I believe that all H2RG detectors can/will provide coefficients in the same format.

@stscijgbot-jp
Copy link
Collaborator Author

stscijgbot-jp commented Aug 23, 2024

Comment by Maria Pena-Guerrero on JIRA:

Reference file question:

Please check if the following structure woks for reference file (sample file [^nrc_conv_kernel.asdf]). Example for a NIRCam ASDF tree:
root (AsdfObject)
├─nrca1 (dict)
│ ├─gamma (ndarray): shape=(4, 1025), dtype=complex64
│ └─zeta (ndarray): shape=(4, 1025), dtype=complex64
├─nrca2 (dict)
│ ├─gamma (ndarray): shape=(4, 1025), dtype=complex64
│ └─zeta (ndarray): shape=(4, 1025), dtype=complex64
├─nrca3 (dict)
│ ├─gamma (ndarray): shape=(4, 1025), dtype=complex64
│ └─zeta (ndarray): shape=(4, 1025), dtype=complex64
├─nrca4 (dict)
│ ├─gamma (ndarray): shape=(4, 1025), dtype=complex64
│ └─zeta (ndarray): shape=(4, 1025), dtype=complex64
├─nrcb1 (dict)
│ ├─gamma (ndarray): shape=(4, 1025), dtype=complex64
│ └─zeta (ndarray): shape=(4, 1025), dtype=complex64
├─nrcb2 (dict)
│ ├─gamma (ndarray): shape=(4, 1025), dtype=complex64
│ └─zeta (ndarray): shape=(4, 1025), dtype=complex64
├─nrcb3 (dict)
│ ├─gamma (ndarray): shape=(4, 1025), dtype=complex64
│ └─zeta (ndarray): shape=(4, 1025), dtype=complex64
└─nrcb4 (dict)
  ├─gamma (ndarray): shape=(4, 1025), dtype=complex64
  └─zeta (ndarray): shape=(4, 1025), dtype=complex64

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Maria Pena-Guerrero on JIRA:

another question, is the convolution only to be applied for full frame data?

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Timothy Brandt on JIRA:

Maria Pena-Guerrero: that is correct; this is only to be applied to full frame data.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Maria Pena-Guerrero on JIRA:

David Law we need a response regarding the format of the reference file for all the instruments. Please see comment 22/Aug/24 10:06 AM

 

The branch of the datamodels repo is:

spacetelescope/stdatamodels#321

The branch of the jwst repo is:

#8726

 

Please go ahead and test the code. Try the following:

pip install git+[https://github.com/penaguerrero/jwst@jp3622]

pip install git+https://github.com/penaguerrero/stdatamodels@ conv_kernel_model

 

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Maria Pena-Guerrero on JIRA:

David Law we are pushing this ticket to the next build since there is still quite a bit of testing and coordination required between the jwst, stdatamodels, and CRDS for the changes in this ticket to take effect, especially since the changes are to be applied by default.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by David Law on JIRA:

Maria Pena-Guerrero Agreed, this will need to be dealt with early in a build to get the CRDS changes in place.  Will look back at this once 11.1 is done.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Maria Pena-Guerrero on JIRA:

David Law is this a good time to take up this ticket again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant