JP-3330: Add NIRSpec wavelength corrections to slit WCS

[hayescr]

Resolves JP-3330

Closes #7794

This PR updates the wavecorr step to add zero-point corrections for each slit into their model WCS. This is done with a 1D lookup table to convert between the center of slit wavelengths and the corrected wavelengths. The corrected wavelengths in the lookup tables are calculated using the pixel shifts in the associated reference file and the wavelength and dispersion in a given 2D extracted slit (averaged across the pixels in the cross-dispersion direction). Includes updates to the wavecorr tests to check that this transform roundtrips along with a few additional checks.

Checklist for maintainers

• added entry in CHANGES.rst within the relevant release section
• updated or added relevant tests
• updated relevant documentation
• added relevant milestone
• added relevant label(s)
• ran regression tests, post a link to the Jenkins job below.
  How to run regression tests on a PR
• Make sure the JIRA ticket is resolved properly

[nden]

Regression tests run: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1364/

[nden]

The implementation in this PR matches the description in the note attached to the issue and the JIRA ticket.
The documentation needs to be updated to include the new algorithm.
One further change to consider is whether the wavelength correction should be a separate frame in the WCS pipeline. As it stands now, the only way to exclude this correction is to rerun the pipeline and skip wavecorr. If it's a separate frame in the WCS pipeline one could evaluate the WCS skipping that frame. However, this is a convenience and purely optional.

[hayescr]

The new commits address the previous comments, update the documentation, and add in the wavelength correction as a separate WCS frame, wavecorr_frame following the slit_frame.

[codecov]

Codecov Report

Attention: Patch coverage is 81.35593% with 11 lines in your changes missing coverage. Please review.

Project coverage is 58.13%. Comparing base (b7e0b10) to head (470fd2e).
Report is 311 commits behind head on master.

Files with missing lines	Patch %	Lines
jwst/pathloss/pathloss.py	0.00%	7 Missing ⚠️
jwst/flatfield/flat_field.py	20.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8376      +/-   ##
==========================================
+ Coverage   58.02%   58.13%   +0.10%     
==========================================
  Files         388      388              
  Lines       38977    38973       -4     
==========================================
+ Hits        22617    22656      +39     
+ Misses      16360    16317      -43     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[nden]

@hayescr Is this ready for review? If so do you want to take it out of Draft and rebase?

[nden]

This PR needs a change log entry.

Another regression tests job started: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1414/

[melanieclarke]

I think these changes might require some changes to the flat_field step as well: in the flat_for_nirspec_slit function, there is some handling to produce a flat field with and without wavelength correction. The use_wavecorr=False option will need to be updated, since it currently assumes the wavelength correction is not in the WCS.

Edited to add: also in the get_wavelengths function in lib/wcs_utils.py

[hayescr]

I think these changes might require some changes to the flat_field step as well: in the flat_for_nirspec_slit function, there is some handling to produce a flat field with and without wavelength correction. The use_wavecorr=False option will need to be updated, since it currently assumes the wavelength correction is not in the WCS.

Edited to add: also in the get_wavelengths function in lib/wcs_utils.py

Agreed it looks like these instances primarily use the wcs to generate the uncorrected wavelengths for fixed slit background corrections (since the background will be a extended source), which will no longer work, so I can work on adding fixes here. @nden and @melanieclarke, would it be better to reproduce uncorrected wavelengths from the wcs for these steps (by skipping the wavelength correction transforms), or to add a "uniform wavelength/uncorrected wavelength" extension that would be saved in the wavecorr step in case users wanted access these after-the-fact, similar to how the flat_field/pathloss/photom steps add the extended source corrections as extra extensions for point sources?

The pathloss step also calculates point source and extended source corrections for fixed slit observations (so it can apply the right corrections for point sources and the uniform background in master_background_step), but it is currently using the same wavelength for the point source and uniform pathloss corrections. Should this be corrected as well (see the lines below)?

https://github.com/hayescr/jwst/blob/250a44a54618ff9278d9b8d715746ab914701f96/jwst/pathloss/pathloss.py#L979-L991

[melanieclarke]

@hayescr - For the pathloss correction wavelengths, I think you're right that it shouldn't be using the corrected wavelength array for both corrections. It should probably use uncorrected wavelengths for uniform, corrected wavelengths for point source. But you may need to double check with Elena to make sure the pathloss correction isn't expecting uncorrected wavelengths for both.

[nden]

Thanks for working on this. I have a few comments.

• Now that a separate frame wavecorr_frame exists in the WCS pipeline, when use_wavecorr is False the transform can be evaluated excluding the wavecorr transform, like in

detector2slit = wcs.get_transform('detector', 'slit_frame')
wavecorr2world = wcs.get_transform("wavecorr_frame", "world")
ra, dec, lam = (detector2slit | wavecorr2world)(x, y)

That's one way to do it. Is there any benefit to providing a wavelength array without correction to users?

• When testing the wavecorr transform on a brightobj observation I would expect the correction to be zero for a source in the center of the slit (0, 0) but the wavelength changes. Here's my test

s2w=calint.meta.wcs.get_transform('slit_frame', 'wavecorr_frame')
s2w(0,0,2e-6)
Out[54]: (0.0, 0.0, 1.9986767250452716e-06)

Am I doing something wrong or is this a reference file issue?

• Finally, and it's not an issue that this PR introduces, I believe the step is not marked as skipped in all cases. For example, a test on a MOS observation with extended sources does not set model.slits[index].meta.cal_step.wavecorr to Skipped.

[hayescr]

Thanks @nden for the feedback. I don't think that providing the uncorrected wavelength array is necessary (since it can still be reproduced from the WCS without rerunning the pipeline with wavecorr off), but I thought I would check.

The zero-point corrections are not exactly zero for a perfectly centered point source in the reference files. For the S1600A1 slit the zero-point shift is ~0.08 pixels for a perfectly centered source, and if that was a PRISM observation, the wavelength shift would be about ~0.0015e-6, matching what you found.

I see where the code is not setting model.slits[index].meta.cal_step.wavecorr to Skipped, and I can update that as well while making these changes.

[hayescr]

@nden this is ready for re-review. I've updated the steps that depended on the old format of wavecorr.

For flatfield, I refactored the way flat_for_nirspec_slit checks for wavelengths to have it use the updated lib.wcs_utils.get_wavelength, because the two were getting the wavelengths in a similar way (i.e., looking for a wavelength array first and then using the wcs to calculate wavelengths if an array wasn't available). There are some log warnings/errors that flat_for_nirspec_slit included that get_wavelength does not, but I think that should be the only difference in actual behavior. Let me know if it looks like I missed something though.

I checked with Elena Manjavacas about the pathloss step for fixed slit: it should be using the corrected wavelengths for point sources and uncorrected wavelengths for uniform sources, so I've pointed this step to also use get_wavelength for the two cases.

The wavecorr step is now marked as complete/skipped for each individual MOS slit separately, and this shows up in the 2d unrectified spectra files (e.g., the cal files), but one issue is that these appear to be overwritten in the resampled (s2d) spectra. Perhaps the resampling step is populating the slit metadata with the multislit metadata unless it's told otherwise?

[melanieclarke]

Sending a few suggestions for cleaning up the change log.

Otherwise, I think this looks pretty good. I think the processing keywords in the slit models post-resampling are a bit beyond the scope for this ticket, but we should probably make a note of it in JP-3630.

Testing locally with PID 1492 for FS and 2736 for MOS, I mostly see the changes I expected. The wavelength correction still looks good in the x1d files.

However, I tried running the 1492 data with --steps.wavecorr.skip=True and comparing it to the data with wavecorr on. I see some differences in the cal file I did not expect: PATHLOSS_PS is set to unity when wavecorr is off; FLATFIELD_UN and PATHLOSS_UN show some small differences. Can you please check this case? Test data was jw01492003001_03104_00001_nrs1_rate.fits, for example.

[hayescr]

@melanieclarke thanks for the review and comments on the change log. I took a look at the example you listed, some comments below on each case:

PATHLOSS_PS is set to unity when wavecorr is off: This is because this example is a FS observation and currently wavecorr is what calculates the source x and y position in the slit for FS point sources. So with wavecorr off the source_xpos and source_ypos both default to 0 and by definition the path loss corrections at the center of the slit are unity. This is not introduced by this PR, but because the source positions for point sources are used in a couple places (pathloss, extract_1d for use_source_pos, etc.) it might be good to move this out of wavecorr and somewhere else in the spec2 pipeline (assign_wcs perhaps?).

PATHLOSS_UN show some small differences when wavecorr is off: in my testing of this, I see that the (non-NaN) differences in PATHLOSS_UN between wavecorr on and off is either 0 or a small number (~1.2e-7) that looks like a floating point error, does this match with what you found?

FLATFIELD_UN show some small differences when wavecorr is off: I also see this; the fractional differences are at the level of < ~1e-5. I think this comes from the difference in how the wavelengths are obtained. With the update I made to have flat_field use get_wavelengths, the wavelengths used for interpolating the flat_field data come from the WCS when wavecorr is on and will come from the stored wavelength array when wavecorr is off. If I force the flat_field to use the WCS for calculating the wavelengths in both cases I find no difference in the FLATFIELD_UN. Comparing the array and WCS wavelengths there is a small difference (on the order of < 1e-7um -> though that is about 1e-4 of a pixel in this case), which looks to be coming from the fact that the wavelength arrays are float32 and recalculating the wavelengths from the WCS are float64.

[melanieclarke]

Thanks for the investigation and explanation, @hayescr! That all makes sense.

I agree that it would probably be more useful to assign the source position in the assign_wcs step instead of in wavecorr, but that's a separate issue. Can you please file a ticket for it?

Testing a merge of your branch with a development branch of my own, I noticed one more small thing to fix. Suggested change below.

[hayescr]

@melanieclarke yes I'll open a ticket (it's now noted on JP-3648) to move the source position assignment. Thanks for the catch on the missing wcs case.

[melanieclarke]

Thanks @hayescr.

I started a new regression test run here:
https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1498/

[melanieclarke]

Regression test changes look right to me: there are changes to the wavelength in wavecorr for NIRSpec spec2 for MOS, BOTS, and FS, then changes to everything downstream of flat_field. Master background and MIRI LRS failures are unrelated.

Everything looks good to me. It would be helpful if @nden can review one more time, though.

[melanieclarke]

Except one thing - @hayescr, can you please fix the merge conflicts in the change log?

[hbushouse]

Looks good to me overall and I agree that the regtest differences seem to comply with what's expected. Just a few minor comments/questions to address.

[hbushouse]

Is there still a use case for retrieving and returning the uncorrected wavelengths? Is this necessary for steps like pathloss and/or flatfield that want to compute their calibrations based on both uncorrected and corrected wavelengths (i.e. for uniform and point sources, respectively)?

[hayescr]

Yes, that's correct. Pathloss, flatfield, and photom need access to the uncorrected and corrected wavelengths to calculate and store the uniform/point source corrections. These corrections are then used in the master_background step in spec3 for properly handling background subtraction (with the background as a uniform source) for point sources that are observed in the NIRSpec fixed slit mode.

[hbushouse]

I don't think cal_step attributes are defined (in the datamodels schema) at the slit level; only at the top model.meta level. I guess this operation is still OK to do, because it will just create those attributes within each slit instance, but I doubt they will actually result in keywords in the FITS file on disk (because they aren't defined in the schema). They would only exist within the datamodel's slit meta object.

[hayescr]

I think that is correct. I don't see any relevant FITS keywords that are produced for each slit, but the attributes do show up in the datamodel's slit meta object. So there is some way to keep track of whether this was step was performed for an individual slit.

[nden]

LGTM.
Thanks @hayescr and @melanieclarke for the work