JP-2819: Bugfix - ensure wavelength array is of equivalent shape to data array #7005
Conversation
|
Starting the Jenkins job now: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/400/ |
|
Yes, that kind of change log entry is fine. |
Codecov ReportBase: 79.68% // Head: 79.70% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #7005 +/- ##
==========================================
+ Coverage 79.68% 79.70% +0.02%
==========================================
Files 411 411
Lines 37231 37225 -6
==========================================
+ Hits 29667 29670 +3
+ Misses 7564 7555 -9
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
In addition to some unrelated differences in NIRCam results (due to updated ref files), the regtest run shows the expected additional wavelength extension in some of the slitless products, but the calints also shows slight differences in the SCI and ERR arrays. Could this be due to slightly different wavelength values being computed than before, which is then leading to slightly different flux cal values during application of photom? If so, how do we know which wavelength values are correct? |
|
Tracing through photom to see how wavelengths were computed previously for LRS slitless, I'm assuming it ends up calling I'm wondering how that matches (or doesn't) what's being done in this PR (conceptually similar, but details differ). Also wondering if the |
|
IIRC, originally the |
|
For the case of LRS slitless I don't see a problem with populating the wavelength array. The wavelengths don't get corrected or modified by anything else in the pipeline and should give the same results as before when going through the extract_1d step. This is because the extract_1d step first checks to see if the wavelength array is populated and, if so, use it rather than computing the wavelengths on the fly. And the way it computed the wavelengths on the fly is identical to what's being done in this PR. It also seems foolish to have the wavelength array in the data products if it's not populated. Of course that could be solved by just making sure that the wavelength attribute in the datamodel is set to None, so that it doesn't get created in the LRS slitless products. |
|
@tapastro were you going to modify the PR code yet to just call the |
|
Yes, sorry - I was running regtests against this commit to see if any functional difference existed. I'll clean it up and ping a review request. |
2fbd7cd to
05dca55
Compare
|
Realized I hadn't run regtests against the code update - started one here: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/408/ |
|
The regtest results show the expected differences due to the presence of the wavelength extension in products that didn't have it before, but it's also showing 100% differences in the wavelength extension values for the calints product: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/408/testReport/junit/jwst.regtest/test_miri_lrs_slitless/_stable_deps__test_miri_lrs_slitless_tso_spec2_calints_/ Have you looked at the wavelength values in the new results to make sure they're OK? |
The new wavelengths look correct - they are nan where they aren't defined, and real-valued where they are. The current truth value is an array of zeroes. |
|
Interesting that the current calints truth file has all the wavelengths zero. They should've been calculated and populated during the photom step and hence included in the calints results. Weird. |
|
Yeah, this was the main issue raised in the initial issue #6963 - it's quite possible I solved this in a more roundabout way than needed, as I didn't realize this info should be calculated in photom. I assumed such information would populated once available, in assign_wcs. |
|
You're right. In the existing flow the 2D array of wavelengths is computed during the photom step, but only used locally to then compute the flux conversion factors for each pixel (because they're wavelength-dependent). That wavelength array was never stored in the |
Resolves JP-2819
Closes #
This PR fixes a bug in the previous PR #6964, which truncated the wavelength array extent for nan regions, causing array shape mismatch issues between the wavelength and data. This PR uses the data array shape to create the wavelength array.
Checklist for maintainers
CHANGES.rstwithin the relevant release sectionHow to run regression tests on a PR