Closed SimonHeybrock closed 7 months ago
I'm not sure I understand this code, but are they not already part of the coord transform graph here: https://github.com/scipp/scippneutron/blob/834ed0399e743d4948348dcbecd26ae629d38013/src/scippneutron/conversion/graph/tof.py#L39
I have to check if the definition used in SANS is fully equivalent. It might be that their $Q_x$ and $Q_y$ is in a plane perpendicular to the incident beam and not defined by the (x,y,z) coord system.
I have asked the scientists on Slack. If you are impatient, you may dig into this to find out: https://github.com/mantidproject/mantid/blob/main/Framework/Algorithms/src/Qxy.cpp.
Seems like the directions $e_x$ and $e_y$ are the same as in the coordinate system spectruminfo
uses to specify samplePos
and detector element positions. So it can't be 'local to the detector element'.
Then probably $e_z =$ beam direction.
The definition we use can be found in Qxy.cpp
the other thing that’s contained in this function is the gravity correction.
The basic definition is
const double Q = 4.0 * M_PI * sinTheta / wavLength;
// Now get the x & y components of Q.
const double Qx = a * Q;
// Test whether they're in range, if not go to next spectrum.
if (Qx < axis.front() || Qx >= axis.back())
break;
const double Qy = b * Q;
where
double phi = atan2(detPos.Y(), detPos.X());
double a = cos(phi);
double b = sin(phi);
double sinTheta = sin(spectrumInfo.twoTheta(i) * 0.5);
Qxy is also used by the ILL and EQSANS reduction workflows.
I think this is probably equivalent to the definition using wave vectors used in ScippNeutron (but that does not do gravity). We can check which yields better performance. Note the comment on gravity, but look at Qxy.cpp
, math is not included in the snippets above.
For now I am adding this in ESSsans, as there are already related bits such as gravity handling. We should consider moved this to ScippNeutron in its entirety.
I'll call this fixed by scipp/esssans#71. I will open a new issue for moving all generic conversion code from esssans to ScippNeutron
This is needed for SANS in particular. Should it be added to the regular coord transform graphs?