Use skymatch from stcal

[WilliamJamieson]

spacetelescope/stcal#310 moves the shared skymatch code between jwst and romancal into stcal. This PR switches JWST over to using this code rather than having its own copy.

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

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 73.61%. Comparing base (320636f) to head (5eb2ed5). Report is 865 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8901      +/-   ##
==========================================
- Coverage   73.71%   73.61%   -0.11%     
==========================================
  Files         372      368       -4     
  Lines       37111    36296     -815     
==========================================
- Hits        27356    26718     -638     
+ Misses       9755     9578     -177     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

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

[nden]

@WilliamJamieson cube_skymatch will need to be adjusted because it reuses some of the code in skymatch, e.g. https://github.com/spacetelescope/jwst/blob/main/jwst/cube_skymatch/cube_skymatch_step.py#L25

[WilliamJamieson]

Regression test run: https://github.com/spacetelescope/RegressionTests/actions/runs/13274030261, are these regression test failures related to these changes?

[emolter]

are these regression test failures related to these changes?

Unfortunately I believe so. I'm not aware of any recent failures in those tests on main, and it would make sense because the failing tests call calwebb_image3

[mcara]

The code for region.py in gwcs has better accuracy than the code that was used in jwst. @nden reported two failures in regression tests when #8892 was merged: https://github.com/spacetelescope/jwst/pull/8892#issuecomment-2429469191 I wonder whether those were the same errors that we are seeing now. Unfortunately that regression test in #8892 has expired and we can no longer see it (unless we recreate exactly the same environment).

Looking at the current regression test, there are 7 different pixels that sit away from the edges of input dithers. For all other pixels, the difference between pixel values (current PR vs truth) is smaller than 6e-6. For these 7 pixels, the difference is > 1e-3 up to 0.014. This needs some investigation however, I do not believe this change indicates a problem with this PR. It is just nice to be able to explain it.

[mcara]

[mcara]

Upon further analysis, the differences between those 7 pixels in this PR compared to main is due to small differences in computed relative sky values. This results in different (by minuscule amounts) pixel values in sky-subtracted images used as input to drizzle. In the outlier detection step, two pixels in one of the images now have values slightly above the rejection threshold resulting in them being flagged as cosmic rays. This, in turn, significantly modifies input image's weight (for those two pixels) leading to a total of 7 pixels being different in the resampled image. This kind of differences are expected and here it is simply because those two pixels were already very close to being rejected. We do not see any failures in other tests that use skymatch indicating that these differences in flagging cosmic rays are extremely rare. pyregion.py from gwcs, as I mentioned above, has better implementation compared to the one in jwst, and IMO these two tests should be OKified.

[WilliamJamieson]

The code for region.py in gwcs has better accuracy than the code that was used in jwst. @nden reported two failures in regression tests when #8892 was merged: #8892 (comment) I wonder whether those were the same errors that we are seeing now. Unfortunately that regression test in #8892 has expired and we can no longer see it (unless we recreate exactly the same environment).

This is exactly the case. The _det function in gwcs.region exactly matches the value returned by np.cross for 2D vectors. Which is the case we are discussing. Thus the regression test failures seen for these changes in Skymatch are simply the pre #8892 regression test values. We never fully explained why the #8892 changes happened, so these changes are probably the right way to go.

EDIT: I ran the failing tests locally with #8892 reverted and compared the output with what this PR produced and confirm that they are the same.

[WilliamJamieson]

Thank you all for the reviews and approvals. Once spacetelescope/stcal#310 is merged, I'll re-point the stcal version back to the main stcal branch which will make this PR ready to be merged.