Fix issues in outlier det. due to GWCS.inverse enforcing bbox

[mcara]

Fixes failures in JWST pipeline due to GWCS inverse transformation enforcing bounding box.

Tasks

• update or add relevant tests
• update relevant docstrings and / or docs/ page
• Does this PR change any API used downstream? (if not, label with no-changelog-entry-needed)
  • write news fragment(s) in changes/: echo "changed something" > changes/<PR#>.<changetype>.rst (see below for change types)
  • run regression tests with this branch installed ("git+https://github.com/<fork>/stcal@<branch>")
    • jwst regression test
    • romancal regression test

news fragment change types...

• changes/<PR#>.apichange.rst: change to public API
• changes/<PR#>.bugfix.rst: fixes an issue
• changes/<PR#>.general.rst: infrastructure or miscellaneous change

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.26%. Comparing base (1ad3a7c) to head (9f2c3fb).
⚠️ Report is 58 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #324      +/-   ##
==========================================
- Coverage   89.26%   89.26%   -0.01%     
==========================================
  Files          64       64              
  Lines       10095    10103       +8     
==========================================
+ Hits         9011     9018       +7     
- Misses       1084     1085       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[emolter]

@mcara @nden Did this failure show up anywhere besides the outlier detection unit tests? e.g., in the regression tests or with any real data? The outlier detection tests were not calling AssignWcsStep properly and fixing that seemed to resolve the problem without any code changes outside the test itself: https://github.com/nden/jwst/pull/1/files

Is there a good reason to explicitly ignore the bounding box when calling reproject()? It seems to me that if the bounding box is reasonable, then the proposed change should not be necessary. The bounding box was not reasonable for the outlier tests, and calling AssignWcs made it reasonable and made the tests pass. I wonder if something similar could be happening for the resample unit test?

[mcara]

Did this failure show up anywhere besides the outlier detection unit tests? e.g., in the regression tests or with any real data?

I did not run regression tests (or real data) since unit tests were failing.

Is there a good reason to explicitly ignore the bounding box when calling reproject()? It seems to me that if the bounding box is reasonable, then the proposed change should not be necessary. The bounding box was not reasonable for the outlier tests, and calling AssignWcs made it reasonable and made the tests pass. I wonder if something similar could be happening for the resample unit test?

I am not sure about the root-cause, but it appears (to me) that the issue is only with some instruments. In any case, drizzle can filter out pixels that are outside of the output frame by itself (without much overhead) and it can also handle NaN in pixmap. However, it cannot handle the case when ALL values in pixmap are NaN which is what the failures were in many unit tests.

[emolter]

I am not sure about the root-cause, but it appears (to me) that the issue is only with some instruments. In any case, drizzle can filter out pixels that are outside of the output frame by itself (without much overhead) and it can also handle NaN in pixmap. However, it cannot handle the case when ALL values in pixmap are NaN which is what the failures were in many unit tests.

Why does it appear to you that the failures are instrument-specific?

The all-NaN pixmaps were an effect of a bad input WCS, which led reproject() to return NaN no matter what was input. As I said, calling AssignWcsStep prior to running the test (instead of assign_wcs.pointing.create_fitswcs) solves the problem for the outlier unit tests. I don't think an all-NaN pixmap should happen unless the input WCS is not correct for the input data. It seems like the proposed fix just hides that pre-existing problem.

[mcara]

Why does it appear to you that the failures are instrument-specific?

Maybe I misspoke. I just saw some of the regression tests passing and others not. Quite possibly my conclusion was wrong.

However, I do not see any benefit in using bounding_box for the inverse transformation for pixmap calculation specifically if the intension is to use it for resample/drizzle:

1. drizzle computes intersections between input and output images and tries to sample input images only where input pixels would map onto output frame.
2. drizzle checks that output coordinate does not land outside the output array;
3. enabling bounding box on the inverse transform will only slowdown computations.

[mcara]

But I do agree it may be hiding something.

[nden]

I want to do more testing before we merge this.