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

IRS2 intermittently bad pixels algorithm fix #7805

Closed
stscijgbot-jp opened this issue Aug 10, 2023 · 15 comments · Fixed by #7745
Closed

IRS2 intermittently bad pixels algorithm fix #7805

stscijgbot-jp opened this issue Aug 10, 2023 · 15 comments · Fixed by #7745

Comments

@stscijgbot-jp
Copy link
Collaborator

Issue JP-3321 was created on JIRA by Maria Pena-Guerrero:

This ticket is related to JP-3157. The algorithm is a dynamic check on the reference pixel values. The previous overcorrection mitigation factor is turned into the number of sigmas away from the mean (more intuitive quantity). This is what the algorithm does:

Loop through the reference pixel sequences and calculates per column the mean, standard deviation, and differences between even and odd, and store these in their corresponding array.

Calculate the means and standard deviations for each of these 3 arrays

Flag anything  >  mean_of_array + FACTOR * standard_deviation_of_array

Replace these flagged (suspicious) pixels with the nearest good ref pix

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Maria Pena-Guerrero on JIRA:

This is being addressed in PR 7745.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Howard Bushouse on JIRA:

Fixed by #7745

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Melanie Clarke on JIRA:

Reducing jw01386013001_03102_00002_nrs2_rate.fits with v1.12.1 and default parameters and comparing to the MAST version, reduced with v1.11.4, I see small changes in the regions originally pointed out in JP-3157 (see attached), but I don't think this is a cause for concern.  The change to the parameter definition looks like it is working as intended.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Howard Bushouse on JIRA:

Melanie Clarke Are your testing results sufficiently satisfactory to declare this ticket closed?

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Melanie Clarke on JIRA:

Howard Bushouse - I think so.  I want to check with the NIRSpec detector experts about whether the implementation satisfactorily addresses the problem, and if we might want to tune the input parameters, but we can file new tickets if needed.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Melanie Clarke on JIRA:

We've received a helpdesk question about a new artifact, introduced with v1.12.x, that seems to be related to this change.  There are several new bad rows in the rate file that did not appear in v1.11.4.  

It appears to be because of this change in the refpix step, in the clobber_ref function, in PR-7745:

data[..., ref:ref + 2] = 0.

was changed to:

data[..., ref:ref + 4] = 0.

effectively removing two extra reference pixels near a flagged reference pixel.  With this line restored to the previous version, the bad rows do not appear.

Helpdesk ticket is INC0193805, test data is jw01810002001_07101_00001_nrs1_uncal.fits.

Maria Pena-Guerrero - why was this line changed? It doesn't seem to be noted in either the change notes or the commit logs.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Maria Pena-Guerrero on JIRA:

This change occurred because the flagged tax pixels in the reference file were not all removed, once the change was made from 2 to 4, all the bad pixel rows were being removed. It is explained in the ESA document that the 2 interleaved pixels for IRS2 are each read twice consecutively, hence the change from 2 to 4.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Melanie Clarke on JIRA:

Okay, thanks for clarifying.

I don't think that's a correct interpretation of the text, though.  Each pixel is read twice, so there should be only two readouts associated with the bad flag. 

I'm attaching an image of a section of jw01810002001_07101_00001_nrs1_uncal.fits, before and after the clobber function.  Top image is before correction, middle is with 2 pixels clobbered, bottom is with 4 (current implementation).  After the change, the clobber function sets science pixels to zero, not just the interleaved reference pixels.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Melanie Clarke on JIRA:

I sent a PR to address this issue, reverting the line in the clobber function to the previous version: #8005

If it is possible, it would be helpful to get this patched right away, since the effect on the data is significant, and the change is small.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Melanie Clarke on JIRA:

Alicia Canipe Howard Bushouse - The bug fix discussed above should go in as a patch to Build 10.0 before it goes to production if at all possible.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Katie Kaleida on JIRA:

We will start the process of preparing a DMS 10.0.1 patch to correct this issue.  Can you confirm what data is affected by this issue?  Is it all data taken in the nrs1 detector and the irs2 readout pattern?  Melanie Clarke?

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Melanie Clarke on JIRA:

It is all data taken in the irs2 readout pattern, but the effects are most obvious and harmful in NRS1 data.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Howard Bushouse on JIRA:

For the purpose of providing suitable tracking for a potential B10.0.1 patch containing this latest fix, it would be best to have a separate JP ticket to document this one issue that's been fixed by #⁠8005. Can you create that Melanie Clarke ?

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Melanie Clarke on JIRA:

Howard Bushouse - The separate ticket is JP-3440.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Melanie Clarke on JIRA:

Howard Bushouse - I think this ticket should be closed now.  Remaining issues were resolved in JP-3440.

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

Successfully merging a pull request may close this issue.

1 participant