JP-824: Add option to save combined background image

[tapastro]

Closes #3746

Resolves JP-824

Description

This PR adds the option to save the combined background image model, in a method pulled directly from the optional flat_field output. Currently will not work as written, oops. Do we really need to close that bkg_model?

Checklist

• Tests
• Documentation
• Change log
• Milestone
• Label(s)

[codecov]

Codecov Report

Merging #5954 (699fe54) into master (434c540) will increase coverage by 0.04%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##           master    #5954      +/-   ##
==========================================
+ Coverage   77.44%   77.49%   +0.04%     
==========================================
  Files         404      404              
  Lines       35373    35552     +179     
==========================================
+ Hits        27395    27551     +156     
- Misses       7978     8001      +23     

Flag	Coverage Δ		*Carryforward flag
nightly	77.45% <100.00%> (ø)		Carriedforward from 9442819
unit	56.14% <71.42%> (+0.07%)	⬆️

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

Impacted Files	Coverage Δ
jwst/background/background_step.py	91.37% <75.00%> (-8.63%)	⬇️
jwst/background/background_sub.py	86.61% <100.00%> (-9.09%)	⬇️
jwst/outlier_detection/outlier_detection_step.py	82.26% <0.00%> (-4.65%)	⬇️
jwst/wfs_combine/wfs_combine_step.py	97.77% <0.00%> (-2.23%)	⬇️
jwst/outlier_detection/outlier_detection.py	91.15% <0.00%> (+6.08%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 434c540...699fe54. Read the comment docs.

[hbushouse]

Code looks good. Should update the background step docs to mention this, and of course the requisite change log entry.

[hbushouse]

I think we might have at least 1 regtest that applies the background step during calwebb_spec2 processing (might be a MIRI LRS slit observation?), which could be updated to include saving the background and doing a comparison on it. Then you can even check off the Tests box above!

[jdavies-st]

Yeah, the regression test is

jwst/jwst/regtest/test_miri_lrs_slit_spec2.py

Lines 28 to 34 in 434c540

	args = ["config/calwebb_spec2.cfg", rtdata.input,
	"--steps.resample_spec.skip=true", # remove when bug fixed
	"--save_bsub=true",
	"--steps.assign_wcs.save_results=true",
	"--steps.flat_field.save_results=true",
	"--steps.srctype.save_results=true"]
	Step.from_cmdline(args)

Also, looking at the test, it seems we are skipping resample in that test for the following reason:

#4373

We should turn resampling back on, and test the s2d file.

[hbushouse]

#4373 Ah, from the great regtest marathon, when all @hbushouse and @jdavies-st did for Christmas was write tests! Those were the days. ;-)

[tapastro]

@hbushouse @jdavies-st It took me a bit to find some code I could test this with, having forgotten to check the regtest data first. Using the data from the test mentioned above, I'm getting an error when attempting the save_model call; the traceback goes through many lines of of astropy fits io but ends in a numpy core line with "NotImplementedError: MaskedArray.tofile() not implemented yet." Has this been seen before? Is some model massaging required? It seems as though the culprit is the err array in the ImageModel. I can't believe this is the first time an err is being saved...

[stscieisenhamer]

Need changelog!

[tapastro]

Need changelog!

@stscieisenhamer , do you have a suggestion on the comment I made above regarding issues printing an ImageModel with an included MaskedArray to file?

[stscieisenhamer]

Sorry, missed that comment. The issue is not that an err array is being saved, but this is a masked err array. Others a bit more knowledgeable of the code base may know different, however I do not believe any of the DataModel arrays can be numpy masked arrays. The masking is explicit in the dq array, and all other data arrays should be non-masked. This may be the first time an attempt at saving a DataModel with masked arrays has been attempted, and the claim is that all arrays should be unmasked and that the dq array is consistent.

[hbushouse]

So the obvious next question is where/how one of the arrays in the background model is ending up as a masked array. Have you been able to figure that out? Perhaps by whatever routine is being used to create the combined/sigma-clipped array?

[tapastro]

So the obvious next question is where/how one of the arrays in the background model is ending up as a masked array. Have you been able to figure that out? Perhaps by whatever routine is being used to create the combined/sigma-clipped array?

Finding that wasn't too difficult - the err array is explicitly initialized as a masked array. I've been futzing around with the output though, to understand exactly how I should be "collapsing" it - the mask is in place to utilize the results of a sigma_clip on the background data. The combined err does an uncertainty calculation, but it calls the masked array without using .data or .mask, and I'm not sure exactly what is being fed into the line (Line here). If I were to force-apply the mask to the data, is the goal of the sigma_clip to remove those entries from the error calculation?

[hbushouse]

Looks like this latest update should fix the model saving problem. Can remove the 1 commented line.

Still needs a change log entry and step docs update.

[hbushouse]

Looks good. All the boxes checked!

change log has been updated