-
Notifications
You must be signed in to change notification settings - Fork 28
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
Implement outlier detection step. #981
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #981 +/- ##
==========================================
- Coverage 73.32% 71.08% -2.25%
==========================================
Files 103 104 +1
Lines 6733 7006 +273
==========================================
+ Hits 4937 4980 +43
- Misses 1796 2026 +230
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Some minor comments. The most important one: help me with the provenance of the outlierpars reference file, and what the value of pars['resample_data'] is for our case?
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.
These changes look good. The only outstanding comments are about the outlier_pars file and the value of the resample_data argument. I don't really know how that works or what the defaults are; can you explain what we're doing there? Thanks!
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 did a superficial review and left inline comments. I still need to run the code and look at the output.
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.
The step completes without crashing. I am not sure the results are entirely correct but I think we can merge this as is and debug when we have good data.
The test I ran uses as input 3 identical images (copies of the same file). It found quite a lot of outliers which I think is unexpected.
One thing that needs to change now is adding the OUTLIER
DQ flag to this page:
https://github.com/spacetelescope/romancal/blob/main/docs/roman/references_general/dq_flags.inc
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 added the missing flag to the docs, merging.
These are changes made by spacetelescope/romancal#981
These are changes made by spacetelescope/romancal#981
These are changes made by spacetelescope/romancal#981
These are changes made by spacetelescope/romancal#981
These are changes made by spacetelescope/romancal#981
Resolves RCAL-501
This PR addresses the implementation of the outlier detection step for Roman (replaces PR#979).
The link to the regression test results is here.
Checklist
CHANGES.rst
under the corresponding subsection