-
Notifications
You must be signed in to change notification settings - Fork 174
JP-4024: Set negative NIRSpec flat fields to NaN and DNU #9522
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9522 +/- ##
==========================================
- Coverage 78.90% 78.90% -0.01%
==========================================
Files 36
8000
5 365
Lines 37213 37212 -1
==========================================
- Hits 29363 29362 -1
Misses 7850 7850 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just a couple questions about whether we should be using NaN instead of 1.0 in other places. One below, and the other:
Since the cal factor is set in the f-flat, should the fore_optics_flat
also use NaN for bad values? I think those shouldn't leak through, since DQ is set for those pixels, but there's no reason to set a definitely wrong value there.
Co-authored-by: Melanie Clarke <mclarke@stsci.edu>
Good point, I'll track those down and set them to NaN as well, no sense in using a wrong default fill value (especially if they're set to DNU anyway). |
@melanieclarke thanks for taking a look already, I went ahead and made the changes you suggested, and have pulled this out of draft. Let me know if there's anything else you think should be addressed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regtest results look as expected - a handful of new NaNs, some changes to DQ values. Thanks for the fix!
Thanks for the fast review! |
Resolves JP-4024
This PR updates the flat_field behavior for handling negative flat field values for NIRSpec data. Instead of setting the flat field to 1 for any pixels with a negative combined flat, this PR sets the flat field to NaN and flags the pixels with DNU (and adds the remaining bad flat flags). NIRSpec flats include flux calibration (sitting around a value of order ~ 1e10) so a flat field value of 1 is not a good fall back for the NIRSpec combined flat field. The previous behavior was resulting in significant outliers in some NIRSpec data, so it would be better to set any such pixels to NaN/DNU.
From local testing I expect that this will change a small number of outlier pixels to NaN in spec2 and spec3/tso3 products across all NIRSpec modes (for an example IFU data set this was on the order of two dozen pixels) but will also update the DQ flags a larger number of pixels that were already marked as DNU for other reasons (e.g., dead pixels for which the flat fields may be negative). Since this only updates the NIRSpec specific flat field functions it shouldn't affect any other instruments.
Tasks
Build 12.0
(use the latest build if not sure)no-changelog-entry-needed
)changes/
:echo "changed something" > changes/<PR#>.<changetype>.rst
(see changelog readme for instructions)docs/
pageokify_regtests
to update the truth files