Issue-168: Updating Usage of 'np.where' Function

[kmacdonald-stsci]

Closes #168

This PR addresses the usage of np.where and removes it where possible for performance reasons in the ramp fitting step.

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: 100.00% and project coverage change: +0.31 🎉

Comparison is base (a3c1e27) 74.78% compared to head (557ef77) 75.10%.

❗ Current head 557ef77 differs from pull request most recent head 66b10ff. Consider uploading reports for the commit 66b10ff to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #169      +/-   ##
==========================================
+ Coverage   74.78%   75.10%   +0.31%     
==========================================
  Files          29       29              
  Lines        5700     5664      -36     
==========================================
- Hits         4263     4254       -9     
+ Misses       1437     1410      -27     

Impacted Files	Coverage Δ
src/stcal/ramp_fitting/ols_fit.py	79.83% <100.00%> (+2.39%)	⬆️
src/stcal/ramp_fitting/utils.py	90.57% <100.00%> (-0.15%)	⬇️
tests/test_ramp_fitting.py	88.09% <100.00%> (-0.24%)	⬇️

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

[braingram]

Excellent!

I took a brief look at other uses last week but never finished updating the branch to a point where I could open a PR:
https://github.com/braingram/stcal/tree/no_where

I see some uses in this PR that I missed in my branch and also some other uses in my branch which are not in this PR.

The changes included in this PR appear to address the bulk of the uses of np.where. Thanks!

Was the test added to increase coverage? I'm not familiar enough with ramp fitting to review the test.

Also, a bit off-topic but while looking at the contribution of these lines of code to the runtime of ramp fitting. I noticed that roughly 40% of a test call to ols_ramp_fit_single was spent on these two lines of code:
https://github.com/spacetelescope/stcal/blob/main/src/stcal/ramp_fitting/ols_fit.py#L1657-L1658
which appears to be iterating through all pixels. I opened a separate issue #170 for that to discuss if this can be converted to a vector operation or in another way optimized. I wanted to bring it up here in case there is an easy fix.

[kmacdonald-stsci]

https://github.com/spacetelescope/stcal/blob/main/src/stcal/ramp_fitting/ols_fit.py#L1657-L1658

The added test was to increase code coverage and add a case that needs testing.

[hbushouse]

Code updates look good to me. Needs a regression test run and resolution of conflicts.

Has anyone done a test a against Roman?

[hbushouse]

@PaulHuwe can you (or your designee) run a Roman regtest against this PR branch?

[braingram]

@PaulHuwe can you (or your designee) run a Roman regtest against this PR branch?

I think I set this up correctly to run the Roman devdeps regtest with the source branch for this PR:
https://plwishmaster.stsci.edu:8081/job/RT/job/Roman-devdeps/295/

[hbushouse]

@PaulHuwe can you (or your designee) run a Roman regtest against this PR branch?

I think I set this up correctly to run the Roman devdeps regtest with the source branch for this PR: https://plwishmaster.stsci.edu:8081/job/RT/job/Roman-devdeps/295/

There were a number of failures, with only 1 in the ramp_fitting step itself. It appears to be an unrelated error that is common to all the failures. Should we call this a success?

[braingram]

There were a number of failures, with only 1 in the ramp_fitting step itself. It appears to be an unrelated error that is common to all the failures. Should we call this a success?

I would call this a success but @PaulHuwe might know more about the existing failures which all appear to be unit related.

The previous devdeps run (started by timer with no overrides) had 19 failures:
https://plwishmaster.stsci.edu:8081/job/RT/job/Roman-devdeps/294/

The run that used the source branch for this PR had 15 failures. All of these appear to match failures from the previous run:
https://plwishmaster.stsci.edu:8081/blue/organizations/jenkins/RT%2FRoman-devdeps/detail/Roman-devdeps/295/tests/

[PaulHuwe]

Not that is matters, as this was already merged, but the RomanCAL test repo files are presently being updated, as the latest release required remaking of files.