search

spacetelescope / jwst

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

Refpix not working right on NIRCam subarray #5764

Closed stscijgbot-jp closed 3 years ago

stscijgbot-jp commented 3 years ago

Issue JP-1929 was created on JIRA by Thomas Beatty:

We're testing the TSO pipeline for a NIRCam TSO data challenge, and it looks like the refpix step is not correctly separating the refpix correction step for each of the four amplifiers on NIRCam ALONG. 

The desired behavior should be that the top/bottom reference pixel correction is applied to each amplifier individually, averaging the odd/evens on just that amp. The observed behavior seems to be that the odd/even pixels are averaged across the whole subarray.

I've attached comparison files from pipeline v0.18.0 and NCDHAS, which is the old, internal, pipeline from the NIRCam instrument team. You can see that prior to the reference pixel correction the two match (PlotComp_SN_PreRefPix_Int.png) and after the correction NCDHAS looks better than the STSCI output (PlotComp_SN_PostRefPix_int.png). In particular, the odd/even columns have converged in NCDHAS, but not STSCI.

I've also attached the output files for the NCDHAS and STSCI pipelines, pre- and post-refpix correction, in case that's helpful.

I marked this as 'critical' because the effect is to increase the noise in TSO data by a factor of ~10x, which puts us way over the failure threshold for NIRCam's TSO commissioning.

 

stscijgbot-jp commented 3 years ago

Comment by Howard Bushouse on JIRA:

If I look back at the Cal WG page that documents the requirements for the ref pixel correction ( ), there are comments here and there regarding subarrays like "in subarray, all reference pixels are read through a single output, so they all can be averaged together", which makes it sound like subarrays use only 1 amp for readout and hence there's no need to compute things per amplifier. I haven't yet dug up any more detailed notes from the discussion of the subarray algorithm that occurred in a Cal WG meeting, but perhaps those can shed more light on this question. In the meantime, perhaps some of the instrument experts can provide a quick answer as to whether NIRCam subarrays in particular use 1 or multiple amps for readout, and if it's a mixture, is there a way for the pipeline to know how many amps were used?

stscijgbot-jp commented 3 years ago

Comment by Bryan Hilbert on JIRA:

The grism TSO subarrays (FULL, SUBGRISM256, SUBGRISM128, SUBGRISM64) can be read out using either 1 or 4 amps. The user selects this in APT. 

stscijgbot-jp commented 3 years ago

Comment by Howard Bushouse on JIRA:

Refpix step code inspection confirms that the top/bottom even/odd column algorithms are using a single value across all rows, i.e. they assume the readout was done using 1 amp. A step upgrade would be necessary to handle things on an amp-by-amp basis for subarrays, though I suspect that upgrade should be relatively straightforward, since it's already being done amp-to-amp for full-frame images.

stscirij commented 3 years ago

It's a bit more complicated than that. For full frame images the reference pixels are always in the same place, whereas for subarray images we have to try and figure out where they are. Probably have to embed the subarray in a full frame image and use DQ flags to differentiate between real reference pixels and nonexistent ones

stscijgbot-jp commented 3 years ago

Comment by Anton Koekemoer on JIRA:

Looking through the requirements page (https://outerspace.stsci.edu/display/JWSTCC/Vanilla+Reference+Pixel+Correction) it seems fairly clear (to me at least, unless I'm missing something) that this is to be done separately for each amplifier, eg requirement #⁠1 at the top states:

  1. For each amplifier, the sigma-clipped mean values.."

(and requirement #⁠3 for subarrays seems to be written also in this context, or rather, it doesn't specify to use only a single amplifier).

Also, for odd/even values, this ticket suggests that the code seems to combine them into a single value although the requirements appear to describe calculating mean values separately for the odd/even columns (and subtracting these values separately, from the respective columns).

But I'll leave it to Karl Gordon  to confirm whether my impression of the requirements is correct as above or let us know if something else is intended.

It would also help to know what the code is then actually doing at the moment - is it computing these values separately for each amplifier (as implied by #⁠1 above)?

stscirij commented 3 years ago

For full-frame exposures, the reference pixel corrections are calculated using each amplifier separately, and odd and even columns separately, as well as using the side reference pixels. For subarrays, we were told from the beginning that subarrays would only use 1 amplifier, so a single reference pixel value could be calculated for the whole image and subtracted. There is no information in the header to tell us whether more than 1 amplifier is being used.

stscijgbot-jp commented 3 years ago

Comment by Howard Bushouse on JIRA:

At the time the subarray algorithm was implemented in the refpix step (~3 years ago) Robert Jedrzejewski is correct that there was no way to determine the number of amps used to perform the detector readout. More recently, information has been propagated from the front end systems to allow the addition of the NOUTPUTS keyword to NIRCam headers. So now it is possible to know how many amps were used for a subarray exposure and hence the refpix subarray algorithm could be enhanced.

stscijgbot-jp commented 3 years ago

Comment by Bryan Hilbert on JIRA:

In terms of locating the reference pixels, the bad pixel masks should now have reference pixels flagged, right? So as long as the reference pixel step is being done after dq_init has completed, the reference pixels should be flagged as such?

stscirij commented 3 years ago

Yes, reference pixels should have the REFERENCE_PIXELS dq bit set as long as it was set in the DQ mask reference file.

stscijgbot-jp commented 3 years ago

Comment by Thomas Beatty on JIRA:

Re: Anton Koekemoer's comment about odd/even values, I believe the code and output are correctly separating the odd/even columns for the reference pixel correction, as described in the requirements. It's just not accounting for the scenario when NOUTPUT=4, and separating the calculations to occur on just the data for each amplifier. I've run a test using a fake subarray that was only on one amp, and then everything worked as expected.

stscijgbot-jp commented 3 years ago

Comment by Everett Schlawin on JIRA:

Is NOUTPUTS now a standard keyword? If not, a workaround is that the number of amplifiers could be determined by the frame time following the equations here (https://jwst-docs.stsci.edu/near-infrared-camera/nircam-instrumentation/nircam-detector-overview/nircam-detector-subarrays), they vary by a factor of almost 4 so it doesn't require an extremely accurate TFRAME value. Admittedly it's a lot more annoying that finding the value of one keyword, so hopefully NOUTPUTS is standard.

stscijgbot-jp commented 3 years ago

Comment by Thomas Beatty on JIRA:

Everett Schlawin, yep, NOUTPUTS is now listed as one of the standard keywords for NIRCam grism observations: JWST Keyword Dictionary (stsci.edu)

stscijgbot-jp commented 3 years ago

Comment by Howard Bushouse on JIRA:

I just read the NOUTPUTS value from a lot of NIRCam uncal files produced during DMS B7.7 testing and any time the keyword appears it has a value of 1. I looked back at the Keyword Dictionary and SDP tickets involved in implementing this keyword and there's lot of talk about having the value default to 1 whenever an exposure is taken in subarray mode, and have a value of 4 for all full-frame exposures. If it defaults to 1 for all subarrays, then obviously it won't help us with this issue. There's also discussion in some of the tickets about SDP actually reading the value from a PPSDB table entry, once it becomes available there (don't know if it is yet or not).

So I'm linking in Mike Swam to see if he can help figure out at least what the current state of things is with populating NOUTPUTS. The relevant SDP ticket is JSDP-1152 and some of the Keyword Dictionary tickets are JWSTKD-265 and JWSTKD-302.

stscijgbot-jp commented 3 years ago

Comment by Mike Swam on JIRA:

JSDP-1152 was lost in the Team-Gold backlog (did not get moved over to Team-Coffee).  It has now been moved, and the pre-requisite APT-91673 has been complete for some time, so it should be possible to work this ticket in an upcoming DMS build.

stscijgbot-jp commented 3 years ago

Comment by Howard Bushouse on JIRA:

Thanks Mike Swam. So once JSDP-1152 gets implemented, the upgrade to the refpix step in the pipeline can be tackled. Of course the enhancement to the refpix algorithm could be worked before that, but it just wouldn't ever get used until we start seeing values other than NOUTPUTS=1 for subarray exposures.

Cal WG should also assign a priority to the refpix enhancement work.

stscijgbot-jp commented 3 years ago

Comment by Anton Koekemoer on JIRA:

Scheduled this for JWST Cal WG Meeting 2021-03-02 to discuss prioritization of this refpix/ amplifier issue (which, to be clear, would take place in the DMSWG as this is a bug ticket, so this Cal WG discussion is just to confirm that we are all on the same page about this).

stscijgbot-jp commented 3 years ago

Comment by Michael Regan on JIRA:

Isn't the prioritization done outside of the CALWG? There is no algorithm question here we just didn't handle stripe mode correctly and now we will.

stscijgbot-jp commented 3 years ago

Comment by Anton Koekemoer on JIRA:

Michael Regan I'm glad you also agree there is no algorithm question - ie, the specifications already state that the correction is to be calculated for each amplifier separately, and this needs to be implemented for subarrays, so this ticket does remain a bug (not an enhancement). It is scheduled for (brief) discussion in the CalWG primarily to ensure that we are all on the same page about this, and as a bug it would then move to the DMSWG to be tracked further (and prioritized there). Does this help?

stscijgbot-jp commented 3 years ago

Comment by Robert Jedrzejewski on JIRA:

I'm about to start on this and I just wanted to confirm the scope of the work

For NIR detector subarray data:

Can someone clarify please?  Thanks!!

stscijgbot-jp commented 3 years ago

Comment by Anton Koekemoer on JIRA:

Thanks Robert Jedrzejewski, I'll rag here Michael Regan  (and also Thomas Beatty) who should be able to clarify.

stscijgbot-jp commented 3 years ago

Comment by Thomas Beatty on JIRA:

Robert Jedrzejewski I can speak to your first two questions.

1) That's correct, though just to be sure, the "data sections" are really the different sections of the detector being read out by the different amplifiers. With 4 amps the detector is divided vertically into 4 regions, each 512 pixels wide. What should be happening is that the existing odd/even reference pixel correction should be calculated and applied to each of those 512-pixel sections separately (if we're reading out with 4 amps).

2) The side reference pixel correction should be attempted if both or just one side are available. There's a deeper question here if the side reference pixels for amps #⁠1 and #⁠4 (i.e., the ones we get) are exactly relevant to the behavior in the middle amps #⁠2 and #⁠3 (without side refs), but our preliminary testing at UA indicates that there's a lot of commonality between all of the amps – so I would definitely include it for now.

stscijgbot-jp commented 3 years ago

Comment by Anton Koekemoer on JIRA:

I'll speak to the 3rd and 4th questions:

3) this ticket really is about correcting the treatment of subarrays with multiple amps, which until now have been treated as though they have only 1 amp even in cases where they are using more. NIRSpec evidently does not have any subarrays with > 1 amp (as per APT-91673) so the NIRSpec algorithm that you reference would be unrelated, and not part of the work for this ticket.

(I'll also note that for FULL frame data the refpix correction should already be taking into account > 1 amp, this ticket is really about now correcting this for subarrays).

4) NIRISS also has some subarrays that have more than 1 amp (WFSS128R and WFSS64R). The question also came up about FGS, but Rossy informed us that Pierre Chayer reported that they are not impacted by this.

 

So in terms of NIR instruments this currently concerns NIRCam and NIRISS subarrays that use > 1 amp.

 

I'm also not aware of any MIRI subarrays that are impacted by this issue but Michael Regan  could confirm that (and he could clarify other points for you as well, if needed)

stscijgbot-jp commented 3 years ago

Comment by Howard Bushouse on JIRA:

Fixed by #5926

stscijgbot-jp commented 3 years ago

Comment by Howard Bushouse on JIRA:

Thomas Beatty this has been fixed and the new code is now in the master branch of the jwst repo. So we'll assign the ticket back to you to do testing and confirm the desired results.

stscijgbot-jp commented 3 years ago

Comment by Thomas Beatty on JIRA:

Thanks Howard Bushouse. HST proposals are due this Friday, so I'll take a look next Monday or Tuesday.

stscijgbot-jp commented 3 years ago

Comment by Everett Schlawin on JIRA:

This appears to fix stage 1 for data with NOUTPUTS=4! I attached some files, which I guess appear at the top. (I'm still learning Jira)

One note to people testing like Thomas Beatty: I had to add the NOUTPUTS=4 keyword to _uncal.fits file in the mirage simulation I was testing in order to get the new refpix to work.

stscijgbot-jp commented 3 years ago

Comment by Thomas Beatty on JIRA:

Howard Bushouse, I concur with Everett Schlawin that this looks like it is fixed! Thanks to all for getting this worked out so quickly.