transform_coords building blocks for gravity correction in theta vs wavelength transformations #50
Conversation
| return sc.to_unit(h / (wavelength * neutron_mass), sc.units.m / sc.units.s, | ||
| copy=False) |
There was a problem hiding this comment.
Rewrite this to do large operations last. As written here we first create multiple large intermediates, consuming a lot of memory and compute. Rewrite to multiply with wavelength as the very last step. Also to_unit could be done before.
|
|
||
|
|
||
| # Derivative of y with respect to z | ||
| def to_y_dash(wavelength, sample_position, detector_position): |
There was a problem hiding this comment.
Could this use scattered_beam instead of the two positions?
| return (-0.5 * sc.norm(gy) | ||
| * diff.fields.z / velocity_sq) + (diff.fields.y / diff.fields.z) |
There was a problem hiding this comment.
This is bound to break since it hard-codes the direction of gravity. I would recommend to require a g coordinate that is defined as a vector with every data array. Essentially this vector would replace (parts of) what Mantid defines as "reference frame".
| def to_scattering_angle(w, wavelength, detector_id, position, sample_position): | ||
| z_origin = sample_position.fields.z | ||
| y_origin = sample_position.fields.y | ||
| z_measured = position.fields.z | ||
| y_dash = to_y_dash(wavelength, sample_position, position) | ||
| height = y_dash * (z_measured - z_origin) + y_origin | ||
| return sc.atan2(y=height, x=z_measured) - w |
There was a problem hiding this comment.
Hard coding some directions as well? Can we define/require, e.g., a sample-normal vector instead?
There was a problem hiding this comment.
Yes, fully agree. Would be much happier if we did this with configurable beam and sample normal (up) directions.
There was a problem hiding this comment.
Would not call it up though, since there are reflectometry beamlines where "up" is not the sample normal, since the beamline "lies" on its side.
Calculations previously assumed forward beam direction z and vertical in y.
|
|
||
|
|
||
| def _angle(a, b): | ||
| return sc.acos(sc.dot(a, b) / (sc.norm(a) * sc.norm(b))) |
There was a problem hiding this comment.
Those functions will be provided by scippneutron. I think we should prefer finding a good API there and using that here. ess should only provide specialised building blocks that can be added to or replace components of graphs in scippneutron.
|
The use of |
Yes, but I think we should also avoid testing |
Aim is to put generic building blocks in place rather than wire it in.
transform_coordsbecame available in scipp 0.8 and is the new generic mechanism.Effectively does this
See non
transform_coordsimplementation https://scipp.github.io/ess-notebooks/reflectometry/amor_reduction_detailed.html#Theta-calculation-and-gravity-correctionThis implementation produces identical for input data that @arm61 uses:
