WIP: JP-3248 MIRI Subarrays 390 Hz EMI

[penaguerrero]

Resolves JP-3248

Closes #7729

This PR addresses the creation of a new step to correct MIRI images for EMI contamination.

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: 59 lines in your changes are missing coverage. Please review.

Comparison is base (bd3558e) 75.18% compared to head (2edc01d) 75.24%.

Files	Patch %	Lines
jwst/emicorr/emicorr.py	90.29%	26 Missing ⚠️
jwst/emicorr/emicorr_step.py	36.58%	26 Missing ⚠️
jwst/extract_2d/nirspec.py	0.00%	5 Missing ⚠️
jwst/pipeline/calwebb_dark.py	50.00%	1 Missing ⚠️
jwst/pipeline/calwebb_detector1.py	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7857      +/-   ##
==========================================
+ Coverage   75.18%   75.24%   +0.05%     
==========================================
  Files         467      470       +3     
  Lines       38102    38420     +318     
==========================================
+ Hits        28647    28908     +261     
- Misses       9455     9512      +57     

Flag	Coverage Δ		*Carryforward flag
nightly	77.37% <ø> (ø)		Carriedforward from bd3558e

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

[penaguerrero]

Need to request a new pmap for CRDS so that the code knows about any potential entry for the new step.

[hbushouse]

Only gotten through the first half of the files so far. More to come ...

[hbushouse]

OK, finished going through everything now. That's a lot of new code! Mostly minor comments everywhere.

[penaguerrero]

The docs now build but I had to remove the % and type the word instead... I don't like that solution tho.

[hbushouse]

I committed more reformatting of that rst file and the percent sign now works OK. I think it was really the bad formatting of the numbered list that was screwing it up. See https://jwst-pipeline--7857.org.readthedocs.build/en/7857/jwst/emicorr/description.html

[hbushouse]

Started a regtest run at https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1097

[hbushouse]

Everything here looks good, but seeing a number of errors in the regtest run. Will need to wait till they finish to investigate. Hopefully they're unrelated.

[hbushouse]

Rats. All the errors in the regtest run are due to the use of CRDS-OPS, which doesn't yet have definitions for the new "emicorr" reference file type, hence all the calls to retrieve the ref file throw an error. Need to switch to using CRDS_TEST. Will start a new regtest using that.

[penaguerrero]

the docs look good except for the formatting of the reference file description, I think i need to add a white line so that the ASDF tree looks like we want

[penaguerrero]

ok, the docs built and the tree looks ok but not pretty: https://jwst-pipeline--7857.org.readthedocs.build/en/7857/jwst/emicorr/reference_files.html

[hbushouse]

ok, the docs built and the tree looks ok but not pretty: https://jwst-pipeline--7857.org.readthedocs.build/en/7857/jwst/emicorr/reference_files.html

You could try enclosing the tree text in a literal block. To do that, end the preceding paragraph with "::" and then have your ascii text tree indented, with a blank line before and after, e.g.

This is my intro text::

  This block of text, which is indented,
  should get rendered as a literal, with no formatting.

  Keep indenting each line that you want included in the
  literal block.

Then revert to normal text.

[penaguerrero]

yay! that worked! https://jwst-pipeline--7857.org.readthedocs.build/en/7857/jwst/emicorr/reference_files.html
thanks!

[hbushouse]

Still getting errors in regtests, even when using emicorr ref file from CRDS_TEST. Here's what I've found:

test_miri_image_detector1, in emicorr.py:

                        if colstop == 258:
>                           times_this_int[k, j, nx4-1] = times_this_int[k, j, nx4-1] + ref_pix_sample.astype('ulonglong')
E                           AttributeError: 'int' object has no attribute 'astype'

test_miri_dark is complaining that it can't find an emicorr ref file in CRDS. This might be due to the test data file having a DATE-OBS that's older than 2022-04-01, which is the USEAFTER on the emicorr ref file. Which, in my opinion is stupid. Why not have the USEAFTER go all the way back to cover all possible existing MIRI data. Surely the EMI problem did not just start in April 2022.

[hbushouse]

@penaguerrero Please try running jwst/regtest/test_miri_image.py locally to debug why it's crashing.

See https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1098/testReport/jwst.regtest/test_miri_image/_stable_deps__test_miri_image_detector1_dq_init_/ for the traceback we're getting in the regtest.

[penaguerrero]

ok! Reg tests ran with some failures but not associated to this PR: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1099/#showFailuresLink

[hbushouse]

ok! Reg tests ran with some failures but not associated to this PR: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1099/#showFailuresLink

This run is again showing all of the "E crds.core.exceptions.CrdsLookupError: Error determining best reference for 'emicorr' = Unknown reference type 'emicorr'" errors, even for MIRI datasets, presumably because it was run using CRDS Ops, but the new emicorr ref file only exists in CRDS_TEST. Need to run again with "CRDS_PATH=/grp/crds/jwst/test" set in the "ENV_VARS" box that comes up after you click on "Build with Parameters" to start the run.

[hbushouse]

Woot! Regtest run https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1101/ shows differences (but no crashes) in the MIRI tests that use subarrays due to new keywords:

     Extra keyword 'R_MIREMI' in a: 'crds://jwst_miri_emicorr_0001.asdf'
     Extra keyword 'S_MIREMI' in a: 'COMPLETE'

and slight changes to pixel values, so that looks like the step is running!

[hbushouse]

I hereby approve this PR.