Jp-3207 ifu outlier detection

[jemorrison]

Resolves JP-3207

Closes #

This PR changes the way outlier detection is done on IFU data, implementing a completely new algorithm developed by @drlaw1558. A change to the outlierpars data models was needed [spacetelescope/stdatamodels#164 for stdatamodels]. In addition, a new data model outlierifuoutput was added to stdatamodels to support analysis of the new algorithm. If save_intermediate_results = True then an output file following outlierifuoutput is created.

Order of work flow:

1. Get PR 164 in stdatamodels merged. (Done)
2. Get PR 167 in stdatamodels merged (Done)
3. Add the new NIRSpec and MIRI outlierpars reference files. [if not in CRDS will use defaults set in outlier_detection. We can use these defaults for NIRSpec get the PR merged and later load the NIRSpec ref file if they need more time]
4. Merge this PR.

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

[jemorrison]

@hbushouse @tapastro I am stuck. I have the ifu outlier rejection code flagging outliers. The way the outlier_detection_step is set up I can not figure out how to update the input models. with the flagged dqs The flag dq arrays and corresponding sci arrays (that are nan) are not returned so the output files written are not flagged.
I am going continue to work on this but could use a fresh set of eye on the problem
The outlier_detection_step is set up use in class defined in outlier_detection.py
Depending on the type of data a different module is called. In IFU data the outlier_detection_ifu.py step is called
All the modules have a do_detection(self) module that does the bulk of the work.

I can not figure out the way that outlier_detection is set up how to return the updated arrays. I have looked in detail at how
outlier_detection.py works and tried to do something similar but it just does not work.

[codecov]

Codecov Report

Patch coverage: 5.50% and project coverage change: -0.26 ⚠️

Comparison is base (991678e) 77.27% compared to head (0ae1eb7) 77.01%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7590      +/-   ##
==========================================
- Coverage   77.27%   77.01%   -0.26%     
==========================================
  Files         456      456              
  Lines       36629    36625       -4     
==========================================
- Hits        28304    28208      -96     
- Misses       8325     8417      +92     

Flag	Coverage Δ		*Carryforward flag
nightly	77.44% <ø> (-0.05%)	⬇️	Carriedforward from 94f6f13

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

Impacted Files	Coverage Δ
jwst/outlier_detection/outlier_detection_step.py	87.87% <ø> (-0.13%)	⬇️
jwst/outlier_detection/outlier_detection_ifu.py	16.26% <4.62%> (-75.01%)	⬇️
jwst/outlier_detection/outlier_detection.py	89.90% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[jemorrison]

@hbushouse I did not mean to put requirements-max.txt into this PR. I think I removed it. But could you check I did that correctly before we merge

[jemorrison]

@hbushouse So how do we record changes made in datamodels in stdatamodels ? I put a note under datamodels and I put a note in the stdatamodels CHANGE log.

[hbushouse]

@hbushouse So how do we record changes made in datamodels in stdatamodels ? I put a note under datamodels and I put a note in the stdatamodels CHANGE log.

You can include a link to the stdatamodels PR using the following syntax:

[#7590, spacetelescope/stdatamodels#164]

[drlaw1558]

Looks good to me; I've tested it on a few example data sets and it seems to be working as expected.

[drlaw1558]

pars files are ready for both outlier_detection and an updated spec3. The spec3 file contains updated extract1d calls too for the MRS PR on extract1d changes.
Question though: when these pars files go live, they contain keywords not in the current MAST pipeline, and offline testing suggests that this will cause crashes in old code versions. How should we deal with that?

[nden]

@jemorrison I believe requirements-max.txt is removed erroneously in this PR.

Also have you run the code with at least two different detectors? I'm trying to figure out the logic.

[braingram]

While looking at the open files issues in stdatamodels and jwst I noticed that this PR should include an update to:
https://github.com/spacetelescope/jwst/blob/master/jwst/datamodels/__init__.py
as the jwst test suite compares stdatamodels.jwst.datamodels to jwst.datamodels

jwst/jwst/datamodels/tests/test_api.py

Line 30 in 21c7b51

def test_stdatamodels_api(model):

and the addition of a new submodule and model in spacetelescope/stdatamodels#164 will cause the above test to fail.

[jemorrison]

@nden I have run this code on NIRSpec and MIRI data using an association for each instrument that contains 2 detectors. The algorithm pulls the data from 1 detector and is basically trying to find bad pixels that are not in the bad pixel mask because the bad pixels vary with time. So it looks for outliers with adjacent pixels that are outliers on all then input data for that detector.

[jemorrison]

@nden I needed requirements-max.txt locally to be changed so that I could use the newer version that I created to update the outlierParsModel. I added that to the PR by mistake. I guess I did not mean to remove requirements-max.txt completely just my update to it. Is that what you are saying I did ?

[jemorrison]

@nden made the changes you requested. I remove a few things that were not needed. I am going to run the regression tests now- more just to make sure nothing breaks. The regression results for IFU outlier detection will be completely different

[jemorrison]

@nden @hbushouse
This PR should now be ready for final review. Thanks for all the suggestions it has made this step better (and I learned a few things along the way).

[hbushouse]

Another (hopefully final?) regtest run started at https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/743/

[hbushouse]

Not sure I understand why the CI tests are still failing due to some kind of error with not finding jwst.datamodels.outlierifuoutput.py, even though that module is now included in this PR and I think it also has the appropriate change to jwst/setup.cfg to use stdatamodels/master. Everything seems to run fine when using this PR branch in regression tests, so maybe it's just something unique in what the CI tests are trying to test. @zacharyburnett @braingram any ideas?

[braingram]

Not sure I understand why the CI tests are still failing due to some kind of error with not finding jwst.datamodels.outlierifuoutput.py, even though that module is now included in this PR and I think it also has the appropriate change to jwst/setup.cfg to use stdatamodels/master. Everything seems to run fine when using this PR branch in regression tests, so maybe it's just something unique in what the CI tests are trying to test. @zacharyburnett @braingram any ideas?

I believe the CI is throwing errors because the file jwst/datamodels/outlierifuoutput.py added in the PR conflicts with the module shadowing added in #7605 jwst/datamodels/outlierifuoutput.py can be removed from this PR which should fix the error.

The 2 errors are because pytest is inspecting jwst/datamodels/outlierifuoutput.py to look for tests. After it attempts to import the file (which instead triggers importing the corresponding file from stdatamodels), the module appears at a different path (because of the changes in #7605) and pytest gives up with an error.

I believe the regression tests aren't showing the same error because they don't search the jwst/datamodels directory for tests (assuming it's using the pytest --bigdata jwst/regtest in the readme.

Let me know if it's helpful to document any of this. There is an open issue about improving the documentation on adding a new datamodel which is likely related: #4436

[hbushouse]

@braingram OK, I'll try removing the outlierifuoutput.py module from this PR and see what happens.

[hbushouse]

Yay! CI tests now succeed. Thanks @braingram

[hbushouse]

Looks like the doc module https://raw.githubusercontent.com/spacetelescope/jwst/master/docs/jwst/outlier_detection/outlier_detection.rst also has some references to IFU mode, which will need updating. If you don't mind, I'll take a stab at updating that and pushing to this PR branch.

[hbushouse]

Woot! CI success. As soon as the latest regtest run at https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/751/console finishes, I'll be merging this.