spacetelescope / jwst

Python library for science observations from the James Webb Space Telescope
https://jwst-pipeline.readthedocs.io/en/latest/
Other
568 stars 167 forks source link

Issues with jump detection in NIRISS images #7771

Closed stscijgbot-jp closed 10 months ago

stscijgbot-jp commented 1 year ago

Issue JP-2625 was created on JIRA by Kevin Volk:

An issue has come up between April 26 and May 26. Running a set of dithered imaging exposures for NIRISS through the pipeline shows that a lot of pixels are now given zero slope because of having lots of jumps flagged in the ramps, which was not the case before. This has something to do with the charge migration fix done on 5 May but probably also has something to do with new reference files for NIRISS delivered in the meantime. Whatever the cause, something needs to be done about the jump flags when using the default parameters. Maybe one needs to just change the default threshold for the jump detection. The NIRISS read noise reference files were updated on May 11th, after the testing for the charge migration fix on May 5th. It is possible that this has affected the results. I request help from people who understand the jump step to figure out whether this is an issue with the code for small numbers of groups or whether one needs to change the default parameters to fix this problem. This is a high priority issue because it will affect the default pipeline products once the charge migration fix is in for the default processing (if it is not already the case).

stscijgbot-jp commented 1 year ago

Comment by Clare Shanahan on JIRA:

I looked into this, and this issue is unrelated to the changes in saturation for charge spilling. The culprits are 1. a pre-existing issue with jump detection not working well on sources and 2. a change to ramp fitting that now excludes groups flagged as jumps (rather than just saturation) when determining if there are enough 'good' groups to fit a ramp. 

 

 

!Screen Shot 2022-06-10 at 12.30.29 PM.png|width=507,height=186!

With the newest version of the pipeline (1.5.3), the centers of many stars appear to be fully saturated as they have many pixels with 0 slope. This wasn't the case in previous version before some of these changes. I looked at the jump and saturation DQ flags for each group. As you can see below, in both pipeline versions many of the star's pixels are flagged as jumps in almost all groups. The small differences between the jump flags in the 2 pipeline versions is due to 2 prs in stcal that impacted flagging of pixels that neighbor jumps. The changes in the saturation flags are as expected - previously 2 pixels were flaggged, now the bordering pixels are flagged giving 18. 

 

!Screen Shot 2022-06-10 at 1.00.43 PM.png|width=612,height=346!

 

!Screen Shot 2022-06-10 at 1.00.50 PM.png|width=615,height=345!

So, the reason that there are now so many 0 slope pixels is due to the change in ramp fitting now eliminating groups with jumps and being left with many more cases of no good groups to fit a ramp to. I think this is actually doing the right thing, the problem is that there are so many jumps being detected in the sources. Is this happening for other instruments as well? Do we want to change the thresholds for jump detection, or change the algorithm itself to better handle detecting jumps in bright sources?

 

I

 

stscijgbot-jp commented 1 year ago

Comment by Kevin Volk on JIRA:

The jump step is not doing the right thing because I looked at a few of the ramps and there are no jumps in the ramps as I see by eye even though the jump step is flagging things as jumps. For example for a moderately bright star where the central pixels have 4 samples prior to saturation the jump step should not flag 2 or 3 of the 4 samples as jumps. In the first place one is not going to have 2 or 3 cosmic rays in the same pixel within 4 frames of data. In the second place, I looked at a few of the pixel ramps and could see by eye that there were no jumps present. Something is failing in the jump detection algorithm if given 4 samples with moderate dispersion in the CDS counts the step flags 3 of the 4 as outliers at the 3 sigma level. I do not know if the jump step uses the uncertainties in the calculations. If it does, one possibility is that the noise is being severely underestimated for some reason. But my general impression is that the noise values reported by the pipeline are actually a bit high not significantly low, at least in the rate images. If the sigma values for the rejection are calculated from the samples themselves then one should never get this type of result where most or all the samples are flagged as jumps in a single ramp. Nor should the jumps be correlated with the stellar signal in the way that is being seen here.

stscijgbot-jp commented 1 year ago

Comment by Swara Ravindranath on JIRA:

Has there been any fix to this issue after Kevin Volk reported on 06/10/2022? The ticket workflow says it is "in testing". Just checking to see if there is a change that needs to be tested.

stscijgbot-jp commented 1 year ago

Comment by Michael Regan on JIRA:

Kevin,

  This will require some detailed work to figure out what is wrong. My starting guess is that observations are noisy than expected given the reference file values or the new flagging rules had unintended side effects. It kind of has to be that given that's how the code works. Claire's description is clear that we will get more groups flagged with the recent pipeline changes. The side effects of these changes can be unexpected but are consistent with the changes.

  This will require a good bit of time to work through the details. Are you sure this is really a problem?

stscijgbot-jp commented 1 year ago

Comment by Kevin Volk on JIRA:

Mike, this is specifically a problem for imaging where we cannot escape stars with cores that "saturate" after a small number of groups up the ramp. In turn this has serious consequences for WFSS mode where such objects give nice spectra but issues will arise because the direct images are bad around the stars and these images are needed for creating the source catalogue. We have not seen this type of issue (yet) in SOSS mode for example, but I worry that for some brightness range the same type of thing will start to appear in the SOSS rate images to the serious detriment of the TSO data. The original question I posed in making this issue is whether we can somehow figure out whether the issue is in the parameter reference file values, in the read noise reference file, or in the code itself. I do not understand things well enough to tackle that personally. It still seems wrong if a ramp of 5 samples gets 4 samples flagged as jumps no matter what the parameters are, but maybe there is a non-coding way to fix this. I do not have statistics on how bad this issue might be because I have not re-reduced data from different sky pointings with the newer pipeline, but it was a serious issue in the first case that I tried. My immediate worry is that when the commission data for NIRISS is reprocessed with the other on-sky revised reference files that such cases will be in every image and produce really bad outputs.

stscijgbot-jp commented 1 year ago

Comment by Clare Shanahan on JIRA:

I looked into this further and I'm fairly confident that jump detection is working as described, but the non-linear nature of the ramps of these bright pixels in the centers of sources is causing jumps to be flagged when there shouldn’t be. This in turn causes many 0.0 slope pixels to appear because groups with jumps (or that neighbor jumps) are being eliminated. As I mentioned above, some additional 0-slope pixels are to be expected because we are now flagging pixels adjacent to saturated pixels, but the recent change to also drop jump groups in ramp fitting revealed that there is an over-abundance of jumps where there probably shouldn't be.

 

When you look at these jump-flagged pixels in the center of sources individually, you’re seeing a lot of ramps like this:

!image-2022-06-23-17-17-51-588.png|width=406,height=265!

The ramps generally look non-linear (and curved up) with no obvious 'jumps', but most of the groups are flagged as jumps. The example above was generated with jump-neighbor flagging set to False so those are all ‘true’ jumps. In this case, this ramp has 4 good groups so it would have a non-zero slope fit. However, many of its neighboring pixels also look like this so those good groups drop out when neighbors are flagged. This scenario is happening a lot and is part of the reason there are now so many 0-slope pixels.

I also followed some of these ramps that look like this through twopoint difference code and they are indeed satisfying the criteria for being flagged as a jump. In this example the first differences of adjacent groups are

[384.48758  440.3114   455.4143   444.4325   582.4209   548.3413 628.96655  689.417   1542.5967],

and sigma, which uses the median of first differences as an estimate for poisson noise, as well as the read noise for this pixel, ends up being 26.36. The first differences minus the median difference in units of sigma exceeds the threshold for the last group, the process is repeated, and every group until group 4 is rightfully flagged as a jump. 

I played around with adjusting 'threshold' a bit and there might be a better choice that avoids flagging so many groups in a non-linear ramp. In this example, setting a threshold of 10.0 avoids all but the last group being flagged, which is what looks right to me by eye, but that is quite high. The two and three group thresholds would also need to be adjusted. The flagging of neighboring pixels also has a threshold level which could be adjusted upward (it only flags neighbors to jumps that are 'big enough', not for small jumps). 

I'm not sure what the solution is here, maybe it is at the linearity correction level, or perhaps how 'sigma' is calculated? Howard also mentioned adjusting the thresholds as a temporary solution. 

 

 

stscijgbot-jp commented 1 year ago

Comment by Kevin Volk on JIRA:

I am a bit confused here. In the example above while there are not "really" any jumps the ramp samples 5 to 9 are flagged as jumps because of the ramp curvature. OK, but these pixels should be getting charge from adjacent core pixels that saturate in say sample 5 so these should also be flagged has "saturated" and ignored anyway. If no pixels nearby actually saturate before sample 5, how can one get zero slopes out in pixels near the core? What does the statement "However, many of its neighboring pixels also look like this so those good groups drop out when neighbors are flagged. This scenario is happening a lot and is part of the reason there are now so many 0-slope pixels." mean in this context? Would this particular pixel get samples rejected because of "jumps" flagged in the immediately adjacent pixels. I thought it was saturation flags were the ones that propagated to adjacent pixels. Do jump flags also propagate to adjacent pixels?

I do not think the issue is the linearity correction. Too many pixels are being flagged, and this is not seen in the flat field exposures.

If there is a way to set the parameters for the step and avoid these issues that is sufficient. That was what I asked initially when I field the issue. In the example ramp the normal sigma values from the first difference signals are such that three of the first four samples are OK, and one can see that values 2, 3, and 4 on the ramp are all within 1 sigma just from the Poisson noise given the gain of about 1.6 electrons. That would argue that the threshold is not badly underestimated. I am not sure why the initial first difference is low compared to the second through fourth samples, but one would assume not due to either charge migration or non-linearity issues since it is the start of the ramp and the signal is low.

stscijgbot-jp commented 1 year ago

Comment by Clare Shanahan on JIRA:

"OK, but these pixels should be getting charge from adjacent core pixels that saturate in say sample 5 so these should also be flagged has "saturated" and ignored anyway. If no pixels nearby actually saturate before sample 5, how can one get zero slopes out in pixels near the core? "

There aren't any directly adjacent saturated pixels - if there were, they would be flagged and jump detection would not happen for those groups. There are probably saturated pixels a few pixels away. 

 

"What does the statement "However, many of its neighboring pixels also look like this so those good groups drop out when neighbors are flagged. This scenario is happening a lot and is part of the reason there are now so many 0-slope pixels." mean in this context? "

 

Sorry if I wasn't clear in my explanation, the crux of the issue I believe is that there are curved, nonlinear ramps near the center of stars, these are getting flagged as jumps (because they rightfully satisfy the criteria) and also impacting their neighbors, and as a result many pixels have a 0.0 slope due to a combination of jump+saturation flags which are prevalent in sources. 

In ramp fitting, a 0.0 slope can occur when gain or readnoise is 0 for that pixel, when all groups but one are flagged as saturated, when all groups but one are flagged as jump, or when a combination of jump+saturation flags causes there to only be one good group left . Pixels may be flagged as saturated or jump when they are inherently saturated pixels / a jump, or if they border a pixel that meets that criteria. I am seeing a LOT of non-linear ramps like this, where some fraction of the groups become flagged as jumps because of the non linearity. The neighbor flagging compounds this issue - these curved ramps are getting some fraction of their groups flagged as jumps, and their neighbors which also look like this are getting some fraction of their groups flagged as jumps, so there are many more instances where all groups get flagged and the slope is 0.0. You also have scenarios where, for example, groups 5+ are saturated (leaving 4 good groups to fit a ramp, initially) but an adjacent pixel with a non-linear ramp flags jumps in all but the first group, so both end up with all groups dropping out and a 0.0 ramp.

 

"Do jump flags also propagate to adjacent pixels?" 

Yes, if the jump is sufficiently large then 4 of the adjacent pixels will be flagged as jumps (if they're not already saturated or donotuse). There is a threshold that controls this (min_jump_to_flag_neighbors, max_jump_to_flag_neighbors). Saturation flags all 8 bordering pixels by default.

 

"If there is a way to set the parameters for the step and avoid these issues that is sufficient. "

This could be a solution, but when I looked into this you have to set the threshold VERY high (from 3 sigma to 10 sigma in this case) to avoid flagging jumps in these non-linear ramps. I thought the issue might lay in the linearity correction because if these ramps looked somewhat more linear, the threshold wouldn't have to be adjusted as much to avoid flagging jumps here but also continue flagging them when appropriate. 

stscijgbot-jp commented 1 year ago

Comment by Kevin Volk on JIRA:

OK thank you for the detailed explanation Clare. I think what we want is to prevent the jump flagging of adjacent pixels unless the jump magnitude is fairly high, say 10000 ADU for example. The adjacent pixel jump flagging is needed for large cosmic rays and such, but I would rather not worry about this for smaller cosmic rays in the hope that that will mitigate this immediate problem. As for setting the jump detection threshold higher, we can experiment with that and hope to get a compromise which leaves a mildly curved ramp alone but works for comic ray jumps at a somewhat higher threshold.

stscijgbot-jp commented 1 year ago

Comment by Paul Goudfrooij on JIRA:

I would like to add another issue related to jump detection that I noticed today. Let me first introduce the reason I'm investigating this, which is that we're getting strange results from the resample step in the image2 pipeline, namely that integrated fluxes of non-saturated stars are sometimes significantly smaller in the i2d images than in the cal images. Now when I switch resample.weight_type from the default 'ivm' to 'exptime' (i.e., equal weights for all pixels), then the aforementioned discrepancy between i2d and cal disappears. So I figured something might be amiss in the var_rnoise maps, which indeed show much higher read noise variance in the central pixels of the target star, and those same pixels also show "jump" flags (i.e., 4) in the DQ array. But the thing is, the ramp slopes of those pixels in the uncal file look just fine, without jumps, in every integration (see attached file ramps.png which shows the slopes of the peak pixel and the 4 nearest neighbors for all three integrations).  And when I re-run the Detector1 pipeline with jump.rejection_threshold = 1000, the pipeline log file states that "From highest outlier, two-point found 0 pixels with at least one CR ..." in all integrations, suggesting that zero pixels were flagged as jumps. But the output rate and rateints files are {}still flagging those pixels as jumps in their DQ arrays{}, which I don't understand. Could someone have a look at that? I have placed the various fits files and the pipeline log file of that last run of the pipeline (pipeline.log) in directory /user/goudfroo/issue_with_jump_step. The star is located at pixel (67, 69) in 0-indexed coordinates.

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

Paul Goudfrooij could you please document the issue with pixels containing jump flags even though the step says it didn't detect any in another ticket, just so we keep the investigations of the different issues separate? And of course supply some example files with particular pixels listed.

stscijgbot-jp commented 1 year ago

Comment by Paul Goudfrooij on JIRA:

OK Howard Bushouse - I created ticket JP-2666 for "my issue".

stscijgbot-jp commented 1 year ago

Comment by Tyler Pauly on JIRA:

Kevin Volk For what it's worth, a quick check on reproducibility does not show the results you provided in your May processing run. Have you tried a more up-to-date pipeline version? I've attached a figure for the cal data in your ppt from this morning, run through 1.6.3. [^jw01094001001_0211f_00001_nis_cal_figure.pdf]

stscijgbot-jp commented 1 year ago

Comment by Kevin Volk on JIRA:

The last version I tried checking this on was a snapshot version of the pipeline from 23 June.  I have a version from 15 August which I will try and see what happens.  It would be nice if the issue was gone....although if so then the discussion today was a waste of everyone's time.

 

stscijgbot-jp commented 1 year ago

Comment by Kevin Volk on JIRA:

Both dithers for this observation have the issue for the 15 August version of the pipeline.  That is the latest version that I have to test with.  Note that the worse case is jw01094001001_0211f_00002_nis_uncal.fits.

 

 

stscijgbot-jp commented 1 year ago

Comment by Tyler Pauly on JIRA:

I checked on the second dither position using pipeline 1.7.0 (might as well test on the most recent release), and I see

>>> ramp2 = datamodels.open('jw01094001001_0211f_00002_nis_ramp.fits')
>>> ramp2.groupdq[:,:,133,131]
array([[0, 4, 0],
       [0, 4, 0],
       [0, 4, 0]], dtype=uint8)
>>> ramp2.data[:,:,133,131]
array([[18773.22 , 35598.79 , 46585.977],
       [18880.877, 35502.375, 46651.99 ],
       [18804.934, 35416.645, 46626.277]], dtype=float32)
>>> cal2 = datamodels.open('jw01094001001_0211f_00002_nis_cal.fits')
>>> cal2.dq[133,131]
4
>>> cal2.data[133,131]
3754.625

I can attach the ramp and cal files here if you'd like. I'll attach a log stretch of the second dither cal data, at least. [^jw01094001001_0211f_00002_nis_cal_logstretch.pdf]

stscijgbot-jp commented 1 year ago

Comment by Kevin Volk on JIRA:

I will try the current version of the pipeline and see what the results are.  Right now I am doing tests of the ETC with the on-sky revisions to the NIRISS throughput files so I do not have time to install a new version of the pipeline today.  I should get exactly the same result since I am not doing anything unusual in the pipeline when I reduce these files.

 

 

stscijgbot-jp commented 1 year ago

Comment by Kevin Volk on JIRA:

I installed pipeline version 1.7.0 today and reduced the two dither positions for jw01094001001_0211f, and I still see the same issue.  How is it possible that I am getting a different result than what Tyler is getting?

I am running the detector1 pipeline in a script as seen below, where filenames is the list of the '_uncal.fits' files in the current directory.


for fname in filenames:
    result1 = Detector1Pipeline()
    result1.refpix.skip = False
    result1.refpix.save_results = True
    result1.linearity.skip = False
    result1.linearity.save_results = True
    result1.dark_current.skip = False
    result1.dark_current.save_results = True
    result1.jump.skip = False
    result1.jump.save_results = True
    result1.ramp_fit.skip = False
    result1.ramp_fit.save_results = True
    result1.persistence.skip = True
    result1.persistence.save_trapsfilled = False
    result1.run(fname)
    outname = fname.replace('_uncal', '_0_ramp_fit')
    goodname = fname.replace('_uncal', '_rate')
    os.rename(outname, goodname)
    outname = fname.replace('_uncal', '_1_ramp_fit')
    goodname = fname.replace('_uncal', '_rateints')
    os.rename(outname, goodname)```
This code is intended to let me select which steps to run hence each step has a .skip value set to True or False.  But the only thing I am skipping here is the persistence step, and that is not relevant to the matter at hand.
stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

Kevin Volk did you create a completely new environment and install everything from scratch? Remember that the core algorithms for steps like jump, saturation, ramp_fit, etc. now reside in the stcal package and that's where some fixes have been made over the past month or two. So if you just installed the latest jwst package without also upgrading stcal, that could explain the difference. Or perhaps one of you is using different reference files?

stscijgbot-jp commented 1 year ago

Comment by Tyler Pauly on JIRA:

This is a result of using .run() instead of .call(). Run does not pull parameter reference files from CRDS, which in this case ignores the pars-jumpstep reference that exists for NIRISS F115W data. This parameter reference file sets the following:

parameters: {class: jwst.jump.jump_step.JumpStep, flag_4_neighbors: 'False', max_jump_to_flag_neighbors: 200.0,
  min_jump_to_flag_neighbors: 10.0, name: jump, rejection_threshold: 12.0}

Setting these by force in your script reproduces the result.

stscijgbot-jp commented 1 year ago

Comment by Paul Goudfrooij on JIRA:

Yes those parameter reference files with high rejection threshold were created to provide a "quick workaround" of the problem we presented on Tuesday - a workaround within the steps and parameters available right now. But it isn't the correct fix, as discussed on Tuesday.

stscijgbot-jp commented 1 year ago

Comment by Tyler Pauly on JIRA:

That would explain the difference, then... I missed the part in the slides that a workaround fix was in place on OPS server. Apologies for the confusion.

stscijgbot-jp commented 1 year ago

Comment by Kevin Volk on JIRA:

I routinely run pipeline steps from scripts with .run() as in the example above.  I did not realize that this means the parameter reference file is not used.  But in this case I want to be using the step defaults for the jump step when I do this test.  We did put in the "band-aid" parameter reference file with a high jump threshold to get around the worst of these issues, but that is not a good solution.  A 12 sigma threshold is much too high for the ramps that are not having the charge migration issue, assuming that our read noise and gain reference files are reasonably accurate.

stscijgbot-jp commented 1 year ago

Comment by Kevin Volk on JIRA:

As to Howard's question above, I did install the pipeline in a new conda environment as per the instructions, which is what I always do.  Presumably the stcal package was installed fresh along with the pipeline itself.

 

stscijgbot-jp commented 1 year ago

Comment by Paul Goudfrooij on JIRA:

Yes I just upgraded my version of the pipeline as well and it stated that it included stcal as part of the update:

"Successfully installed BayesicFitting-3.0.1 asdf-2.13.0 asdf-astropy-0.2.2 astropy-5.1 certifi-2022.5.18.1 crds-11.16.11 drizzle-1.13.6 gwcs-0.18.2 jwst-1.7.1 stcal-1.1.0 stpipe-0.4.2 tweakwcs-0.8.0 wiimatch-0.3.1"

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

Yes, you were both using the latest version of stcal. We now know that the differences in results were due to the pars ref file selection issue.

stscijgbot-jp commented 1 year ago

Comment by Paul Goudfrooij on JIRA:

After a presentation to the CalWG on Sep 6, 2022, the (preliminary) decision was made to include a new step in the detector1 pipeline between the jump and ramp_fitting steps to deal with the issues seen for undersampled (NIRISS) images that are due to the current set up for the jump step and ramp fitting. A proposal on the requirements for this new step is described here. The proposed name for the new step is "undersampling_correction".

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

Fixed by JP-3015 and JP-3029, both of which have now been merged into jwst/master on github. So this is ready for testing by INS. Note that by default the new undersampling_correction step in the Detector1 pipeline is set to be skipped in the code. So in order to test it you'll need to either manually override the skip parameter when running the pipeline or use a parameter reference file to turn it on.

stscijgbot-jp commented 1 year ago

Comment by Paul Goudfrooij on JIRA:

Thanks, Howard Bushouse . Just to clarify - if we want to run the new undersampling_correction step and want to study the impact to the GROUP_DQ values prior to ramp fitting (i.e., if we want to look at the equivalent of the *_ramp.fits files which were the product of the jump step), then I presume we should use undersampling_correction.skip = False and undersampling_correction.save_results = True, correct?

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

Paul Goudfrooij Yes, that's correct. You can save the results either by using save_results on that step, or use the save_calibrated_ramp option on the Detector1 pipeline (which saves the results just before ramp fitting starts). If you save results from the jump step, and then again on the undersampling step, you should be able to easily see the new GROUPDQ flags added by undersampling.

stscijgbot-jp commented 1 year ago

Comment by Paul Goudfrooij on JIRA:

Thanks, Howard Bushouse . Unfortunately it looks like the new step doesn't do exactly what we wanted it to do for pixels for which the accumulated counts reach beyond the signal_threshold during the integration. What is happening is that the "early" groups that did not yet reach the signal_threshold (i.e., the groups that did not get the UNDERSAMP flag) still got assigned jump flags (which were assigned by the jump step due to the charge migration issue), while we requested in JP-3029 (see my comments on Jan 11 and 12, 2023) that the jump detection be re-run on the groups that did not (yet) get the UNDERSAMP flag (for the typically few pixels that did get the undersampling_correction flag during the integration(s), that is). Should we file a new ticket to get that addressed? (I can illustrate what I mean in a writeup or presentation if folks like.)

PS: it occurred to me that another, perhaps better, option might be to run the undersampling_correction step PRIOR to the jump detection step, and have the jump detection step also ignore the groups that have the UNDERSAMP flag set. 

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

Paul Goudfrooij It sounds to me like this step needs to have an entry added to the Cal WG list of specs for all Detector1 steps at https://outerspace.stsci.edu/display/JWSTCC/CALWEBB_DETECTOR1, with a complete and concise description of all aspects of the algorithm, including where to call it in the pipeline (i.e. before or after jump), what needs to happen in the jump step, what needs to happen in the ramp_fit step, etc. I think a lot of details have gotten lost or hidden in the thread of ticket comments that occurred after the basic operation was reviewed by the Cal WG. Once decisions have been made on all the details and they've been documented in a page like the ones for other steps (e.g. https://outerspace.stsci.edu/display/JWSTCC/Vanilla+Ramp+Jump+Detection), we'll then proceed with making any necessary updates and revisions to what exists now.

stscijgbot-jp commented 1 year ago

Comment by Paul Goudfrooij on JIRA:

OK Howard Bushouse - I added an entry for this step in https://outerspace.stsci.edu/display/JWSTCC/CALWEBB_DETECTOR1](https://outerspace.stsci.edu/display/JWSTCC/CALWEBB_DETECTOR1,) (in the table, prior to the jump detection step) along with a link to a detailed description (in the same format as that for the other steps). Let me know if you have any comments or questions on this. 

stscijgbot-jp commented 1 year ago

Comment by Anton Koekemoer on JIRA:

Thanks Paul Goudfrooij  for adding this step in the table for CALWEBB_DETECTOR1 (and, for reference, here are the meeting notes where this was discussed: JWST Cal WG Meeting 2022-09-06).

I've made a slight change however to where this is located - you'd placed it under the "Baseline" column, but that version really refers more to what was operational around launch, and so this belongs more to the "Enhanced" category so I've moved it to that column, specifically "Enhanced v2+" (Karl Gordon  can confirm/ concur or move it elsewhere, as appropriate)

stscijgbot-jp commented 1 year ago

Comment by Paul Goudfrooij on JIRA:

OK Anton Koekemoer - thanks for that adjustment (I didn't know the history of the setup of that table). 

stscijgbot-jp commented 1 year ago

Comment by Paul Goudfrooij on JIRA:

Hi all, just checking on the process to finalize addressing this ticket (now that we added this step in the table for CALWEBB_DETECTOR1). Should this remain the parent ticket? And should separate tickets be created to address the remaining issues with the undersampling_correction step (which involves adjustments to the jump step as well)?  Would I need to create those tickets? (Tagging Howard Bushouse and Anton Koekemoer for this - sorry about the bother.)

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

Paul Goudfrooij Thanks for the complete description provided at https://outerspace.stsci.edu/display/JWSTCC/Description+of+Undersampling+Correction+step . I suggest leaving this ticket open, as the parent ticket, and then we (SCSB) will create any necessary additional JP tickets to make the changes that are needed in order to comply with the updated specs. We will get back to working on this soon.

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

Initial version of the new undersampling_correction delivered in B9.2 has been determined to be inadequate and needs changes, so I've reopened this ticket and reset the Fix Version.

stscijgbot-jp commented 1 year ago

Comment by Everett Schlawin on JIRA:

Nestor Espinoza and Kevin Volk, If I understood correctly at the Cal WG meeting yesterday, this will mark pixels with counts above ~25,000 DN as CHARGE_LOSS to be excluded from the jump step and ramp fitting. Will this cause issues for SOSS time series or other bright targets, where I assume many observations are designed to regularly go above 25,000 DN?

stscijgbot-jp commented 1 year ago

Comment by Kevin Volk on JIRA:

Right now this correction is not applied to SOSS observations.  When it goes live it will affect the other modes of NIRISS but not any observation with the GR700XD in the beam.

stscijgbot-jp commented 1 year ago

Comment by Everett Schlawin on JIRA:

Ah ok, thanks. Maybe someone will figure out a reverse-migration algorithm that works for most modes and detectors.

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

One thing to note about this, is that right now there's no entry for EXP_TYPE='NIS_SOSS' in the CRDS rmap for the jwst_niriss_pars-chargemigrationstep parameter ref files, so when calwebb_detector1 gets run on a SOSS exposure the pipeline gives you the message:


2023-10-05 08:06:11,299 - CRDS - ERROR -  Error determining best reference for 'pars-chargemigrationstep'  =    parameter='META.EXPOSURE.TYPE [EXP_TYPE]' value='NIS_SOSS' is not in ['NIS_AMI', 'NIS_EXTCAL', 'NIS_IMAGE', 'NIS_WFSS']```
Fortunately this doesn't actually stop the pipeline from running, it just proceeds without using a pars-chargemigrationstep ref file, which then means that the default `skip = True` value set in the charge_migration step parameters gets used and hence the step is in fact skipped for SOSS exposures.

This is the desired behavior (skipping the step for SOSS), but you might need to be prepared to answer user's questions about the error message from CRDS. The error message could be avoided by adding a generic entry for EXP_TYPE='NIS_SOSS' in the CRDS rmap that sets the reference file to "N/A", so that it knows a param ref file is not needed or desired for SOSS mode.
stscijgbot-jp commented 1 year ago

Comment by Anton Koekemoer on JIRA:

Thanks Howard. I'm tagging Kevin Volk  and also Rosa Diaz  who can hopefully help with navigating this CRDS issue/ fix that's needed.

stscijgbot-jp commented 11 months ago

Comment by Howard Bushouse on JIRA:

I'm unsure as to the status of this ticket. It was reopened several months ago when the NIRISS team gave SCSB updated requirements for the charge_migration step, which have now been implemented for B10.0 under JP-3299. So I'm wondering if the issues with jump detection are still a problem or whether any testing has been done to see if they are. Should or can we set this back to "Ready to Test" under B10.0?

stscijgbot-jp commented 11 months ago

Comment by Alicia Canipe on JIRA:

Kevin Volk Jo Taylor What do you think? Now that JP-3299 is resolved, how would you like to proceed with this ticket? 

stscijgbot-jp commented 11 months ago

Comment by Kevin Volk on JIRA:

We have to do a test for whether the changes in the jump flagging as a result of the addition of the charge migration correction step has fixed this issue.  Ideally the changes will have fixed this issue but it appeared unexpectedly and we do not know exactly what change caused the jump flagging to go wild.

stscijgbot-jp commented 10 months ago

Comment by Howard Bushouse on JIRA:

Given that the next thing that has to happen here is more testing by the NIRISS team, with no known code work yet to be done, I'm setting the status to "Ready to Test".

stscijgbot-jp commented 10 months ago

Comment by Kevin Volk on JIRA:

I have done some tests with other data sets rather than the one that was highlighted when the issue was first raised last year, and in that case it looks like the issue has been resolved by the changes to the pipeline (especially the charge migration correction to the flagging).  I hope to also take another look at the original data set sometime soon.

stscijgbot-jp commented 8 months ago

Comment by Kevin Volk on JIRA:

Although I did not recheck the original case, it does look like the issue is resolved so I am closing it.