-
Notifications
You must be signed in to change notification settings - Fork 174
JP-2819: Populate wavelength extension for MIRI LRS slitless #6964
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-2819: Populate wavelength extension for MIRI LRS slitless #6964
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6964 +/- ##
==========================================
+ Coverage 79.57% 79.59% +0.01%
==========================================
Files 411 411
Lines 37217 37242 +25
==========================================
+ Hits 29616 29641 +25
Misses 7601 7601
*This pull request uses carry forward flags. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
cfbdba2
to
5af4ceb
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.
Looks OK to me, but @nden should have the definitive word.
Rats, should've asked if you had run regtests on this before I merged it. I'm doing a run now and a handful of errors have already shown up in the log: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST/1938/console |
Ah, sorry about that - slipped my mind. We should expect failures due to cal files having a populated extension, but I guess we'll have to see if there are any surprises. |
Yes, these are not failures/differences (which I would expect), these are ERRORs. |
I'm running test_miri_lrs_slitless locally right now to see if that's the one that's going bad. |
Yep, the LRS slitless test is now crashing in the photom step. I wonder if something in the photom step was also creating the 2D wavelength array and it doesn't need to do that anymore, since it should already be available in the input. Oh, I wonder if the wavelength array needs to be 3D to match the shape of all the other data arrays when dealing with LRS slitless datasets coming from a rateints file. |
The change from the initial commit to the use of the bbox changed the wavelength array shape from the equivalent shape of the data to the shape of the bbox, which is smaller than the data array. This is causing a mismatch when applying an operation to the wavelength array. |
@nden Looking further into the relative indices, it appears that the use of bbox serves to truncate the first method to only include the non-nan extent - i.e. the bbox is working as expected, but it is a mismatch to the data array shape. Was the intent of the bbox use (i.e. the full frame coordinates mentioned above) meant to include the location on the full frame instead of the subarray? I tried to offset the indices generated in the first method by the xstart and ystart values defined in meta.subarray, but there is no defined wcs for those pixel positions. |
8000
FWIW, bbox_grid appears to be in the same pixel-space as the original index_array (which had values x:{0:71}, y:{0:471}), but due to the truncation of the original space to exclude fully-nan regions, the extent of the bbox grid is x:{4:71}, y:{5:394}. |
Resolves JP-2819
Closes #6963
This PR adds a function to assign_wcs to populate the wavelength extension of the calints level 2 output for MIRI LRS slitless data.
Not sure if this is addressing pixel locations appropriately (0-indexed) or if this addition is warranted - would like review on this approach.
Checklist
CHANGES.rst
within the relevant release section