spacetelescope / romancal

Python library to process science observations from the Nancy Grace Roman Space Telescope
https://roman-pipeline.readthedocs.io/en/latest/
Other
28 stars 28 forks source link

RCAL-856: WCS should roundtrip with sufficient accuracy #1273

Open nden opened 3 weeks ago

nden commented 3 weeks ago

Resolves RCAL-856

Closes #1272

This PR is the last one in a chain of PRs aiming to fix roundtripping of WCS transforms. The only change here is to test data, ensuring the WCS is consistent and the inverse is wihtin the bounding box.

The order the PRs should be merged is

Checklist

codecov[bot] commented 3 weeks ago

Codecov Report

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

Project coverage is 78.89%. Comparing base (6b1c696) to head (77d1f40).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1273 +/- ## ========================================== - Coverage 78.99% 78.89% -0.10% ========================================== Files 117 117 Lines 8101 8109 +8 ========================================== - Hits 6399 6398 -1 - Misses 1702 1711 +9 ``` | [Flag](https://app.codecov.io/gh/spacetelescope/romancal/pull/1273/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) | Coverage Δ | | *Carryforward flag | |---|---|---|---| | [nightly](https://app.codecov.io/gh/spacetelescope/romancal/pull/1273/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) | `62.79% <ø> (ø)` | | Carriedforward from [c4156a2](https://app.codecov.io/gh/spacetelescope/romancal/commit/c4156a210be56b735e86cd4c2dfbf05c3b708d93?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) | *This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) to find out more.

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

schlafly commented 2 weeks ago

I think I missed the part of this test that checks that things round trip?

This WCS contains only the v2v3tosky bit, and not the CRDS distortion bit. That's good since it makes for an easier unit test but bad since it won't catch the only case we have had problems here so far, where the failure to round trip was in the CRDS distortion file. Do we intend to check the CRDS reference files as well?

I note that the bounding box is inconsistent with the size of the image, and that we're pretending that the sizes of the pixels are ~1". I think that's likely fine. romanisim has some code for making up a "fake" distortion function here https://github.com/spacetelescope/romanisim/blob/main/romanisim/tests/test_wcs.py#L19-L38 and using that in unit tests instead of the actual CRDS distortion functions. It's not obvious to me that that is helpful here but I link it in case you would have rather used some kind of distortion function.