fixed issue #367#382
Open
rozyczko wants to merge 2 commits into
Open
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #382 +/- ##
============================================
+ Coverage 0.00% 31.65% +31.65%
============================================
Files 42 48 +6
Lines 3578 3832 +254
============================================
+ Hits 0 1213 +1213
+ Misses 3578 2619 -959
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This file contains hidden or 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
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.
This pull request standardizes the handling of resolution function width conventions across the codebase, ensuring that all resolution functions return sigma (the standard deviation of a Gaussian) and that each engine wrapper (refnx and refl1d) interprets these values correctly. This fixes long-standing inconsistencies (see GitHub issue #367) where the same resolution function could produce different results depending on the backend. The PR also adds comprehensive cross-engine regression tests and updates all affected tests to the new conventions.
Resolution function standardization:
smearing()method, regardless of their parameterization (e.g., FWHM percentage, FWHM knots, or pointwise sigma). A new constant,SIGMA_TO_FWHM, is used for conversion where needed.Engine wrapper updates:
refnxandrefl1dwrappers now correctly convert sigma to the expected width convention for their respective engines: refnx expects FWHM, refl1d expects sigma. This removes previous ad-hoc or missing conversions.Testing improvements:
A new integration test (
test_cross_engine_resolution.py) verifies that the same model and resolution function produce consistent results across both engines, with a tolerance wide enough to catch convention errors but not minor algorithmic differences.All affected unit tests have been updated to expect sigma output from
smearing(), using the newSIGMA_TO_FWHMconstant for assertions.Other:
This change ensures that resolution smearing is handled consistently and transparently, preventing subtle bugs and making cross-engine comparisons reliable.