-
Notifications
You must be signed in to change notification settings - Fork 171
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
JP-3035: Fix computation of the photometric keywords PIXAR_* and scale resampled intensities according to actual pixel scale ratio. #7894
Conversation
Codecov ReportAttention:
📢 Thoughts on this report? Let us know!. |
c072859
to
5730d3d
Compare
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.
I think this looks OK. I worry a bit, though, about rescaling the intensity of the output images based on a scale factor derived from one of the PIXAR_xx keywords in the input images. Since those keyword values are just some kind of average or nominal pixel area, and are only as accurate as the person who computed them and put them into a ref file chose to be, it seems like it could be risky.
I think these are some very very good questions!
Simple MathFor simplicity, let's imagine that all input pixels have value of 1 or some other constant value (MJy/sr). I will use "prime" symbol to denote output (resampled) quantities. Based on my understanding of the documentation, total flux captured in the input image would be:
where N is the number of pixels in the input image and I is pixel intensity assumed to be constant across the image and also on the sky (thanks to flat-fielding) and P is the The flux in the resampled image would have a similar formula:
(I is the same as in input because it is a surface brightness independent on pixel area). Ideally flux should be preserved and so
Therefore we can write:
(just some tautology). Please observe how I (aptly I may say 😄 ) introduced the notation A more complicated math would not assume a constant intensity but would deal with the number of terms in summation ( Previously the code was computing something like this:
where R was the pixel ratio computed as the ratio of the output pixel scale (same for all pixels so
In addition,
Array A can be taken as Pixel Area Map of any instrument. NOTEThe following links: https://jwst-pipeline.readthedocs.io/en/latest/jwst/photom/reference_files.html, https://jwst-pipeline.readthedocs.io/en/latest/jwst/photom/main.html (see "Pixel Area Data" section) all say something like this:
This is incorrect. No matter what method I used to compute average pixel area, it was alway quite different from |
Another benefit of the new code and scaling of individual input image intensities is that it allows to correctly resample input images that have different distortion models (i.e., if the distortion model has changed between visits, or photometric calibration results in different values in |
283b843
to
b51c1f2
Compare
Many of the regression tests are erroring with these errors:
These arise from |
Restarted regression tests after this latest commit: |
Latest regtest run: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/951/ Lots of unrelated failures. PR branch needs a rebase to pick up other changes from master. |
1a87f0f
to
7d9df43
Compare
f8a451d
to
ef87047
Compare
9d37586
to
8ccbc96
Compare
@mcara What's the status of this? Are you done with your attempts to handle spectroscopic modes or still investigating? |
Yes, I am done. Spectroscopic data have never had PIXAR_* modified, so I chose to not handle spectroscopic data, at least for now. The current algorithm will not work as it is tricky to derive pixel scale (or pixel area from one axis) from a spectroscopic WCS (and there are many types with different specifics). |
…ed intensities according to actual pixel scale ratio.
69e4d01
to
bc43b26
Compare
update change log entry
I know this is already implemented and working and seems correct. I just want to make a point here that in the _cal files that have been flat fielded, the variation in pixel area across the detector that affects the amount of flux a pixel receives has already been corrected to be the same. Imagine that our QE of the detector is 100% for every pixel, and I take an image of a completely uniform white light source with exactly the same flux at all points. My detector will not register the same value at each pixel as some pixels have larger or smaller area than others due to distortions. So uniform QE and uniformly-illuminated astronomical scene still yields different per-pixel flux because of distortions that change the size of the pixels. But that is only in the raw data. _cal files have been flat fielded, and with our assumption that QE is 1 at all pixels, our counts from a flat lamp will also vary with pixel area, even though the sensitivity of pixels is exactly 1 everywhere, in exactly the same way as above. So when I divide by the normalized flat, then I will divide out the differences in pixel area. So once data has been flattened, any flux difference due to pixel area has been removed. And this is the input into the Are we all on the same page? It would be nice if the |
@jdavies-st I agree (as we have for years) that once images have been flat-fielded they have the correct surface brightness across all pixels, which I believe is the form that On a related note, when PR #8187 was merged recently, which shifted the |
|
And this correction that I described in the previous message is specific to JWST pipeline: how it defines |
It is not surprising at all that the result has changed. The whole purpose of this PR was to compute a scale factor ( Basically, iscale = np.sqrt(img.meta.photometry.pixelarea_steradians / input_pixel_area_computed_from_WCS) (ratio of reported "nominal" area to the area computed from WCS). |
Xref: #9246 |
Resolves JP-3035
Closes #7668
Closes #7390
This PR re-designed how resample computes photometric keywords
PIXAR_*
as well as corrects image data so that the flux of the resampled image (when image intensity is multiplied by PIXAR_SR) matches the flux of the input image. In my brief testing on NIRCAM images, flux error could have been up to 1.6% using previous computations and now it is down to about 0.005% (difference between the computed flux in the input and output images).This required undoing some of the changes made in #5389.
I expect a large number of regression (and some unit) tests to fail due to this PR. I will will deal with failing unit tests in next commits.
This PR also allows some additional info to be passed with custom user-provided
output_wcs
as well it modified how output resampled image size is computed from such a WCS.CC: @nden @hbushouse
Checklist for maintainers
CHANGES.rst
within the relevant release sectionHow to run regression tests on a PR
Regression tests:
https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/931
https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/934
https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/944
https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/945
https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/953
https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/974