spacetelescope / jwst

Python library for science observations from the James Webb Space Telescope
https://jwst-pipeline.readthedocs.io/en/latest/
Other
545 stars 158 forks source link

JP-3523: Use zero background instead of NaN background where undefined #8597

Closed drlaw1558 closed 1 week ago

drlaw1558 commented 1 week ago

Resolves JP-3523 by having IFU master background default to zero background values beyond the wavelength range of the input backgrounds, instead of automatically NaN-ing out all such wavelength and causing crashes in cube building.

Closes #8244

Checklist for PR authors (skip items if you don't have permissions or they are not applicable)

codecov[bot] commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 59.55%. Comparing base (23fb5cd) to head (eb65e0b). Report is 1 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #8597 +/- ## ========================================== + Coverage 59.54% 59.55% +0.01% ========================================== Files 391 391 Lines 39292 39283 -9 ========================================== Hits 23396 23396 + Misses 15896 15887 -9 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

hbushouse commented 1 week ago

Style check is showing that min_wave and max_wave are not used anymore and hence can be removed.

hbushouse commented 1 week ago

@drlaw1558 Also, please rebase your PR branch, so that it pulls in new definitions for the stdatamodels package, which had a new release come out yesterday. Without that update, we get ~200 unrelated failures in regression tests.

drlaw1558 commented 1 week ago

@hbushouse Fixed the variables, merged master into the branch, and rebased. Had to force push after the rebase for some reason.

hbushouse commented 1 week ago

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

hbushouse commented 1 week ago

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

All failures are for IFU master_background tests, which makes sense, but the only differences showing up are in the DQ arrays where some pixels that used to have a DO_NOT_USE flag set no longer do. I don't see any differences in resulting pixel values. Could just be a limitation of the tests, in that maybe they don't have any wavelength gaps in the bkg coverage.

hbushouse commented 1 week ago

Looking at the code changes in detail again, I can confirm that as far as the changes in this one step are concerned, the only difference is that DO_NOT_USE flags are no longer set in the DQ array of the output background image, without any changes to the actual pixel values in the background images. So the regression test results seem consistent with that.