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

Fix geometric extrapolation when points-per-decade is provided or data is a single point #568

Open
pkienzle opened this issue May 11, 2023 · 0 comments · Fixed by #571
Open
Assignees

Comments

@pkienzle
Copy link
Contributor

I noticed that the log_delta_q is calculated as the inverse of the equation listed in the documentation:

$$ \log \Delta q = (\log q_n - \log q_1) / (n - 1) $$

This works out as long as this condition for calculating log_delta_q is met as the calculations for n_low and n_high also use the inverse term appropriately. However, if this condition is not met, then log_delta_q is calculated by:
log_delta_q = log(10.) / DEFAULT_POINTS_PER_DECADE
or
log_delta_q = log(10.) / points_per_decade
which are no longer the inverse term. This would result in an incorrect calculation of n_low and n_high.

For example:

In[2]: import numpy as np
In[3]: q = np.logspace(-5, -4, 11)
In[4]: import sasmodels.resolution as resolution
In[5]: resolution.geometric_extrapolation(q, 1e-6, 1e-3)
Out[5]: 
array([1.00000000e-06, 1.25892541e-06, 1.58489319e-06, 1.99526231e-06,
       2.51188643e-06, 3.16227766e-06, 3.98107171e-06, 5.01187234e-06,
       6.30957344e-06, 7.94328235e-06, 1.00000000e-05, 1.25892541e-05,
       1.58489319e-05, 1.99526231e-05, 2.51188643e-05, 3.16227766e-05,
       3.98107171e-05, 5.01187234e-05, 6.30957344e-05, 7.94328235e-05,
       1.00000000e-04, 1.25892541e-04, 1.58489319e-04, 1.99526231e-04,
       2.51188643e-04, 3.16227766e-04, 3.98107171e-04, 5.01187234e-04,
       6.30957344e-04, 7.94328235e-04, 1.00000000e-03])
In[6]: resolution.geometric_extrapolation(q, 1e-6, 1e-3, points_per_decade=10)
Out[6]: 
array([1.00000000e-06, 1.00000000e-05, 1.25892541e-05, 1.58489319e-05,
       1.99526231e-05, 2.51188643e-05, 3.16227766e-05, 3.98107171e-05,
       5.01187234e-05, 6.30957344e-05, 7.94328235e-05, 1.00000000e-04,
       1.00000000e-03])

@pkienzle can you confirm I am understanding intended functionality correctly? I think this issue would pre-date this PR.

Originally posted by @caitwolf in #563 (comment)

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

Successfully merging a pull request may close this issue.

2 participants