JP-3129: Fix WCS correction validation #7494
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mcara
force-pushed
the
fix-large-corr-check
branch
from
dae9b09 to
91bbb8a
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #7494 +/- ##
==========================================
- Coverage 77.59% 77.59% -0.01%
==========================================
Files 452 452
Lines 36095 36097 +2
==========================================
+ Hits 28009 28010 +1
- Misses 8086 8087 +1
*This pull request uses carry forward flags. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
mcara
changed the title
mcara
force-pushed
the
fix-large-corr-check
branch
from
91bbb8a to
9b941b5
Compare
mcara
force-pushed
the
fix-large-corr-check
branch
from
9b941b5 to
1fc7b23
Compare
mairanteodoro
approved these changes
Mar 15, 2023
Comment on lines
+395
to
399
| # Increase matching tolerance to pass '_is_wcs_correction_small' test. | ||
| # This test would detect large corrections and therefore | ||
| # would flag the quality of the fit as "bad" and therefore, it will not | ||
| # apply computed corrections ('fit_quality_is_good' test was designed by | ||
| # apply computed corrections ('_is_wcs_correction_small' test was designed by | ||
| # Warren for evaluating "quality of fit" for HAP). |
There was a problem hiding this comment.
Is this comment still valid?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves JP-3129
tweakreg_stephas code that "validates" the fit by checking that corrections (as measured by the change of the positions of image boundary vertices before and after WCS corrections) are smaller than10*tolerance. This is wrong check since valid corrections can be larger than10*tolerancesince usually the magnitude of corrections is dependent onsearchradparameter as well and usuallysearchrad >> tolerance. Any correction smaller thansearchradbut larger than10*toleranceis a valid correction whenuse2dhist=True. Alternatively,xoffsetandyoffsetcan determine the magnitude of corrections. The origin of the factor10in10*toleranceis unexplained too.This PR combines the two quantities -
searchrad(orxoffsetandyoffset) andtolerance- to make this code work for a larger set of step parameters.Checklist for maintainers
CHANGES.rstwithin the relevant release sectionHow to run regression tests on a PR
Regression tests: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/599/