spacetelescope / stcal

https://stcal.readthedocs.io/en/latest/
Other
10 stars 32 forks source link

JP-3242: Proper Handling of Timing for Final Slope Computation for only 0th Good Group Ramps #173

Closed kmacdonald-stsci closed 1 year ago

kmacdonald-stsci commented 1 year ago

Resolves JP-3242

This PR addresses special handling of timing for ramps that have only good 0th group in a ramp. The addition of this special case only properly computed the variances. The timing division for the final slope computation still used the group time, which is not correct for certain special cases. All timing divisions have been pushed back to the segment calculations, so there is no longer one final divide. There is also the further subset special case for only 0th good group with the use of ZEROFRAME.

Checklist

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 98.61% and project coverage change: +0.13 :tada:

Comparison is base (cc1edfd) 75.28% compared to head (2f2b9cf) 75.42%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #173 +/- ## ========================================== + Coverage 75.28% 75.42% +0.13% ========================================== Files 29 29 Lines 5706 5714 +8 ========================================== + Hits 4296 4310 +14 + Misses 1410 1404 -6 ``` | [Impacted Files](https://app.codecov.io/gh/spacetelescope/stcal/pull/173?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) | Coverage Δ | | |---|---|---| | [tests/test\_ramp\_fitting.py](https://app.codecov.io/gh/spacetelescope/stcal/pull/173?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope#diff-dGVzdHMvdGVzdF9yYW1wX2ZpdHRpbmcucHk=) | `89.34% <97.29%> (+0.66%)` | :arrow_up: | | [src/stcal/ramp\_fitting/ols\_fit.py](https://app.codecov.io/gh/spacetelescope/stcal/pull/173?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope#diff-c3JjL3N0Y2FsL3JhbXBfZml0dGluZy9vbHNfZml0LnB5) | `80.13% <100.00%> (+0.23%)` | :arrow_up: | | [src/stcal/ramp\_fitting/ramp\_fit\_class.py](https://app.codecov.io/gh/spacetelescope/stcal/pull/173?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope#diff-c3JjL3N0Y2FsL3JhbXBfZml0dGluZy9yYW1wX2ZpdF9jbGFzcy5weQ==) | `55.91% <100.00%> (+0.47%)` | :arrow_up: | | [src/stcal/ramp\_fitting/utils.py](https://app.codecov.io/gh/spacetelescope/stcal/pull/173?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope#diff-c3JjL3N0Y2FsL3JhbXBfZml0dGluZy91dGlscy5weQ==) | `90.61% <100.00%> (+0.04%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

hbushouse commented 1 year ago

Still needs a change log entry and converted out of Draft form.

kmacdonald-stsci commented 1 year ago

Still needs a change log entry and converted out of Draft form.

The tests have been updated. The change log has been updated. And the PR is out of draft form.

hbushouse commented 1 year ago

Need another stcal maintainer to approve. Also probably needs a regtest against Roman. @PaulHuwe @mairanteodoro bueller? Who can do this for us?

kmacdonald-stsci commented 1 year ago

Need another stcal maintainer to approve. Also probably needs a regtest against Roman. @PaulHuwe @mairanteodoro bueller? Who can do this for us?

Paul is no longer to be tagged on STCAL PR's. I tagged Jon Eisenhamer and Dave Davis.

ddavis-stsci commented 1 year ago

Hi Ken,  I'm the one you should tag for these requests.             Dave

On 6/21/23 3:38 PM, Ken MacDonald wrote:

Need another stcal maintainer to approve. Also probably needs a
regtest against Roman. @PaulHuwe
<https://urldefense.com/v3/__https://github.com/PaulHuwe__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!woWVmrGNQtl1__N8QE-sLMFEEV0wjXAPTB7I3c6oqfnzlzyDvohlW_Ru-7dWzT72citKQrPmawdqncoMWkDPysca$>
@mairanteodoro
<https://urldefense.com/v3/__https://github.com/mairanteodoro__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!woWVmrGNQtl1__N8QE-sLMFEEV0wjXAPTB7I3c6oqfnzlzyDvohlW_Ru-7dWzT72citKQrPmawdqncoMWtX1KrX6$>
bueller? Who can do this for us?

Paul is no longer to be tagged on STCAL PR's. I tagged Jon Eisenhamer and Dave Davis.

— Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https://github.com/spacetelescope/stcal/pull/173*issuecomment-1601565781__;Iw!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!woWVmrGNQtl1__N8QE-sLMFEEV0wjXAPTB7I3c6oqfnzlzyDvohlW_Ru-7dWzT72citKQrPmawdqncoMWnBuJAeI$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ALXCXWIZLJDGFG6CQWBAPALXMNER7ANCNFSM6AAAAAAZG2CESQ__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!woWVmrGNQtl1__N8QE-sLMFEEV0wjXAPTB7I3c6oqfnzlzyDvohlW_Ru-7dWzT72citKQrPmawdqncoMWri4f7CC$. You are receiving this because your review was requested.Message ID: @.***>

ddavis-stsci commented 1 year ago

It looks like we'll have to update some of our truth files in artifactory before these will pass,

>           return func(*args, **kwds)
E           AssertionError: 
E           Not equal to tolerance rtol=1e-06, atol=0
E           
E           Mismatched elements: 1 / 1 (100%)
E           Max absolute difference: 10.00001
E           Max relative difference: 1.000001
E            x: array(-1.000001e-05, dtype=float32)
E            y: array(10.)

I'll look at this in more detail unless you have some input. You can see the test results at https://plwishmaster.stsci.edu:8081/job/RT/job/Roman-devdeps/322/