spacetelescope / jwst

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

Update sky footprint (S_REGION) for resampled spectroscopic products #1744

Closed hbushouse closed 6 years ago

hbushouse commented 6 years ago

Right now the S_REGION keyword is first populated by the set_telescope_pointing script during level-1b processing, which computes the footprint based on the JWST aperture vertices listed in the SIAF. For imaging exposures, S_REGION is then updated by the assign_wcs step based on the full WCS specification for the exposure, and again by the resample step when it creates a drizzled/combined image.

But spectroscopic exposures do not have their S_REGION value updated at all during either assign_wcs or resampling. Hence the footprint in the calibrated products can often still be set to the full area of the detector on the sky, rather than something more useful like the actual slit or entrance aperture (IFU) projection on the sky. The archive (CAOM in particular) really needs to have the actual slit/aperture footprint computed, at least for our final, resampled products.

So at the very least we should try to recompute a realistic value for S_REGION in:

hbushouse commented 6 years ago

A problem in the current population of S_REGION at level-1b is due to the fact that the SIAF doesn't have complete info for all instrument apertures, especially some of the IFU and slit apertures. For example, MIRI MRS exposures on the short-wave detector may use an original PPS aperture of 'MIRIFU_CHANNEL1A', but because some of the SIAF info for that aperture is missing (like X/Y reference point and X/Y image scale), SDP resets the APERNAME to something generic like 'MIRIM_FULL', which does have a complete set of info in the SIAF. But that in turn means we're getting incorrect values for items like V2_REF, V3_REF, and all the aperture vertices. This in turn means that not only is our computed pointing (RA_REF, DEC_REF) incorrect (it's giving pointing for the Imaging detector instead of the MRS detector), but the S_REGION is also using values appropriate for the footprint of the Imaging detector, instead of the MRS detector.

For example, CV3 test dataset jw93135311001_02101_00001_mirifushort_uncal.fits (available at /grp/jwst/ins/mary/b7.1rc9_full/jw93135/) has S_REGION =

S_REGION= 'POLYGON ICRS  315.4334256011184 68.13928143846911 &'                 
CONTINUE  '315.3486633938344 68.14172066715514 315.35470198410417 &'            
CONTINUE  '68.17294015103279 315.43961112297023 68.1707868402061&'              
CONTINUE  '' / spatial extent of the observation                        

The extent of this footprint in Dec is about 3600*(68.17-68.14) = 111 arcsec, which is the size of the Imager detector on the sky. The size of the MRS aperture is closer to only ~8 arcsec.

So even our S_REGION values in the _uncal.fits products are completely wrong for some spectroscopic modes.

hbushouse commented 6 years ago

Same problem for NIRSpec IFU exposures, e.g. /grp/jwst/ins/mary/b7.1rc9_full/jw95175/jw95175001001_02101_00001_nrs1_uncal.fits uses an original aperture of 'NRS_FULL_IFU', but that gets reset to APERNAME='NRS1_FULL', which results in S_REGION:

S_REGION= 'POLYGON ICRS  80.36842987277946 -69.27400925308463 &'                
CONTINUE  '80.49041859236152 -69.23496076397755 80.60372130012513 &'            
CONTINUE  '-69.27943316679449 80.47944611760516 -69.31955979061618&'            
CONTINUE  '' / spatial extent of the observation 

which represents a footprint of ~2.7 arcmin on the sky (equivalent to the full detector in imaging mode), while the IFU entrance aperture is actually only ~3 arcsec.

hbushouse commented 6 years ago

MIRI LRS slitless exposures seem to be handled correctly, in that they retain their original APERNAME='MIRIM_SLITLESSPRISM', which does have SIAF aperture vertices true to the actual size of the SUBPRISM subarray used for these exposures. This is the best we can do for this mode, because there isn't a physical slit or aperture. The data cover the projection of the subarray on the sky.

For example, /grp/jwst/ins/mary/b7.1rc9_full/jw80600/jw80600012001_02101_00002_mirimage_uncal.fits has APERNAME='MIRIM_SLITLESSPRISM' and

S_REGION= 'POLYGON ICRS  6.129062057016541 -66.61666531543419 &'                
CONTINUE  '6.123609139171861 -66.61648604109422 6.1262061377771095 &'           
CONTINUE  '-66.60369049240492 6.1316195590134095 -66.60387168628867&'           
CONTINUE  '' / spatial extent of the observation        

which represents a footprint of about 45 arcsec in height, which is about the size of the subarray in that direction.

hbushouse commented 6 years ago

MIRI LRS fixed-slit exposures suffer from the same problem as IFU. The original aperture of 'MIRIM_SLIT' gets reset by SDP to 'MIRIM_FULL', which means the computed footprint represents the entire area of the Imaging FOV on the sky, rather than the actual slit (which is only about 5 x 0.5 arcsec).

For example, /grp/jwst/ins/mary/b7.1rc9_full/jw80600/jw80600013001_02101_00001_mirimage_uncal.fits has:

S_REGION= 'POLYGON ICRS  6.178064585431394 -66.63750746068662 &'                
CONTINUE  '6.098549778241279 -66.63474614897345 6.1051102189828095 &'           
CONTINUE  '-66.60355438839859 6.184562238919687 -66.60602349167554&'            
CONTINUE  '' / spatial extent of the observation      

which is equivalent to a footprint of about 112 arcsec, which is the size of the Imager FOV and obviously way bigger than the 5 x 0.5 arcsec slit.

hbushouse commented 6 years ago

Coming up with a fix to the problem of the wrong SIAF aperture entries being used in the original population of S_REGION in level-1b processing is a high priority, because it essentially means we are not creating properly calibrated data for many instrument modes.

jdavies-st commented 6 years ago

Do we know why

The original aperture of 'MIRIM_SLIT' gets reset by SDP to 'MIRIM_FULL'

? What is the purpose of this?

hbushouse commented 6 years ago

It's because SDP code used in the population of level-1b header keywords relies on entries in the SIAF table to get values for the CRPIXn and CDELTn keywords. Specifically, it uses the values from the SIAF columns XSciRef, YSciRef for CRPIXn and columns XSciScale, YSciScale for CDELTn. SIAF entries for apertures that don't correspond to physical readout regions of the detector (pre-defined subarrays correspond to physical readout regions, while "virtual" slit regions do not) don't have the X/YSciRef and X/YSciScale columns populated (and the TEL group won't populate them). So SDP has no source for populating the CRPIXn and CDELTn keywords. The remaining WCS-related keywords are populated by the set_telescope_pointing script from quaternion info.

So in an effort to find some values to put into the CRPIXn and CDELTn keywords, SDP resorted to using a different Aperture entry in the SIAF, namely one that has all the columns populated. So when they see that the SIAF row for MIRIM_SLIT is incomplete, they resort to using the row for MIRIM_FULL. Which then means that everything done by the set_telescope_pointing script also ends up using the entries for MIRIM_FULL, instead of MIRIM_SLIT.

Even worse, none of the SIAF entries for apertures corresponding to the MIRI MRS (IFU) is complete, so SDP resorts to using MIRIM_FULL for those too! Hence even the basic pointing info for all MIRI MRS exposures is off by several arcminutes! (the distance between the MIRI Imager detector and the MRS entrance aperture on the sky).

hbushouse commented 6 years ago

SDP is tracking work to be done on their side in https://jira.stsci.edu/browse/JSDP-364 and is summarized as:

Further notes after a SDP/SCSB/SysEng mtg on 5/30/2018:
(attendees: Bushouse, Dencheva, Swade, Swam)

The CRPIXn, CDELTn values are NOT required by the SCSB data model. They may be CURRENTLY
required by the SCSB set_telescope_pointing.py code, but SCSB will be changing that, if it’s true.

It is BAD for WCS computations when level1b code chooses another aperture instead of the 
PPS-supplied APERNAME, in order to fill missing PRD/SIAF-supplied values, EXCEPT for the
NIRCAM generic->detectorSpecific scenario. The reason it is often a BAD choice is it
OVERWRITES the V2Ref,V3Ref from the original PPS-specified aperture, which were MORE
appropriate in the original, given APERNAME. Same for the S_REGION (a more appropriate value 
is overwritten). We are not checking for filled V2,V3Ref in our “unfilled” test, but instead are
checking CRPIX1 (which comes from a different PRD field). So this test needs adjustment across
the instruments in the level1b code. Rework the APERNAME-replacement code for all instruments
to ONLY apply this APERNAME change when going from a NIRCam full-instrument value down to a
detector-specific value. Remove other replacements.

CRPIXn, CDELTn will be DEFAULTED (by the KWDICT) to value=1, so that SDP code does not have
to do anything with them when the aperture does not set them. SCSB will fill them.

Keep the KWDICT pass-throughs for now, but let SCSB know which WCS-related keywords we will
continue fill in level1b (if any), and SDP will work to MOVE the logic for filling these values to
SCSB's set_telescope_pointing.py, and OUT of level1b.

LEAVE guider data header/products alone (APERNAME and WCS keywords).

NOTE: This work is for b7.2, which SCSB would like to start 6/4 -> midJuly. We should generate
updated level1b outputs (pre-set_telescope) and pass them on to SCSB for their development
work.
nden commented 6 years ago

There are two parts to be implemented for this issue:

  1. Populate FITS WCS keywords.

    • [ ] Spatial keywords are populated from SIAF values.
    • [ ] Spectral keywords are set to their defaults
  2. Update S_REGION in

The only remaining question is who populates WCSAXES. If SDP is not doing this we need to do it in set_telescope_pointing based on EXP_TYPE. @hbushouse ?

hbushouse commented 6 years ago

SDP may include WCAXES in their initial population of the intermediate level-1b product, but I think we should reset it based on the WCS that we determine to be appropriate for the type of observation.

nden commented 6 years ago

@stscieisenhamer I need to classify all possible EXP_TYPE values as "imaging" or "spectral". Are the lists/tuples in associations.lib.dms_base the best place to get those (so I don't have to maintain them in a separate place)? Are they complete? For example,

imaging = ACQ_EXP_TYPES + IMAGE2_SCIENCE_EXP_TYPES + IMAGE2_NONSCIENCE_EXP_TYPES
stscieisenhamer commented 6 years ago

These are the lists, at least for the creation of associations. Since associations should handling everything, these lists should be complete. So, yes, these should be the definitive lists. The only issue is the subclassification that you have run into: For association use, there will be need to sub-classify and re-classify. It may be possible that an EXP_TYPE may appear more than once. As such, you may want to do the grouping as a set to avoid the duplication.

The imaging set is as you have stated. The spectral is defined by SPEC2_SCIENCE_EXP_TYPES and TSO_EXP_TYPES.

The one exception I immediately see is NRC_TSIMAGE. This is in TSO_EXP_TYPES but is imaging.

nden commented 6 years ago

The spectral footprint is implemented in #2327

nden commented 6 years ago

This ticket has become a bit confusing as it mixes several issues. The original issue is that the spectroscopic footprints were not correct. I believe this issue has been resolved in several PRs - #2327, #2364 #2474.

The cause for the original issue was identified to be the aperture name substitution in SDP and subsequent V2_REF, V3_REF incorrect values. SDP is stopping aperture name substitution, however in the process they identified that for some EXP_TYPE values APERNAME is NULL and V2_REF, V3_REF cannot be assigned. These are non-science EXP_TYPEs. A proposed solution is to have set_telescope_pointing check the value of APERNAME and not run update_s_region if it's NULL. A separate ticket should be filed for this. (#2596) In addition it was decided that set_telescope_pointing (and not SDP) should populate headers with FITS WCS keywords from the SIAF. There should be another issue opened for this.

Since the original issue in this PR was resolved this can be closed or kept as a reference.