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]

I am getting mean differences in the computed wavelengths (this method vs the old one) of the order 10**-10 which is probably too large. I believe the previous method subtracts the correction while this one adds it. Since the previous results were vetted by the NRS team I'm assuming they are correct but please let me know if this has changed. So, in short I wonder if the plus sign here is an oversight.

[hayescr]

The old method was subtracting the correction, and the difference between subtracting and adding the correction should the be source of these large differences. When reviewing the zero-point corrections the NRS team noted that the correction should be added rather than subtracted and we confirmed this looking at observed data. So the plus sign is an intentional update.

[melanieclarke]

Just to confirm what Chris said... The NIRSpec cal team looked at this and is convinced that the minus sign was an error, and we should use the plus sign going forward.

[nden]

There's probably no need to explicitly assign an inverse model to a 1D tabular model but it doesn't hurt.
lam_corrected need to be monotonically increasing and that's probably always true but I wonder if there should be a check for this.

[hayescr]

The lam_corrected should always be monotonically increasing, but if it would be useful, I can add a check for this that will skip the wavelength correction in case it is not monotonically increasing and would cause later errors.

[jdavies-st]

Yes, no need to define an inverse for a Tabular1D model as it is automatically defined since 2019.

astropy/astropy#9083

And note there's already a check there to ensure ascending order as needed by scipy.interpolate for the inverse. Feel free to do the same check here for the forward transform. 👍

[hayescr]

The most recent commits now remove the explicit inverse and include a check that the lam_corrected is ascending. This should be the case for real data, but to cover the possibility that it is not monotonically increasing, the updated code will now skip the wavelength correction if lam_corrected is not ascending. If you think it should instead throw an error, I can make that change.

[nden]

As I see it this checks that slit.meta.wcs is consistent with slit.wavelength, regardless of whether it's updated or not. It's a good check but if you agree please change the comment.

[hayescr]

I've gone ahead and changed the comment.

[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 82.97872% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 56.40%. Comparing base (6580914) to head (250a44a).
Report is 1 commits behind head on master.

Files	Patch %	Lines
jwst/wavecorr/wavecorr.py	82.97%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8376      +/-   ##
==========================================
+ Coverage   56.38%   56.40%   +0.02%     
==========================================
  Files         387      387              
  Lines       38716    38747      +31     
==========================================
+ Hits        21830    21857      +27     
- Misses      16886    16890       +4     

☔ 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.