AL-837: Compute resample output WCS from known s_region instead of recalculating footprints

[emolter]

Resolves AL-837

This PR simplifies the calculation of the output WCS for resample. Previously, the wcs_from_footprints function accepted a list of WCS objects, one for each input model, and computed the footprints from those WCS objects to pass into _calculate_new_wcs(). However, computing the footprints here is unnecessary because that information is already stored in the S_REGION keyword. This PR makes wcs_from_footprints take in an actual footprint instead, in the form of either a numpy array or an S_REGION string.

Most of the changes are in the stcal repository; see stcal PR here.

Tasks

• request a review from someone specific, to avoid making the maintainers review every PR
• add a build milestone, i.e. Build 11.3 (use the latest build if not sure)
• Does this PR change user-facing code / API? (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)
  • update or add relevant tests
  • update relevant docstrings and / or docs/ page
  • start a regression test and include a link to the running job (click here for instructions)
    • Do truth files need to be updated ("okified")?
      • after the reviewer has approved these changes, run okify_regtests to update the truth files
• if a JIRA ticket exists, make sure it is resolved properly

news fragment change types...

• changes/<PR#>.general.rst: infrastructure or miscellaneous change
• changes/<PR#>.docs.rst
• changes/<PR#>.stpipe.rst
• changes/<PR#>.datamodels.rst
• changes/<PR#>.scripts.rst
• changes/<PR#>.fits_generator.rst
• changes/<PR#>.set_telescope_pointing.rst
• changes/<PR#>.pipeline.rst

stage 1

• changes/<PR#>.group_scale.rst
• changes/<PR#>.dq_init.rst
• changes/<PR#>.emicorr.rst
• changes/<PR#>.saturation.rst
• changes/<PR#>.ipc.rst
• changes/<PR#>.firstframe.rst
• changes/<PR#>.lastframe.rst
• changes/<PR#>.reset.rst
• changes/<PR#>.superbias.rst
• changes/<PR#>.refpix.rst
• changes/<PR#>.linearity.rst
• changes/<PR#>.rscd.rst
• changes/<PR#>.persistence.rst
• changes/<PR#>.dark_current.rst
• changes/<PR#>.charge_migration.rst
• changes/<PR#>.jump.rst
• changes/<PR#>.clean_flicker_noise.rst
• changes/<PR#>.ramp_fitting.rst
• changes/<PR#>.gain_scale.rst

stage 2

• changes/<PR#>.assign_wcs.rst
• changes/<PR#>.badpix_selfcal.rst
• changes/<PR#>.msaflagopen.rst
• changes/<PR#>.nsclean.rst
• changes/<PR#>.imprint.rst
• changes/<PR#>.background.rst
• changes/<PR#>.extract_2d.rst
• changes/<PR#>.master_background.rst
• changes/<PR#>.wavecorr.rst
• changes/<PR#>.srctype.rst
• changes/<PR#>.straylight.rst
• changes/<PR#>.wfss_contam.rst
• changes/<PR#>.flatfield.rst
• changes/<PR#>.fringe.rst
• changes/<PR#>.pathloss.rst
• changes/<PR#>.barshadow.rst
• changes/<PR#>.photom.rst
• changes/<PR#>.pixel_replace.rst
• changes/<PR#>.resample_spec.rst
• changes/<PR#>.residual_fringe.rst
• changes/<PR#>.cube_build.rst
• changes/<PR#>.extract_1d.rst
• changes/<PR#>.resample.rst

stage 3

• changes/<PR#>.assign_mtwcs.rst
• changes/<PR#>.mrs_imatch.rst
• changes/<PR#>.tweakreg.rst
• changes/<PR#>.skymatch.rst
• changes/<PR#>.exp_to_source.rst
• changes/<PR#>.outlier_detection.rst
• changes/<PR#>.tso_photometry.rst
• changes/<PR#>.stack_refs.rst
• changes/<PR#>.align_refs.rst
• changes/<PR#>.klip.rst
• changes/<PR#>.spectral_leak.rst
• changes/<PR#>.source_catalog.rst
• changes/<PR#>.combine_1d.rst
• changes/<PR#>.ami.rst

other

• changes/<PR#>.wfs_combine.rst
• changes/<PR#>.white_light.rst
• changes/<PR#>.cube_skymatch.rst
• changes/<PR#>.engdb_tools.rst
• changes/<PR#>.guider_cds.rst

[emolter]

regression tests started here; unit tests are expected to fail because of dependency on stcal PR branch

[codecov]

Codecov Report

Attention: Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 63.68%. Comparing base (1f87cb2) to head (094c0ff).
Report is 12 commits behind head on main.

Files with missing lines	Patch %	Lines
jwst/assign_mtwcs/moving_target_wcs.py	75.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #8893   +/-   ##
=======================================
  Coverage   63.68%   63.68%           
=======================================
  Files         375      375           
  Lines       38689    38692    +3     
=======================================
+ Hits        24638    24640    +2     
- Misses      14051    14052    +1     

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

[emolter]

New round of regression tests. Previous failures were caused by the use of center=True in the call to compute_s_region_imaging causing the S_REGION keyword to contain the vertices of the corner pixel centers instead of the vertices of the corners themselves. The most recent commit modifies update_s_region_imaging to always call compute_s_region_imaging with center=False. This is likely to lead to many failures, but should also cause the S_REGION to correspond to the "true" footprint of the image on the sky.

[emolter]

This needs new regtests now that #8896 has been merged: https://github.com/spacetelescope/RegressionTests/actions/runs/11406757782

[emolter]

new round of regtests now this is merged on the stcal side

[melanieclarke]

Code looks fine to me, with one tiny style nitpick. Can you please review the docstring for make_output_wcs, though? The ref_wcs parameter is missing, and the text needs to be updated to make it clear that s_region is now required, not a gwcs.WCS object.

Regression tests all look like minor changes except for the moving target. For that one, the size of the resampled image is significantly different -- that mode will need different handling.

[emolter]

Regression tests all look like minor changes except for the moving target.

I really should have caught this sooner 😞 Thanks for pointing it out! I think updating s_region at the end of assign_mtwcs is the proper way of handling this. I added that in the most recent commit, and it does indeed resolve the problem with the nircam regtest (see here). It does change the s_regions in the HDRTAB but I think that's a good thing: they now better reflect the s_regions of the WCS footprints that went into making the combined product.

Re-running the full regtest suite here to see if this change affects any of our other moving target products.

I also updated the docstring - let me know what you think.

[melanieclarke]

MT test looks much better! I'm glad it was a relatively easy fix.

Thanks for updating the docstring - just a couple additional small suggestions.

[emolter]

Attempting to update the s_region for everything in assign_mtwcs had lots of unintended consequences. This should get moved elsewhere, perhaps into calwebb_image3. I will play with that.

Re-running all jobs, limiting update to S_REGION to only imaging modes: https://github.com/spacetelescope/RegressionTests/actions/runs/11615207638

edit: this looks better but fgs_image3 regtests are failing. This might be due to out-dated S_REGIONs in the input- checking on that

[emolter]

yet another regtest round started here. For some reason re-running jobs is causing lots of failures...

[emolter]

@melanieclarke have a look at the most recent regtest run and let me know if all the failures look in line with what you'd expect now https://github.com/spacetelescope/RegressionTests/actions/runs/11633944467

[melanieclarke]

LGTM! Regtest changes now all look reasonable.

[emolter]

Didn't know this, but apparently the ok-ify script does not play nicely with "re-run jobs". Starting a regression test run on main, will okify once that is done https://github.com/spacetelescope/RegressionTests/actions/runs/11671137200