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

fixed switched nomenclature for length and width #533

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from
Draft
Changes from 4 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
ae168e5
fixed switched nomenclature for length and width
caitwolf Oct 29, 2022
fd03bda
fixed single letter variable names of 'w' and 'l'
caitwolf Oct 31, 2022
a6d8ec0
Merge branch 'master' into use-dQ-Data-slit-length-and-width-switched
caitwolf May 11, 2023
81ffb2e
Fixed inverse q-spacing in the geometric extrapolation function
Jun 27, 2023
23b05a9
Merge branch 'master' into 568-fix-geometric-extrapolation-when-point…
caitwolf Jun 27, 2023
f07dc5d
fixing typo from resolving merge conflict to ensure inverse term of l…
Jun 27, 2023
5bdfdce
Merge branch 'master' into use-dQ-Data-slit-length-and-width-switched
pkienzle Jul 25, 2023
a2b169e
Merge branch 'master' into 568-fix-geometric-extrapolation-when-point…
caitwolf Nov 21, 2023
17010b7
Merge branch 'master' into use-dQ-Data-slit-length-and-width-switched
caitwolf Jan 16, 2024
563ad5b
switched order of length and width in slit_resolution method for cons…
caitwolf Jan 16, 2024
d0dfa73
updated romberg_slit_1d to use length, width ordering of arguments, t…
caitwolf Jan 17, 2024
0e0b63d
fixed typo in the doc string for resolution.slit_resolution
caitwolf Jan 17, 2024
aa2b7b2
fixed width, length ordering in slit smearing fucntions to length, width
caitwolf Jan 17, 2024
e57e776
testing opencl ubuntu tests on github ci
caitwolf Jan 17, 2024
98053f4
switched the log_delta_q parameter in the first place so the variable…
caitwolf Jan 18, 2024
2fc80e0
Merge pull request #571 from SasView/568-fix-geometric-extrapolation-…
caitwolf Jan 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 33 additions & 34 deletions sasmodels/resolution.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ def __init__(self, q, q_length=None, q_width=None, q_calc=None):

# Build weight matrix from calculated q values
self.weight_matrix = (
slit_resolution(self.q_calc, self.q, q_length, q_width)
slit_resolution(self.q_calc, self.q, q_width, q_length)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than reversing the order of the parameters here, please reverse the order of the parameters in the slit_resolution method. That way Slit1D (the external API) and slit_resolution will have the same order and future swapping errors a little less likely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched back to q_length, q_width here and updated slit_resolution

)
self.q_calc = abs(self.q_calc)

Expand Down Expand Up @@ -339,7 +339,6 @@ def slit_resolution(q_calc, q, width, length, n_length=30):

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On line 85:

to $q_j \in \left[q, \sqrt{q^2 + \Delta q_\perp}\right]$, so

the $q_\perp$ term has to be squared for the units to work. Which is to say that we should reread and recheck the equations in the doc string.

Copy link
Contributor Author

@caitwolf caitwolf Jan 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've corrected the $q_\perp$ term in the doc string.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this time I have not done a thorough read of the doc string to check for additional typos in the equations.


"""

# np.set_printoptions(precision=6, linewidth=10000)

# The current algorithm is a midpoint rectangle rule.
Expand All @@ -348,37 +347,37 @@ def slit_resolution(q_calc, q, width, length, n_length=30):
weights = np.zeros((len(q), len(q_calc)), 'd')

#print(q_calc)
for i, (qi, w, l) in enumerate(zip(q, width, length)):
if w == 0. and l == 0.:
for i, (qi, wi, li) in enumerate(zip(q, width, length)):
if wi == 0. and li == 0.:
# Perfect resolution, so return the theory value directly.
# Note: assumes that q is a subset of q_calc. If qi need not be
# in q_calc, then we can do a weighted interpolation by looking
# up qi in q_calc, then weighting the result by the relative
# distance to the neighbouring points.
weights[i, :] = (q_calc == qi)
elif l == 0:
weights[i, :] = _q_perp_weights(q_edges, qi, w)
elif w == 0:
in_x = 1.0 * ((q_calc >= qi-l) & (q_calc <= qi+l))
abs_x = 1.0*(q_calc < abs(qi - l)) if qi < l else 0.
#print(qi - l, qi + l)
elif wi == 0:
weights[i, :] = _q_perp_weights(q_edges, qi, li)
elif li == 0:
in_x = 1.0 * ((q_calc >= qi-wi) & (q_calc <= qi+wi))
abs_x = 1.0*(q_calc < abs(qi - wi)) if qi < wi else 0.
#print(qi - w, qi + w)
#print(in_x + abs_x)
weights[i, :] = (in_x + abs_x) * np.diff(q_edges) / (2*l)
weights[i, :] = (in_x + abs_x) * np.diff(q_edges) / (2*wi)
else:
for k in range(-n_length, n_length+1):
weights[i, :] += _q_perp_weights(q_edges, qi+k*l/n_length, w)
weights[i, :] += _q_perp_weights(q_edges, qi+k*wi/n_length, li)
weights[i, :] /= 2*n_length + 1

return weights.T


def _q_perp_weights(q_edges, qi, w):
def _q_perp_weights(q_edges, qi, length):
# Convert bin edges from q to u
u_limit = np.sqrt(qi**2 + w**2)
u_limit = np.sqrt(qi**2 + length**2)
u_edges = q_edges**2 - qi**2
u_edges[q_edges < abs(qi)] = 0.
u_edges[q_edges > u_limit] = u_limit**2 - qi**2
weights = np.diff(np.sqrt(u_edges))/w
weights = np.diff(np.sqrt(u_edges))/length
#print("i, qi",i,qi,qi+width)
#print(q_calc)
#print(weights)
Expand Down Expand Up @@ -581,32 +580,32 @@ def romberg_slit_1d(q, width, length, form, pars):
width = [width]*len(q)
if np.isscalar(length):
length = [length]*len(q)
_int_w = lambda w, qi: eval_form(sqrt(qi**2 + w**2), form, pars)
_int_l = lambda l, qi: eval_form(abs(qi+l), form, pars)
_int_w = lambda wi, qi: eval_form(sqrt(qi**2 + wi**2), form, pars)
_int_l = lambda li, qi: eval_form(abs(qi+li), form, pars)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The $w_i$, $l_l$ labels here are inconsistent with length going to $q_\perp$ (adding in quadrature) and width going to $q_\parallel$ (adding linearly).

I think we should change the romberg_slit_1d signature to use length, width rather than width, length for consistency, and reverse the use of length and width within this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the romberg_slit_1d function to use length, width rather than width, length and reversed the use of length and width within the function. However, this led me to a similar change for slit_extend_q that was still using width, length. It appeared as though slit_extend_q was called in different ways so I've tried to correct this in the appropriate places. The tests that are still implemented are passing, but this will need careful review.

# If both width and length are defined, then it is too slow to use dblquad.
# Instead use trapz on a fixed grid, interpolated into the I(Q) for
# the extended Q range.
#_int_wh = lambda w, h, qi: eval_form(sqrt((qi+h)**2 + w**2), form, pars)
q_calc = slit_extend_q(q, np.asarray(width), np.asarray(length))
Iq = eval_form(q_calc, form, pars)
result = np.empty(len(q))
for i, (qi, w, l) in enumerate(zip(q, width, length)):
if l == 0.:
total = romberg(_int_w, 0, w, args=(qi,),
for i, (qi, wi, li) in enumerate(zip(q, width, length)):
if li == 0.:
total = romberg(_int_w, 0, wi, args=(qi,),
divmax=100, vec_func=True, tol=0, rtol=1e-8)
result[i] = total/w
elif w == 0.:
total = romberg(_int_l, -l, l, args=(qi,),
result[i] = total/wi
elif wi == 0.:
total = romberg(_int_l, -li, li, args=(qi,),
divmax=100, vec_func=True, tol=0, rtol=1e-8)
result[i] = total/(2*l)
result[i] = total/(2*li)
else:
w_grid = np.linspace(0, w, 21)[None, :]
l_grid = np.linspace(-l, l, 23)[:, None]
w_grid = np.linspace(0, wi, 21)[None, :]
l_grid = np.linspace(-li, li, 23)[:, None]
u_sub = sqrt((qi+l_grid)**2 + w_grid**2)
f_at_u = np.interp(u_sub, q_calc, Iq)
#print(np.trapz(Iu, w_grid, axis=1))
total = np.trapz(np.trapz(f_at_u, w_grid, axis=1), l_grid[:, 0])
result[i] = total / (2*l*w)
result[i] = total / (2*li*wi)
# from scipy.integrate import dblquad
# r, err = dblquad(_int_wh, -h, h, lambda h: 0., lambda h: w,
# args=(qi,))
Expand Down Expand Up @@ -1175,13 +1174,13 @@ def demo_slit_1d():
Show example of slit smearing.
"""
q = np.logspace(-4, np.log10(0.2), 100)
w = l = 0.
#w = 0.000000277790
w = 0.0277790
#l = 0.00277790
#l = 0.0277790
resolution = Slit1D(q, w, l)
_eval_demo_1d(resolution, title="(%g,%g) Slit Resolution"%(w, l))
width = length = 0.
#width = 0.000000277790
width = 0.0277790
#length = 0.00277790
#length = 0.0277790
resolution = Slit1D(q, width, length)
_eval_demo_1d(resolution, title="(%g,%g) Slit Resolution"%(width, length))

def demo():
"""
Expand Down