Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more FITS comparison support for regression tests #8919

Open
stscijgbot-jp opened this issue Oct 24, 2024 · 2 comments
Open

Add more FITS comparison support for regression tests #8919

stscijgbot-jp opened this issue Oct 24, 2024 · 2 comments

Comments

@stscijgbot-jp
Copy link
Collaborator

Issue JP-3789 was created on JIRA by Melanie Clarke:

The jwst regression tests primarily use astropy FITSDiff for comparing test products to truth files. With FITSDiff, it isn't possible to set a different tolerance for header value comparisons and data comparisons, and sometimes the data need a broader tolerance than the header does. It is possible to call FITSDiff multiple times, once for the primary header extension with a strict tolerance and once for the data extensions with a looser tolerance, but it's inconvenient, and it would generate two separate reports that are not easy to report in a single test failure.

It would be useful to have a convenience function to wrap FITSDiff calls, to be used in regression tests, that allows a different tolerance by extension and aggregates the reports appropriately. We could also consider adding some more readable summary reports on the differences found per extension: the default list of pixel differences, starting at the edges in, can be hard to parse.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Ned Molter on JIRA:

Adding another thing to the wishlist for FITSDiff calls: at present, when a new header keyword is added or removed, the FITSDiff thinks there's a problem in every header keyword below the one that was added or removed; see e.g. https://github.com/spacetelescope/RegressionTests/actions/runs/11803937276/attempts/1  This leads to very long outputs from what is really a small change.  Also, the line that says "extra keyword in a: KEYWORD" is confusing because it just takes the last keyword in the list even if in reality the new keyword was added somewhere in the middle.

It would be nice to find a way around this, e.g., by ignoring order / position in the list when checking for extra keywords.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Melanie Clarke on JIRA:

Adding another item to consider, stemming from an update to pathloss files from the NIRSpec team in CRDS context jwst_1300.pmap.

Regression tests for data that picked up the new pathloss file (a point source in S200A2 in regression test data jw01309022001_04102_00004_nrs2_cal.fits), showed differences in the pathloss extension in the cal file, but not in the SCI/ERR/VAR arrays. This turns out to be because of the default tolerance settings for fitsdiff: with rtol=1e-5, atol=1e-7, the differences don't show up because the data is on the order of 1e-12. With rtol=1e-5 and atol at default, the differences do show up. This means that essentially no differences will show up in regression tests for NIRSpec science data post flat-fielding, with default tolerances.

We may need to revise the tolerances for all comparisons on NIRSpec data post flat-fielding, or reconsider whether the defaults are doing what we expect them to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant