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

Add ramp_fit parameter to turn on/off one-group slope calculations #6013

Closed stscijgbot-jp closed 2 years ago

stscijgbot-jp commented 3 years ago

Issue JP-2071 was created on JIRA by Michael Regan:

Currently, the ramp fitting step will fit ramps with only a single useable group using the difference between the first group and the superbias. This assumes that the superbias reference file used darks taken at the same temperature as the current observations. Because there will be secular drifts in temperature over the life of the observatory (especially during commissioning), there can be a mismatch in temperature which will lead to the wrong slope. It would be best to have a parameter value that can turn this off and on with the default being off until this is tested with on-orbit observations.

stscijgbot-jp commented 2 years ago

Comment by Howard Bushouse on JIRA:

Alicia Canipe Anton Koekemoer this ticket needs a priority level from INS and/or the Cal WG (or possibly discussion by the Cal WG before prioritizing?)

stscijgbot-jp commented 2 years ago

Comment by Anton Koekemoer on JIRA:

thanks Howard Bushouse for the comment, thanks also Michael Regan  for filing this, I've added the "INS Team priority" field to the ticket, and for assigning a priority I have now added this ticket to the agenda of our next JWST CalWG meeting 2021-11-30

stscijgbot-jp commented 2 years ago

Comment by Karl Misselt on JIRA:

NIRCam long ago requested that, for multi-accum data, the first frame also be downlinked; I believe it maybe called frame0.fits (?). So in a case where, e.g. the first group of a shallow4 integration saturated, we could pull the 1st frame out of the saturated group of 4 as an estimate rather than discarding the pixel completely. I don't believe the pipeline currently does anything with frame0 data. Is that correct? In a situation like this, those data would be very much like NGROUP=1 data or data with a single usable group, if one step deeper - IF (NIRCamCoadd && FirstGroupSaturated) extractFrame0. If (frame0 !saturated) apply single group algorithm. Perhaps this is all offline/post processing and not something for the pipeline.

stscijgbot-jp commented 2 years ago

Comment by Howard Bushouse on JIRA:

That's correct Karl Misselt, in that the zero-frame data are stored by DMS in an extra extension of the uncal (level-1b) products when those data are included in the downlink, but currently nothing is done with them (yet).

stscijgbot-jp commented 2 years ago

Comment by Anton Koekemoer on JIRA:

thanks Karl Misselt  and Howard Bushouse, I've added the Frame0 item on the agenda for our next meeting JWST Cal; WG 2022-01-04

stscijgbot-jp commented 2 years ago

Comment by Anton Koekemoer on JIRA:

also noting that the Frame0 item has its own ticket, https://jira.stsci.edu/browse/JP-374

 

stscijgbot-jp commented 2 years ago

Comment by Howard Bushouse on JIRA:

It doesn't actually say it explicitly anywhere in this ticket or in the minutes of the Cal WG frame0 discussion, but I believe the Cal WG agreed to implement what was originally requested in this ticket, which is to add a new param to the ramp_fit step that would allow a user to toggle on/off the slope calculation that's currently done when only the first frame of an integration is not saturated, and that the default setting for the new param is OFF (i.e. do NOT compute a slope when only the first frame is not saturated). Correct?

This is completely separate from the implementation of the use of frame0 for NIRCam exposures, but could/would also apply to that situation (i.e. even after we implement the NIRCam frame0 correction, a user could still turn that off when desired). Correct?

stscijgbot-jp commented 2 years ago

Comment by Howard Bushouse on JIRA:

Whoops, there was nothing about this new param in the discussion that took place regarding the frame0 correction, but there was agreement in the earlier meeting discussing only this option: https://outerspace.stsci.edu/display/JWSTCC/2021-11-30+Meeting+Notes So ignore my question above. The Cal WG has given the green light to proceeding with this ticket.

stscijgbot-jp commented 2 years ago

Comment by Anton Koekemoer on JIRA:

Thanks Howard Bushouse  and yes, the meeting where this ticket (JP-2071) was primarily discussed and a decision made was not JWST Cal; WG 2022-01-04 (where it was mentioned just briefly, in the context of the Frame0 discussion, JP-374) but rather the earlier meeting, JWST Cal WG 2021-11-30 where the decision was indeed as you summarize, ie to implement this parameter to turn on/off the one-group slope calculations, where the implementation prioritization level was decided as "high" (ie not critical for the operational build during commissioning, but important enough to implement for the development builds so that it can be run off-line post-B7.9), with more details in the minutes - ie, yes, this ticket can proceed, as you concluded too.

 

 

stscijgbot-jp commented 2 years ago

Comment by Howard Bushouse on JIRA:

Fixed by #6737

stscijgbot-jp commented 2 years ago

Comment by Alicia Canipe on JIRA:

Anton Koekemoer  Armin Rest  Karl Misselt and Bryan Hilbert

We have a ticket for submitting the parameter reference files (CRDS-572) to allow this parameter to be turned on or off. The new parameter is described below. The pipeline default is True – is that the desired value for NIRCam? Note also that the Frame0 correction (JP-374) is now implemented, with a bug fix included in Build 8. 

A suppress_one_group boolean variable has been added as a parameter to the ramp fitting step class.

When true (the default) saturated ramps with only the 0th group being good will be treated as completely saturated ramps. The calculations will be done as if all calculations are saturated, but the DQ flags will be set as if the first group is good, i.e., the pixel DQ will not have DO_NOT_USE set.

When false saturated ramps with only the 0th group being good will be treated as the normal special case of having only one good group.

stscijgbot-jp commented 2 years ago

Comment by Bryan Hilbert on JIRA:

And this is completely independent of the frame0 improvement, right? So if the 0th group is the only unsaturated group and suppress_one_group is false, the ramp fitting step will still fit using only the 0th group, as opposed to the first frame and 0th group?

stscijgbot-jp commented 2 years ago

Comment by Howard Bushouse on JIRA:

Kenneth MacDonald can you please comment on the interaction between the "suppress_one_group" parameter and the use of zeroframe data for calculating slopes? Does "suppress_one_group" only apply to how the regular group data are used or does it also affect the use of zeroframe?  For example, if all groups for a pixel are flagged saturated, but the zeroframe is OK (not saturated), will we get a slope calculation based on the zeroframe data when "suppress_one_group" is True (the default)?

stscijgbot-jp commented 2 years ago

Comment by Kenneth MacDonald on JIRA:

In JWST suppress_one_group is defaulted to True for all instruments:

https://github.com/spacetelescope/jwst/blob/3548187c8ccb6b90d2eb86290b26709c2035cddb/jwst/ramp_fitting/ramp_fit_step.py#L186

There have been some modifications to exactly how this works due to MIRI setting the first few groups to DO_NOT_USE.  For these, if the first group after the DO_NOT_USE groups is good, but the rest saturated, then that ramp is to be suppressed as well, even though technically it's not the group 0 that is the good group. The handling for this was implemented in PR #⁠92

spacetelescope/stcal#92

The above handler is currently being worked on as well. It does not work correctly with multiprocessing, but it is a minor fix for it to work with multiprocessing.

If the suppress_one_group is True, then ZEROFRAME is not used:

https://github.com/spacetelescope/stcal/blob/dcda1bc7dd9d1377d63db7f1671531c213b6b8c2/src/stcal/ramp_fitting/ramp_fit.py#L52

I didn't have clear instructions on the interaction with suppress_one_group and ZEROFRAME, but I assumed that if suppress_one_group was true, then ZEROFRAME would not be used even if available because it would be a one "group" (it's actually just one frame) ramp as well, which have been selected to be suppressed.

 

The ZEROFRAME is currently being worked on as well, since it, too, doesn't properly work with multiprocessing.  Again, it's a minor change in the code, though, to get it to work with multiprocessing.  The work to integrate these options into multiprocessing is JP-2623.

stscijgbot-jp commented 2 years ago

Comment by Howard Bushouse on JIRA:

Thanks Kenneth MacDonald. Having "suppress_one_group" apply to any pixel that only has 1 good data point, regardless of whether that 1 good point is from group 0, group n, or frame 0, seems to make sense to me from the standpoint of consistency. Would be interested to know if the NIRCam team agrees.

stscijgbot-jp commented 2 years ago

Comment by Anton Koekemoer on JIRA:

tagging the NIRCam reps Bryan Hilbert  and Armin Rest for input on the question above from Howard Bushouse 

stscijgbot-jp commented 2 years ago

Comment by Bryan Hilbert on JIRA:

That makes sense to me too.

stscijgbot-jp commented 2 years ago

Comment by Karl Misselt on JIRA:

Well it appears that JIRA swallowed my comment from last night - "502: Naughty Gateway". I assure you it was deep, insightful, and positively Shakespearean in the beauty and intricacy of its prose. But instead, you get this.

I can't think of even a pathological case (caveat: NIRCam-centric) where you'd want to automatically in pipeline processing use a single sample up the ramp that was NOT the first frame; or maybe first group. So in answer to Howard Bushouse, if one elects to suppress_one_group, it should definitely apply to any "ramp" with a single sample no matter where in time said sample falls. Even further, even if suppress_one_group is not set - i.e. we want to use frame0 to estimate the signal, that should NOT apply to any groupN/frameN for N>0. i.e. suppress_one_group should be a switch for frame0, but TRUE for all frames/goups > 0. I don't think we should ever use a single group or frame in the middle of the ramp for estimating rates. In general, I think using a single group in automatic processing should only apply to frame 0 - maybe group 0 as well. I actually use an input parameter that sets the minimum group count to consider a ramp usable; any segment or ramp with fewer samples than that is masked; currently set to 2 by default. Sorry if I don't understand the pipeline structure or input parameters correctly here.

As an aside, I think we mostly envisioned frame0 as a possible post processing step to recover saturated data. It was important in MIPS because we had so few pixels, less so with NIRCam. But the option is there and IF the lower level products are easily accessible and not hidden and visulaizable, one can make an evaluation of their data and decide whether it makes sense to try to recover pixels in e.g. the core of a saturated PSF.

Bryan Hilbert - I was a bit confused by "the ramp fitting step will still fit using only the 0th group, as opposed to the first frame and 0th group?". The AND in 'first frame and 0th group?' seems to imply that the pipeline would/could use both? That shouldn't happen, they aren't independent. the first frame (frame0) is in the 0th group already. If one is going to use frame 0, it should only use frame0. I apologize if I missed what you're saying here!

stscijgbot-jp commented 2 years ago

Comment by Bryan Hilbert on JIRA:

Karl Misselt that is what I was asking. I was just curious about how the frame0 code and the first group code interacted. I understand there would be weird correlations if both were used in ramp fitting. 

stscijgbot-jp commented 2 years ago

Comment by Howard Bushouse on JIRA:

The use of frame0 and group0 are mutually exclusive. The code only ever uses one or the other, never both. If group0 is saturated and frame0 is present and not saturated, then frame0 is used (by itself) to estimate a slope. If group0 is not saturated, it's used without even considering frame0.