JP-1663: NIRSpec MOS flat field DQ handling #5284
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5284 +/- ##
==========================================
- Coverage 52.76% 52.67% -0.09%
==========================================
Files 406 407 +1
Lines 36788 36898 +110
Branches 5709 5724 +15
==========================================
+ Hits 19411 19436 +25
- Misses 16139 16224 +85
Partials 1238 1238
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
9bdc351 to
4f4f7ef
Compare
| # their DQ to NO_FLAT_FIELD | ||
| # Find pixels in the flat that have a value of NaN and add to | ||
| # DQ to DO_NOT_USE + NO_FLAT_FIELD | ||
| bad_flag = dqflags.pixel['DO_NOT_USE'] + dqflags.pixel['NO_FLAT_FIELD'] |
There was a problem hiding this comment.
We are only supposed reject pixels where the DQ flag is set to DO_NOT_USE, right? Why are we doing the work in code here that should be done by the creators of the reference file?
I.e. if a pixel has NaN in the reference file, it should also have DO_NOT_USE set for that pixel. All other bits set are informational only.
Same applies to the many instances of this below in the code.
There was a problem hiding this comment.
Probably we can take this out once the flat field reference files are made correctly. There is no flagging of DO_NOT_USE. But it does not harm - if the flat field is a 0 or a NaN it should not be use by the code - this is just a second level of checking.
There was a problem hiding this comment.
Sometimes you just gotta do what you gotta do, in order to cover all bases, just in case someone else didn't do what they gotta do.
There was a problem hiding this comment.
My only worry is that because the code does this "fixing" of the reffile, it won't be obvious if incorrect reference files keep getting delivered. We won't know. And given the complexity and the shear number of instances of this in the flatfield code, since we are addressing all of these here and now, we will have to go back and change every instance of this a 2nd time if we want to verify that we have correct reffiles.
There was a problem hiding this comment.
There was a problem hiding this comment.
Well I put the 2nd level of checks for that exact reason. Actually the checks were already there but used a different DQ flag. As someone who will be making reference files during commissioning I think it is going to be a stressful and crazy time. I could see some essential flagging not being set correctly. But it would only add to the stress if the pipeline failed - The pipeline should be smart enough to know that a flat value of 0 of NAN should not be used. The NIRSpec team is going to redeliver reference files but that might take a while. Having a common sense checks in the pipeline seems ok to me. - That is my two cents
jwst/flatfield/flat_field.py
Outdated
| flat_zero = np.where(flat_data == 0.) | ||
| flat_dq[flat_zero] = np.bitwise_or(flat_dq[flat_zero], | ||
| dqflags.pixel['NO_FLAT_FIELD']) | ||
| flat_dq[flat_zero] = np.bitwise_or(flat_dq[flat_zero],bad_flag) | ||
|
|
||
| # Find all pixels in the flat that have a DQ value of NO_FLAT_FIELD |
There was a problem hiding this comment.
Need to update the comment here to stay in sync with code change.
jwst/flatfield/flat_field.py
Outdated
| flat_zero = np.where(f_flat == 0.) | ||
| f_flat_dq[flat_zero] = np.bitwise_or(f_flat_dq[flat_zero],bad_flag) | ||
|
|
||
| # Find all pixels in the flat that have a DQ value of NO_FLAT_FIELD |
jwst/flatfield/flat_field.py
Outdated
| flat_zero = np.where(flat_2d == 0.) | ||
| s_flat_dq[flat_zero] = np.bitwise_or(s_flat_dq[flat_zero],bad_flag) | ||
|
|
||
| # Find all pixels in the flat that have a DQ value of NO_FLAT_FIELD |
jwst/flatfield/flat_field.py
Outdated
| flat_zero = np.where(flat_2d == 0.) | ||
| d_flat_dq[flat_zero] = np.bitwise_or(d_flat_dq[flat_zero],bad_flag) | ||
|
|
||
| # Find all pixels in the flat that have a DQ value of NO_FLAT_FIELD |
jwst/flatfield/flat_field.py
Outdated
| bad2 = np.bitwise_and(dq_list[1], dqflags.pixel['DO_NOT_USE']) | ||
| iflag = np.where( (bad1==1) & (bad2 ==1)) | ||
|
|
||
| # locate places that flat_dq is set to DO_NOT_USE and reset to UNRELIABLE FLAT |
There was a problem hiding this comment.
Why are we unsetting DO_NOT_USE values? And if we do, why not reset them to NO_FLAT_FIELD, as opposed to UNRELIABLE_FLAT? If they were originally marked as DO_NOT_USE, they must be really bad, got reset to 1, and hence no flat-field value was applied to the science data.
There was a problem hiding this comment.
After discussions with James Muzerolle on how to combine the s-flat, d-flat and f-flat (see JP-1663 Aug 31 - Sept 1)
the 3 flats are independent of each other. Just because 1 flat has an DO_NOT_USE value it does not mean that the combined flat should be flagged as DO_NOT_USE. It should only be DO_NOT_USE if all 3 flats are do not use. So if one of the flats was flagged as do_not_use but the others were not we changed the final DQ value to be UNRELIABLE_FLAT
jwst/flatfield/flat_field.py
Outdated
| bad3 = np.bitwise_and(dq_list[2], dqflags.pixel['DO_NOT_USE']) | ||
| iflag = np.where( (bad1==1) & (bad2 ==1) & (bad3 ==1)) | ||
|
|
||
| # locate places that flat_dq is set to DO_NOT_USE and reset to UNRELIABLE FLAT |
| iloc = np.where(np.bitwise_and(flat_dq, dqflags.pixel['DO_NOT_USE'])) | ||
| flat_dq[iloc] = dqflags.pixel['UNRELIABLE_FLAT'] | ||
| # now only set DO_NOT_USE to pixels that are set in both flats as DO_NOT_USE | ||
| flat_dq[iflag] = np.bitwise_or(flat_dq[iflag],dqflags.pixel['DO_NOT_USE']) | ||
| elif n_dq == 3: |
There was a problem hiding this comment.
Isn't there some more elegant way to do all of this checking of dq values from any of 1, 2, or 3 flats, like just a single loop that has a range of n_dq and combines values from all the individual dq arrays that exist? As opposed to having hardwired code blocks to handle each instance of 0, 1, 2, or 3 dq arrays.
There was a problem hiding this comment.
I can look into this. To me there is a balance between elegance and knowing quickly what the code is doing to tracking down errors. I went cautious and tried to make it very clear what the code was doing.
4558e0e to
d0b2d1f
Compare
|
@hbushouse |
This PR set the various flat components (d_flat, f_flat, s_flat) to 1 if the input data is a NAN or the DQ flags are set to DO_NOT_USE + NO_FLAT_FIELD.
Only the DQ flag of DO_NOT_USE is used to determine if the the flat data should be used to create the interpolated NIRSpec flat.
Resolves #5293 / JP-1663