Provider for qbins #170
Provider for qbins #170
Conversation
SimonHeybrock
left a comment
There was a problem hiding this comment.
This does not appear to be used anywhere, what is the idea here?
| wmin, wmax = wbins[0], wbins[-1] | ||
| qmin = reflectometry_q(wavelength=wmax, theta=theta_min) | ||
| qmax = reflectometry_q(wavelength=wmin, theta=theta_max) | ||
| qmin = max(qmin, sc.scalar(1e-3, unit='1/angstrom')) |
There was a problem hiding this comment.
Any reason for this particular hard maximum? Should be documented in the docstring at least.
There was a problem hiding this comment.
From what I know about reflectometry it's a reasonable lower edge.
Depending on the setting for BeamDivergenceLimits the qmin value might be too small, even below 0.
In that case qmin is going to be too low, so it is truncated.
I'll add information about it in the docstring.
It doesn't really matter that we have a hard coded lower bound because if the user wants to set the QBins manually they can do that just like before.
src/ess/amor/utils.py
Outdated
| qmin = reflectometry_q(wavelength=wmax, theta=theta_min) | ||
| qmax = reflectometry_q(wavelength=wmin, theta=theta_max) | ||
| qmin = max(qmin, sc.scalar(1e-3, unit='1/angstrom')) | ||
| return QBins(sc.geomspace('Q', qmin, qmax, 499)) |
There was a problem hiding this comment.
Why 499? Should this be an input param?
There was a problem hiding this comment.
It's a reasonable default. I don't think we should make it an input parameter. If the user wants more control they can set QBins manually.
There was a problem hiding this comment.
Won't people wonder about the unusual number of bins (498)?
There was a problem hiding this comment.
Yeah you're right it's odd, I'll change it to 501 👍
The idea is that the user doesn't need to select It is used in the Amor notebook, see this line that was removed: https://github.com/scipp/essreflectometry/pull/170/files#diff-403cf917aae91bdc5b55ef5e02c326fc55ca8642cc104dddefbadadd5ca44840L63 |
Fixes #142