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

[kmacdonald-stsci]

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

• added entry in CHANGES.rst (either in Bug Fixes or Changes to API)
• updated relevant tests
• updated relevant documentation
• updated relevant milestone(s)
• added relevant label(s)

[codecov]

Codecov Report

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

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

Additional details and impacted files

@@            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	Coverage Δ
tests/test_ramp_fitting.py	89.34% <97.29%> (+0.66%)	⬆️
src/stcal/ramp_fitting/ols_fit.py	80.13% <100.00%> (+0.23%)	⬆️
src/stcal/ramp_fitting/ramp_fit_class.py	55.91% <100.00%> (+0.47%)	⬆️
src/stcal/ramp_fitting/utils.py	90.61% <100.00%> (+0.04%)	⬆️

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

[hbushouse]

Needs a change log entry.

[hbushouse]

I think this all looks reasonable to me. The real test will be in ... the tests, to see if we get the right answers in real images. If any unit tests use combinations of params (e.g. nframes, frame-time, group-time, etc.) that are nonsensical, I would update them to make them sensical.

[hbushouse]

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

[kmacdonald-stsci]

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]

Looks good.

[hbushouse]

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

[kmacdonald-stsci]

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]

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]

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/

[ddavis-stsci]

LGTM, The changes I see are at the 10E-5 level and are just above our 10E-6 level for tolerance so even with the tests formally failing I don't see an issue with the code.