jwst
jwst copied to clipboard
spacetelescope
Raise the default sigma for jump detection
Issue JP-1843 was created on JIRA by Alicia Canipe:
INS is reporting that the jump detection step is flagging too many pixels using the default value. We should raise this value to... TBD? Provide feedback in the comments. We probably don't need individual instrument team parameter files for this.
Misty Cracraft David Law Sarah Kendrew
Bryan Hilbert Nikolay Nikolov Julien Girard Ben Sunnquist
Kevin Volk Jo Taylor Nestor Espinoza
Marcio Melendez Hernandez Shannon Osborne
James Muzerolle Maria Pena-Guerrero
Comment by Marcio Melendez Hernandez on JIRA:
WFS&C Feedback
We have had this issue in the past with a few threaded discussions in Slack. In some cases, depending on the observing strategy, we have raise this value as high as 100 to minimized the number of flagged pixels. Note that even at 100 we don't get rid of all the jump flagged pixels. Many of our observing programs also have dither observations, thus, we can get rid of some flagged pixels that way as well. Overall, a threshold value between 25-100 have worked for us in the past.
https://stsci.slack.com/archives/CBQQMLA7Q/p1574793395113000
Comment by Bryan Hilbert on JIRA:
I've created some simulated data for NIRCam by manually adding jumps with magnitudes from 1-1000DN to a ground testing dark where I've tried to remove all of the real CRs. Playing with the simga rejection threshold, it seems like values in the high 50s to low 60s seem to give the best results of flagging real jumps while not flagging too much noise. I plan to dig into this a little deeper, but I wanted to share my initial impressions.
I've attached a plot of my results so far. I added ~6800 jumps to a 15-group full frame exposure. The jump magnitudes rise smoothly from 1-1000, so there are a number of jumps that are smaller than the noise and will never be found by the jump step. The top panel shows the number of correctly flagged jumps. The middle panel shows the number of added jumps that were not flagged, and the bottom panel shows the number of false positives, where no jump was added but the jump step flagged the pixel anyway.
I will add that for the jwst package regression tests, we typically use values of ~50 to ~200 sigma for jump detection threshold, so that it does not scrape up lots of noise. And we have to do this for all the instruments - or certainly for NIRCam, NIRSpec and MIRI.
$ grep -r rejection_threshold jwst/regtest
jwst/regtest/test_nirspec_irs2_detector1.py: "--steps.jump.rejection_threshold=200",
jwst/regtest/test_miri_image.py: "--steps.jump.rejection_threshold=200",
jwst/regtest/test_nircam_image.py: "--steps.jump.rejection_threshold=50.0",
jwst/regtest/test_miri_lrs_slitless.py: "--steps.jump.rejection_threshold=150",
I suspect it would be the same for others if we checked. Clearly, the meaning of "sigma" here is not physical.
More slack discussion a year ago here:
https://stsci.slack.com/archives/CBQQMLA7Q/p1579899143018900
I suspect this is a bug somewhere in jump step.
Comment by James Muzerolle on JIRA:
I agree with James Davies [X], it looks like there is something amok with the estimation of sigma here. Previous analyses done by the NIRSpec IDT on dark data with injected CRs have indicated optimal results using a threshold around ~4 times the CDS (read) noise.
Comment by Michael Regan on JIRA:
I've tested the noise thresholds in the unit test with both read noise limited ramps and Poisson noise limited ramps and the threshold of flagging matches the predictions. The most likely problem is either in the read noise or gain reference files.
The read noise reference files should be CDS read noise in units of DN.
The gain reference file should be in units of electrons per DN.
All the noise calculations in the step are done in electrons to be able to cleanly calculate the Poisson noise.
Have you confirmed the noise generation is correct? I found a couple of bugs in the way MIRISIM was calculating Poisson noise that led to incorrect effective noise and thus over-flagging of the ramps.
Comment by Michael Regan on JIRA:
Bryan pointed me to the files he used and my quick analysis says that the simulated data has a read noise that is 16 times above the reference file value. That higher than expected noise would require raising the threshold from 4 sigma to 66 sigma which is about what we see.
Comment by Karl Gordon on JIRA:
This sounds like a reference file issue. Can this be confirmed and we can close this ticket?
Comment by Karl Gordon on JIRA:
Raising the sigma factor threshold for this step is a solution, but not a good one.
This step assumes that the ramp is linear and that the only significant noise sources are read noise and photon noise. For simulations, this should be the case. If the ramp is non-linear and this has not been corrected/fully corrected, then this step will flag the non-linear portions of the ramp as outliers.
In my mind, this step is one of the "canary in the coal mine" steps. This step is a good detector of ramp jumps and non-linear ramps - hence if there are problems earlier in the pipeline, then we will know about it due to this step detecting more than the expected number of ramp jumps.
Comment by Bryan Hilbert on JIRA:
Don't forget contributions from 1/f noise, which are present in simulated data from Mirage and pynrc, and are not completely removed by the reference pixel correction step. Also, the NIRCam team found that dark subtraction in the pipeline was increasing the noise in our ramps, because we didn't get enough dark exposures in ground testing to be able to get a decent signal-to-noise in our (SW channel) dark reference files. We have delivered new dark reference files full of zeros to address this issue, but in general I wonder about noise potentially added by reference files in steps prior to the jump step.
Just to update on my testing progress, for my final test I created some simulated data using pynrc, which made for clean ramps with no jumps. (Mirage begins with a ground testing dark, which has its own set of "unknown" jumps prior me adding my set of jumps for this testing. Avoiding these unknown jumps was my motivation for switching to pynrc.) While creating a synthetic ramp, pynrc adds 1/f noise, ktc noise, correlated and uncorrelated pink noise, the even/odd column offsets, amplifier-dependent bias offsets, etc. I then added my set of jumps to the ramp. When I repeated my test using the pynrc data, threshold values of ~6-sigma seemed to do a good job of flagging jumps without digging into the noise.
Comment by Karl Gordon on JIRA:
Very useful information Bryan Hilbert.
I agree that if there are other significant noise sources other than read noise and photon noise, then this step will have too many false positives. It would be straightforward to add other noise sources to this algorithm - if they are known (usually from reference files) and propagated to this step. Understanding the covariance of additional noise sources between frames for the same pixel would be very useful in how to formulate the model for the expected (no ramp jump) case.
You list quite a few additional noise sources. But I'm encouraged that a small change in the sigma fraction (6 sigma) is all that is needed. This implies that the read noise and photon noise are likely the dominate noise sources.
No problem with changing the threshold to something like 6. Needing to change it much larger numbers (25-100) is a red flag to me.
I am (naively) assuming that the value of the threshold is set with a parameter reference file and, so, changing it to something like 6 would be straightforward. And customizable by instrument/mode. Right?
Comment by Bryan Hilbert on JIRA:
Karl Gordon currently the threshold value can only be set as an attribute of the pipeline/step class. I don't think parameter reference files exist yet for this step, but Alicia Canipe can confirm. I think having a parameter reference file would make sense in this case.
I should have mentioned in my comment above: I uploaded an updated set of plots showing how well the jump step did for various thresholds.
Comment by Misty Cracraft on JIRA:
The members of the DMSWG have not yet created a parameter reference file for the jump step.
Comment by Alicia Canipe on JIRA:
Right – I think Michael Regan commented that we might not need parameter files per instrument for jump detection, but if the teams think we do, then it's fine to deliver some. Otherwise, SCSB is migrating all config files to parameter files so the jump detection step would have a default parameter file containing the threshold that INS decides is best as a default value.
Comment by Howard Bushouse on JIRA:
Technically the uber-default value for the detection threshold lives in the jump step code module itself, which is used if/when the user specifies nothing at all via either a .cfg or param ref file. So if you ever want the uber-default changed, that will need to be done in the code. But param ref files can be used to set different defaults on a per instrument or per observing mode basis.
Comment by Michael Regan on JIRA:
I have worked on several different datasets that had "too many jumps detected". In every case the problem was either that the input data did not have noise characteristics that match the reference file or there where more jumps in the input data than the user realized were in there. This step requires the input file and the reference files to match. A 50% error in the read noise, gain, or Poisson noise will throw it off. It has been testing at both the read noise limit and the Poisson noise limit and finds jumps at the correct input threshold. The flagging is a pretty simple process. There can always be a bug but it would have to be an edge case. The baseline cases are well covered with the unit tests. For this ticket to make any progress the user should provide a given ramp that they believe has an incorrect jump flagged, and the read noise and gain reference files used when running the pipeline. Otherwise, it is impossible to debug.
Comment by Michael Regan on JIRA:
We will need to have a parameter reference file for this step that uses different thresholds for flagging pixel neighbors. This is because the IPC is different between the NIR and MIRI detectors. The higher IPC with MIRI means that we need to use different thresholds for the lower and upper limits for flagging neighbors.
Comment by Michael Regan on JIRA:
To Bryan's comment about other noise sources. Most of the ones listed will not affect the jump detection step. For example, the KTC noise is just a variation in the starting DN level. It only affects the y-intercept of the fit. Also, the 1/f noise is in the calibration file that was used to calculate the read noise. So, it is included in the "read noise".
Formally, the read noise value used in the jump step should be the effective noise in the input file. Therefore, the read noise should be calculated from a file that has been processed by the pipeline up to the point of the jump step. Any noise added in by low S/N reference files in prior steps would then be accounted for. Changing the threshold sigma is a workaround not a solution.
Note that it is important for more than just the jump step for the read noise to correctly include any noise introduced by the pipeline. The uncertainties determined by the ramp fitting step need to have the effective read noise value rather the theoretical lower limit. Otherwise, the uncertainty calculation will be incorrect.
Comment by Howard Bushouse on JIRA:
I notice that this ticket has been assigned the priority "calwg_critical" and I was told recently that the assignment of any "calwg_*" priority level is to be used as an indication that the ticket is "shovel ready" for SCSB to work on. But in this case it appears to me that no consensus has yet been reached on what it is we're supposed to do to address the problem. Maybe I misunderstood the info related to calwg assignments. Any enlightenment is appreciated.
Comment by Alicia Canipe on JIRA:
I removed the "calwg_critical" label to get it off the SCSB work dashboard, since I think at least a couple of the teams will prefer to submit parameter reference files. I'm going to leave this ticket open for a bit while we get it sorted out.
Comment by Anton Koekemoer on JIRA:
adding this back to the cal wg dashboard, by giving it a priority of "calwg_high" (not critical) - at least to keep it on our radar in terms of calwg work (and it likely had impact to other instruments beyond NIRCam), since we did discuss this and agree that necessary work was needed, but the work is currently mostly on the instrument teams to examine the detector characteristics during commissioning, and produce/ update parameters accordingly (and not so much on SCSB, at least not for the moment). Hope this helps clarify things.
Comment by Marcio Melendez Hernandez on JIRA:
WFS&C Feedback
We have had this issue in the past with a few threaded discussions in Slack. In some cases, depending on the observing strategy, we have raise this value as high as 100 to minimized the number of flagged pixels. Note that even at 100 we don't get rid of all the jump flagged pixels. Many of our observing programs also have dither observations, thus, we can get rid of some flagged pixels that way as well. Overall, a threshold value between 25-100 have worked for us in the past.
https://stsci.slack.com/archives/CBQQMLA7Q/p1574793395113000
Comment by Bryan Hilbert on JIRA:
I've created some simulated data for NIRCam by manually adding jumps with magnitudes from 1-1000DN to a ground testing dark where I've tried to remove all of the real CRs. Playing with the simga rejection threshold, it seems like values in the high 50s to low 60s seem to give the best results of flagging real jumps while not flagging too much noise. I plan to dig into this a little deeper, but I wanted to share my initial impressions.
I've attached a plot of my results so far. I added ~6800 jumps to a 15-group full frame exposure. The jump magnitudes rise smoothly from 1-1000, so there are a number of jumps that are smaller than the noise and will never be found by the jump step. The top panel shows the number of correctly flagged jumps. The middle panel shows the number of added jumps that were not flagged, and the bottom panel shows the number of false positives, where no jump was added but the jump step flagged the pixel anyway.
Comment by James Muzerolle on JIRA:
I agree with James Davies [X], it looks like there is something amok with the estimation of sigma here. Previous analyses done by the NIRSpec IDT on dark data with injected CRs have indicated optimal results using a threshold around ~4 times the CDS (read) noise.
Comment by Michael Regan on JIRA:
I've tested the noise thresholds in the unit test with both read noise limited ramps and Poisson noise limited ramps and the threshold of flagging matches the predictions. The most likely problem is either in the read noise or gain reference files.
The read noise reference files should be CDS read noise in units of DN.
The gain reference file should be in units of electrons per DN.
All the noise calculations in the step are done in electrons to be able to cleanly calculate the Poisson noise.
Have you confirmed the noise generation is correct? I found a couple of bugs in the way MIRISIM was calculating Poisson noise that led to incorrect effective noise and thus over-flagging of the ramps.
Comment by Michael Regan on JIRA:
Bryan pointed me to the files he used and my quick analysis says that the simulated data has a read noise that is 16 times above the reference file value. That higher than expected noise would require raising the threshold from 4 sigma to 66 sigma which is about what we see.
Comment by Karl Gordon on JIRA:
This sounds like a reference file issue. Can this be confirmed and we can close this ticket?
Comment by Karl Gordon on JIRA:
Raising the sigma factor threshold for this step is a solution, but not a good one.
This step assumes that the ramp is linear and that the only significant noise sources are read noise and photon noise. For simulations, this should be the case. If the ramp is non-linear and this has not been corrected/fully corrected, then this step will flag the non-linear portions of the ramp as outliers.
In my mind, this step is one of the "canary in the coal mine" steps. This step is a good detector of ramp jumps and non-linear ramps - hence if there are problems earlier in the pipeline, then we will know about it due to this step detecting more than the expected number of ramp jumps.
Comment by Bryan Hilbert on JIRA:
Don't forget contributions from 1/f noise, which are present in simulated data from Mirage and pynrc, and are not completely removed by the reference pixel correction step. Also, the NIRCam team found that dark subtraction in the pipeline was increasing the noise in our ramps, because we didn't get enough dark exposures in ground testing to be able to get a decent signal-to-noise in our (SW channel) dark reference files. We have delivered new dark reference files full of zeros to address this issue, but in general I wonder about noise potentially added by reference files in steps prior to the jump step.
Just to update on my testing progress, for my final test I created some simulated data using pynrc, which made for clean ramps with no jumps. (Mirage begins with a ground testing dark, which has its own set of "unknown" jumps prior me adding my set of jumps for this testing. Avoiding these unknown jumps was my motivation for switching to pynrc.) While creating a synthetic ramp, pynrc adds 1/f noise, ktc noise, correlated and uncorrelated pink noise, the even/odd column offsets, amplifier-dependent bias offsets, etc. I then added my set of jumps to the ramp. When I repeated my test using the pynrc data, threshold values of ~6-sigma seemed to do a good job of flagging jumps without digging into the noise.
Comment by Karl Gordon on JIRA:
Very useful information Bryan Hilbert.
I agree that if there are other significant noise sources other than read noise and photon noise, then this step will have too many false positives. It would be straightforward to add other noise sources to this algorithm - if they are known (usually from reference files) and propagated to this step. Understanding the covariance of additional noise sources between frames for the same pixel would be very useful in how to formulate the model for the expected (no ramp jump) case.
You list quite a few additional noise sources. But I'm encouraged that a small change in the sigma fraction (6 sigma) is all that is needed. This implies that the read noise and photon noise are likely the dominate noise sources.
No problem with changing the threshold to something like 6. Needing to change it much larger numbers (25-100) is a red flag to me.
I am (naively) assuming that the value of the threshold is set with a parameter reference file and, so, changing it to something like 6 would be straightforward. And customizable by instrument/mode. Right?
Comment by Bryan Hilbert on JIRA:
Karl Gordon currently the threshold value can only be set as an attribute of the pipeline/step class. I don't think parameter reference files exist yet for this step, but Alicia Canipe can confirm. I think having a parameter reference file would make sense in this case.
I should have mentioned in my comment above: I uploaded an updated set of plots showing how well the jump step did for various thresholds.
Comment by Misty Cracraft on JIRA:
The members of the DMSWG have not yet created a parameter reference file for the jump step.
Comment by Alicia Canipe on JIRA:
Right – I think Michael Regan commented that we might not need parameter files per instrument for jump detection, but if the teams think we do, then it's fine to deliver some. Otherwise, SCSB is migrating all config files to parameter files so the jump detection step would have a default parameter file containing the threshold that INS decides is best as a default value.
Comment by Howard Bushouse on JIRA:
Technically the uber-default value for the detection threshold lives in the jump step code module itself, which is used if/when the user specifies nothing at all via either a .cfg or param ref file. So if you ever want the uber-default changed, that will need to be done in the code. But param ref files can be used to set different defaults on a per instrument or per observing mode basis.
Comment by Michael Regan on JIRA:
I have worked on several different datasets that had "too many jumps detected". In every case the problem was either that the input data did not have noise characteristics that match the reference file or there where more jumps in the input data than the user realized were in there. This step requires the input file and the reference files to match. A 50% error in the read noise, gain, or Poisson noise will throw it off. It has been testing at both the read noise limit and the Poisson noise limit and finds jumps at the correct input threshold. The flagging is a pretty simple process. There can always be a bug but it would have to be an edge case. The baseline cases are well covered with the unit tests. For this ticket to make any progress the user should provide a given ramp that they believe has an incorrect jump flagged, and the read noise and gain reference files used when running the pipeline. Otherwise, it is impossible to debug.
Comment by Michael Regan on JIRA:
We will need to have a parameter reference file for this step that uses different thresholds for flagging pixel neighbors. This is because the IPC is different between the NIR and MIRI detectors. The higher IPC with MIRI means that we need to use different thresholds for the lower and upper limits for flagging neighbors.
Comment by Michael Regan on JIRA:
To Bryan's comment about other noise sources. Most of the ones listed will not affect the jump detection step. For example, the KTC noise is just a variation in the starting DN level. It only affects the y-intercept of the fit. Also, the 1/f noise is in the calibration file that was used to calculate the read noise. So, it is included in the "read noise".
Formally, the read noise value used in the jump step should be the effective noise in the input file. Therefore, the read noise should be calculated from a file that has been processed by the pipeline up to the point of the jump step. Any noise added in by low S/N reference files in prior steps would then be accounted for. Changing the threshold sigma is a workaround not a solution.
Note that it is important for more than just the jump step for the read noise to correctly include any noise introduced by the pipeline. The uncertainties determined by the ramp fitting step need to have the effective read noise value rather the theoretical lower limit. Otherwise, the uncertainty calculation will be incorrect.
Comment by Howard Bushouse on JIRA:
I notice that this ticket has been assigned the priority "calwg_critical" and I was told recently that the assignment of any "calwg_*" priority level is to be used as an indication that the ticket is "shovel ready" for SCSB to work on. But in this case it appears to me that no consensus has yet been reached on what it is we're supposed to do to address the problem. Maybe I misunderstood the info related to calwg assignments. Any enlightenment is appreciated.
Comment by Alicia Canipe on JIRA:
I removed the "calwg_critical" label to get it off the SCSB work dashboard, since I think at least a couple of the teams will prefer to submit parameter reference files. I'm going to leave this ticket open for a bit while we get it sorted out.
Comment by Anton Koekemoer on JIRA:
adding this back to the cal wg dashboard, by giving it a priority of "calwg_high" (not critical) - at least to keep it on our radar in terms of calwg work (and it likely had impact to other instruments beyond NIRCam), since we did discuss this and agree that necessary work was needed, but the work is currently mostly on the instrument teams to examine the detector characteristics during commissioning, and produce/ update parameters accordingly (and not so much on SCSB, at least not for the moment). Hope this helps clarify things.
Comment by Michael Regan on JIRA:
Going to 50 sigma doesn't make sense. Almost every time there is an over flagging of jumps it is related to an incorrect reference file. Errors in the read noise or gain lead to errors in the jump detection.
Agreed Mike. Those high sigma values were for mirage produced simulations, and clearly there were issues in how the noise was modeled from the reference files. More reasonable ~5 sigma thresholds work just fine for jump detection on flight data for all the detectors from my experience.
Comment by Misty Cracraft on JIRA:
It sounds like this was more of an issue for simulated data and not currently an issue for flight data. Is this something we need to discuss further? This has calwg_high, but I'm not sure it warrants that any longer. This should be re-evaluated with current data and determine whether the default jump parameter value is still causing massive overflagging.
Comment by Anton Koekemoer on JIRA:
I propose closing this ticket - the original issue was high priority but that was before launch and it seems this is since resolved.
If the original reporter of the ticket (Alicia Canipe) agrees, then please feel free to close this ticket.