Skip to content

fixed issue #367#382

Open
rozyczko wants to merge 2 commits into
developfrom
367-vector-resolution-interpreted
Open

fixed issue #367#382
rozyczko wants to merge 2 commits into
developfrom
367-vector-resolution-interpreted

Conversation

@rozyczko

Copy link
Copy Markdown
Member

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:

  • All resolution function classes now return sigma (standard deviation) from their 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.
  • The docstrings and comments have been updated throughout to clarify the sigma/FWHM conventions and responsibilities of each wrapper.

Engine wrapper updates:

  • The refnx and refl1d wrappers 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 new SIGMA_TO_FWHM constant for assertions.

Other:

  • Removed unused imports and cleaned up code for clarity.

This change ensures that resolution smearing is handled consistently and transparently, preventing subtle bugs and making cross-engine comparisons reliable.

@rozyczko rozyczko added [scope] bug Bug report or fix (major.minor.PATCH) bugfix Fix to known bug [priority] high Should be prioritized soon labels Jun 15, 2026
@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 31.65%. Comparing base (a472411) to head (2ae07f5).

Additional details and impacted files

Impacted file tree graph

@@             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     
Flag Coverage Δ
integration 31.65% <100.00%> (+31.65%) ⬆️
unittests 0.00% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...rc/easyreflectometry/calculators/refl1d/wrapper.py 63.72% <ø> (ø)
src/easyreflectometry/calculators/refnx/wrapper.py 80.64% <100.00%> (ø)
...rc/easyreflectometry/model/resolution_functions.py 80.39% <100.00%> (+80.39%) ⬆️

... and 40 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Labels

bugfix Fix to known bug [priority] high Should be prioritized soon [scope] bug Bug report or fix (major.minor.PATCH)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant