JP-3437: Address alternating column noise for NIRSpec IRS2

[melanieclarke]

Resolves JP-3437

Closes #7995

This PR adapts traditional readout handling for NIRSpec IRS2 mode, to subtract mean reference pixel offsets by amplifier and column parity. The offset subtraction is done before IRS2 handling for interleaved reference pixels.

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

[codecov]

Codecov Report

Attention: Patch coverage is 89.18919% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 75.38%. Comparing base (4cc0ac1) to head (f94a4a4).
Report is 10 commits behind head on master.

Files	Patch %	Lines
jwst/refpix/reference_pixels.py	87.65%	10 Missing ⚠️
jwst/refpix/refpix_step.py	66.66%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8143      +/-   ##
==========================================
+ Coverage   75.15%   75.38%   +0.22%     
==========================================
  Files         470      474       +4     
  Lines       38604    38904     +300     
==========================================
+ Hits        29014    29326     +312     
+ Misses       9590     9578      -12     

Flag	Coverage Δ		*Carryforward flag
nightly	77.34% <ø> (-0.06%)	⬇️	Carriedforward from 4cc0ac1

*This pull request uses carry forward flags. Click here to find out more.

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

[melanieclarke]

I think this is ready for review now, if someone can run regression tests for me. I expect changes only for NIRSpec IRS2 data, downstream of the refpix step. In local runs of the regression tests, I see improvements to alternating column noise for IRS2 data, compared to the truth files.

[tapastro]

Started a regression set: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1116/

[melanieclarke]

It looks like there are some unrelated failures in the regression tests, for MIRI header keywords and a couple other things. The NIRSpec changes are as expected.

[hbushouse]

Started another regtest run here: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1127 Truth files have been updated for recent ref file updates, so unrelated failures should go away now.

[hbushouse]

Code updates look good to me. Do we need to make updates in the refpix step docs (RTD) for this change to the way IRS2 data are handled?

[melanieclarke]

Thanks @hbushouse. I'll take a look at the docs.

I also need to take another look at how even and odd interleaved pixels are identified, to address a comment in the ticket. I'll put this PR back to draft status while I work on it.

[hbushouse]

All regtest failures in the latest run are now confined to NRS tests and appear to be small differences in array values, as expected.

[melanieclarke]

@hbushouse - I think this is ready for review again. I added some new handling for determining even/odd for interleaved reference pixels and updated the docs for IRS2.

[melanieclarke]

Apologies, I'm setting this back to draft again, because testing shows that this change with current reference files sometimes introduces alternating column noise where it didn't exist before. This needs more study before merging this change.

[hbushouse]

@melanieclarke All the differences in the latest regtest run are unrelated and due to the recent addition of a new keyword in stdatamodels. Is it expected that we won't see any changes to NIRSpec images, because the changes included here are off by default?

[melanieclarke]

Yes, that's right. There should be no changes to default reduction. We'll see if we can/should turn it on by default when current IRS2 reference file investigations are done.

[hbushouse]

OK, so in that case I think this is ready to merge.

[melanieclarke]

Thanks very much for reviewing, @hbushouse!

[melanieclarke]

@hbushouse - Just checking -- it looks like this was scheduled to auto-merge but did not actually go through. Is it waiting for something else?

[hbushouse]

@hbushouse - Just checking -- it looks like this was scheduled to auto-merge but did not actually go through. Is it waiting for something else?

When I try to merge I get an error message saying "Merge attempt failed. Head branch is out of date. Review and try the merge again." Never seen that before! Not even sure what "head branch is out of date" might mean. Perhaps do a rebase of your local PR branch and trying pushing one last time?

[melanieclarke]

That's odd. I rebased from main and pushed again, but I still see a "Head branch was modified" message above. Maybe it needs you to approve again?

[hbushouse]

Yay, successfully merged this time!