-
Notifications
You must be signed in to change notification settings - Fork 177
JP-4095: Fix velocity correction in WCS inverse #9783
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
base: main
Are you sure you want to change the base?
Conversation
wl2 = wl3 | velocity_corr | ||
wl3 = wl3 | velocity_corr |
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.
@tapastro - I came across this while looking at the velocity corrections for NIRSpec WCS. It looks like a typo to me, so I fixed it here but I'm not sure what impacts it will have. Do you want me to move it to another PR for discussion with NIRISS or keep it here?
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9783 +/- ##
=======================================
Coverage 82.49% 82.50%
=======================================
Files 366 366
Lines 37134 37136 +2
=======================================
+ Hits 30633 30638 +5
+ Misses 6501 6498 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Regression tests: It looks like these changes have a minor impact on NIRSpec MOS and IFU products for spec2. The edges of the valid IFU REGIONS change a little bit, I assume because the wavelength inversion has changed a little. For MOS and IFU, MSA flagging is impacted, which modified the DQ plane. The only SCI data affected is when nsclean is run, which I think makes sense, since the masked science regions are changing slightly. NIRISS products do not seem to be impacted by the typo fix. |
Resolves JP-4095
Fix a missing velocity correction in the inverse transform for NIRSpec IFU. This seems to fix a round trip error of up to ~0.5 pixel. Example case: jw01251004001_03107_00001_nrs2_rate.fits, with velosys = 20 km/s.
I checked the other modes for similar errors, and found that NIRSpec MOS and FS needs the same handling. I'm not sure if we've noted similar round trip errors for these modes, but checking MOS regression test data jw01345066001_05101_00001_nrs1_rate.fits with velosys -15 km/s, it looks like it improves round trip error from ~.08 pixel to ~.001 pixel in a quick spot check on slit 35.
For the other instruments, where the velocity_correction is used, they appear to be propagating it to the inverse correctly. I did note what looks like a typo in the NIRISS SOSS handling for order 2 and 3 so I fixed it here, but I can move to another PR if it needs separate review.
Tasks
Build 12.0
(use the latest build if not sure)no-changelog-entry-needed
)changes/
:echo "changed something" > changes/<PR#>.<changetype>.rst
(see changelog readme for instructions)docs/
pageokify_regtests
to update the truth files