-
Notifications
You must be signed in to change notification settings - Fork 169
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-3121: Updating JWST Ramp Fit Code to Work with New C Extension #8355
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8355 +/- ##
==========================================
+ Coverage 57.93% 57.97% +0.04%
==========================================
Files 387 387
Lines 38839 38831 -8
==========================================
+ Hits 22502 22514 +12
+ Misses 16337 16317 -20 ☔ View full report in Codecov by Sentry. |
efb0756
to
1994522
Compare
1994522
to
5be8b01
Compare
5be8b01
to
0e382d0
Compare
067c307
to
9a90640
Compare
Started regtest run at https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1461 |
@kmacdonald-stsci Regtest results at https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1461/testReport/ show differences in just one NIRISS test, in the rate and rateints files. The VAR_RNOISE array shows a smallish number of pixels with different values. Can you confirm whether this is expected? This was run using the latest stcal/main, which includes the new C code for ramp_fit, but with it deactivated (so it should've run the existing python version). |
author Ken MacDonald <[email protected]> 1697200039 -0400 committer Ken MacDonald <[email protected]> 1702991072 -0500 Updating testing for C implementation of ramp fitting. Updating test for C code. Updating first group orphan testing. Updating tests. Not sure if the four CR test is a valid test. Removing test cases in ramp fitting. Updating the CHARGELOSS read noise computation to only update pixels and integrations affected by CHARGELOSS. Updating the change log for PR 8355.
7b99924
to
81f6271
Compare
Using the |
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.
Code updates and regtest results look good.
Resolves JP-3121
This PR addresses the C extensions. In particular, tests need to ensure the
dtypes
ofndarrays
are what is to be expected in a RampModel. Tests have been changed to ensure proper typing, as well as simplified.Checklist for maintainers
CHANGES.rst
within the relevant release sectionHow to run regression tests on a PR