-
Notifications
You must be signed in to change notification settings - Fork 180
JP-3887: Add support for time dependent photometry corrections #9736
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
Conversation
4dde9e9 to
6afb830
Compare
|
Regression tests here with spacetelescope/stdatamodels#555: Tests show one very small change to the background level value for one input file, recorded in the HDRTAB for the i2d output. This appears to be precision related, from minor changes to the way the time dependence is calculated for MIRI imaging photom corrections: the background level fit in skymatch is sensitive to very small changes in the data. There is otherwise no impact to the output data. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9736 +/- ##
==========================================
+ Coverage 82.48% 82.73% +0.24%
==========================================
Files 366 365 -1
Lines 37302 37324 +22
==========================================
+ Hits 30770 30881 +111
+ Misses 6532 6443 -89 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Rerunning regression tests after fixing the sense of the correction, updating the MIRI MRS functional form, and removing the MIRI imaging transition code: Now expecting diffs in both MIRI imaging and MRS modes for spec2, since the old-style timecoeff extensions are not supported, so time corrections will not be applied. |
9e397b1 to
bd4c8b7
Compare
|
@tapastro - the MIRI teams are happy with the changes here and would like to deliver reference files on Friday, coordinated with merging this PR. Do you think we can have this reviewed and ready by then? |
bd4c8b7 to
fccd5b5
Compare
tapastro
left a comment
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.
One small nitpick, otherwise LGTM
Co-authored-by: David Law <[email protected]>
d6cd3aa to
c506bfc
Compare
|
Regtests for okifying after ref file delivery: |
Resolves JP-3887
Requires spacetelescope/stdatamodels#555
Add support for time dependent photometry corrections for all modes.
MIRI imaging corrections will migrate from an exponential-only functional form, assuming an additive correction, to allowing any composition of linear, exponential, and power law forms, assuming a multiplicative correction. Current reference files are not supported: existing TIMECOEFF extensions will be ignored.
MIRI MRS corrections have a different format than the other instruments/modes. The current handling is moved from a
mir_mrsmodule to a newtime_dependencemodule. It will also now use a power law function instead of an exponential.For all other instruments and modes, there are no existing time dependence corrections, but the code has been updated to expect them. As with MIRI imaging, any composition of linear, exponential, and power law forms is allowed, by attaching TIMECOEFF_LINEAR, TIMECOEFF_EXPONENTIAL, and TIMECOEFF_POWERLAW extensions to the PHOTOM reference file as needed. If none of these extensions are present, no time correction is applied.
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_regteststo update the truth files