-
Notifications
You must be signed in to change notification settings - Fork 174
JP-3786: Remove hard assigning NON_SCIENCE to DO_NOT_USE in flatfield #9174
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
Starting regtests: https://github.com/spacetelescope/RegressionTests/actions/runs/13268302727 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9174 +/- ##
==========================================
- Coverage 73.68% 73.68% -0.01%
==========================================
Files 372 372
Lines 37098 37096 -2
==========================================
- Hits 27335 27333 -2
Misses 9763 9763 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Regression tests look fine - the reported failures are unrelated. For clean_flicker_noise, the check for MIRI non-science regions to exclude from the scene uses DO_NOT_USE flags in the flat file. See here: This was modified from the original draft, which used NON_SCIENCE flags for this purpose, because some MIRI flats used NON_SCIENCE and DO_NOT_USE and some used DO_NOT_USE only. Should we change the clean_flicker_noise code back to NON_SCIENCE? Or maybe NON_SCIENCE | DO_NOT_USE? |
@melanieclarke Ah, that sounds familiar. I don't think we'd want to use anything in the slit region for destriping, so that would be NON_SCIENCE | DO_NOT_USE. I've updated the PR. |
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.
Thanks for the update. This looks fine to me. @tapastro - did you want to review?
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.
Updates look good to me.
This PR partially resolves JP-3786 regarding the need to propagate the MIRI LRS slit region to the cal.fits stage in MIRI imaging data. Two steps are necessary for this:
Note that the current resample step still sets NON_SCIENCE to DO_NOT_USE, but this is desirable to ensure that this new slit region doesn't make it into MIRI imaging mosaics. Potentially changing this behavior to a more-general approach to handling DQ flags is separable and doesn't need to be considered here.
Based on testing all 637 MIRI+NIRCAM+NIRISS flatfield reference files used by the latest CRDS rmaps (NIRSpec uses different flatfield code and thus shouldn't be relevant), it looks like none of them currently contain any pixels for which NON_SCIENCE is set but DO_NOT_USE or NO_FLAT_FIELD are not. As such this change shouldn't have any unintentional side effects.
However; the way the code was written it looks like mapping NON_SCIENCE to DO_NOT_USE was done on the science dq array rather than just the flatfield dq array, meaning that any earlier step that had the possibility of setting NON_SCIENCE may have an effect here. A quick grep of the code doesn't show any obvious conflicts, although it's possible that some detector1 reference file for some instrument mode may contain pixels with just NON_SCIENCE flags that could be affected.
pytest didn't raise any problems at the unit test level (except for one clean_flicker_noise test that is presumably unrelated since it isn't using the apply_flat_field argument in this test), but if someone could please set of a reg test run it would be useful to see if that turns up any issues.