Skip to content
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

Update code to update WPFC2 chip gaps #1551

Merged
merged 3 commits into from
Jul 14, 2023

Conversation

stsci-hack
Copy link
Contributor

These simple changes to the SVM and MVM astrodrizzle parameters for WFPC2 data removes the extraneous NaNs that pockmark WFPC2 SVM and MVM filter level drizzle products. This works by treating pixels with DQ=2 as good across WFPC2 images, while the astrodrizzle code handles the DQ=2 pixels which define the vignetted regions of each chip using the `buildShadowMask()' function.

@mdlpstsci
Copy link
Collaborator

@mackjenn Warren has worked this issue. He would like you to review the "final_bits" parameter values for WFPC2 data, as he does not believe it makes sense to have 4096 included. The "final_bits" may just be set to have all but 4096 included... maybe. That's what Jen should review. Including 2 is necessary to fix the NaN values.

@mackjenn
Copy link

mackjenn commented May 8, 2023

@stsci-hack Thanks for investigating! Can you provide a screenshot of the updated DRZ image for comparison with the HLA verison?

Regarding DQ flags, we have different 'final_bits' values depending on the number of input frames:
For N=1, "final_bits": "65535",
For N=2 or more, "final_bits": "8,1024"

Simply adding 2 to the current final_bits values may fix the Nan values, but those are actually bad pixels and should not be used. Since there are other bad pixel flags that do not cause large NaN regions, I'd like to verify that the problem is not related to these pixels also being flagged as CR's and somehow being grown larger than 1 pixel in the driz_cr step.

If I can fix this in a different way, I'll let you know!

@stsci-hack
Copy link
Contributor Author

The NaNs which show up are all flagged as DQ=2 in the original C0M(FLT) files and are already expanded out to 3x3 blocks in the DQ (C1M file). Those blocks of NaN pixels are only flagged with DQ=2, and no other DQ value. The fact that most WFPC2 data is not dithered means that these hot pixels flagged in one image will also be flagged in the other images, so that when treated as 'bad', there is no data in that part of the final mosaic. This is exactly what is happening with these pixels, since hot pixels are detector related, and not random like CRs, since 'final_fillval' is set to 'null'. The result of adding DQ=2 to the 'final_bits', and the designation of DQ=2 pixels can be see in the attached image. The top left shows the SCI array of the FLT image, the top right is the DQ array for the same pixels, and the bottom is the DRZ for the same region. The light gray in the DQ array are DQ=2 pixels which show up as the blocks of NaNs in the original result, whereas only the single pixel CR gets set to NaN now.
wfpc2_u2as0107_flt_SCI_DQ_DRZ

So, we have two options: add 2 to the 'final_bits' value or reset 'final_fillval' to 'INDEF'. Either way, those 'bad' pixels flagged with DQ=2 value in the science portion of the images will end up in the output image, whereas now they are ending up as blocks of NaN values.

@mcara
Copy link
Member

mcara commented May 10, 2023

What actually prompted this PR? Is that NaNs are undesirable in the drizzled images and why? If so, is it OK to consider hot pixels as "good pixels" just for the sake of getting pretty images? Shouldn't this get approval from INS? The alternative would be to set fill value to some finite value. This is also not ideal but better than proposed (in this PR) approach because, for the very least one can look at the weight map and ignore the arbitrarily sett fill value for pixels with 0 weight (or a very small weight). Making a pixel "good" renders this approach unusable.

IMO it is much better to leave NaNs alone and deal with them during data analysis.

@mdlpstsci
Copy link
Collaborator

@mcara Appreciate your comments as it is good to keep the proper perspective. Jenn from WFC3 is looking into this issue in conjunction with Warren to come up with the appropriate solution.

@codecov
Copy link

codecov bot commented Jul 5, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (029935f) 32.35% compared to head (df31a61) 32.36%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1551   +/-   ##
=======================================
  Coverage   32.35%   32.36%           
=======================================
  Files         159      159           
  Lines       35059    35053    -6     
=======================================
  Hits        11345    11345           
+ Misses      23714    23708    -6     
Impacted Files Coverage Δ
drizzlepac/buildmask.py 21.05% <ø> (+1.05%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@mackjenn
Copy link

mackjenn commented Jul 6, 2023

This PR is the result of me misinterpreting the atypical F336W dataset u2as01, which has larger (3x3) blocks of bad pixels (DQ=2), compared to longer wavelength filters, where the DQ=2 flags are only a single pixel.

In the HLA drizzled image, final_scale is 0.1"/pixel so the NaN regions get slightly filled in at the corners and appear smaller (less blocky) than the NaN regions in the HAP drizzled to 0.05"/pixel which threw me off.

NaNs are expected when treating DQ=2 as bad which is what we want to do.

The real problem is that final_fillval=INDEF no longer works in AstroDrizzle, which is what we want for N=1 or 2 frames. That would solve the issue with filling in the NaN pixels (not leaving a swiss-cheese image for CR-split data), while still setting the WHT image to zero so that users can make a mask prior to doing photometry.

@stsci-hack stsci-hack changed the title Update pars to remove WPFC2 NANs Update code to update WPFC2 chip gaps Jul 6, 2023
Copy link
Collaborator

@mdlpstsci mdlpstsci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes approved as we discussed with the remainder of the work to be done under HLA-1061.

@stsci-hack stsci-hack merged commit 00022f6 into spacetelescope:master Jul 14, 2023
@stsci-hack stsci-hack deleted the remove_wfpc2_nans branch July 14, 2023 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants