Closed Lazersmoke closed 1 year ago
TODO's on this include:
BinningParameters
as written only makes sense for linear coordinates, and spheres are nonlinearBinningParameters
objects. The manual conversion is not particularly difficult, but it is annoying to get right.SpinWaveTheory
. There are two possible ways to implement this, depending on whether we trust the spin wave theory calculation at q
vectors which are not compatible with the periodic boundary conditions.This sounds relevant to @ddahlbom
Additional TODO on this, per discussion: need to incorporate broadening into the inner histogram loop, same as LSWT code.
@ddahlbom Currently, the binned intensities functions (linear and spherical) do not handle the form factors, contractors, and possibly kT
correctly, since they bypass the logic in intensities
. What do you + @kbarros recommend to do about the interface for this? I may just finish implementing the binning internals and then turn it over to y'all for making the final user interface arrangements for these parameters (e.g. following the google doc)
@Lazersmoke Sure, that sounds great. Just let us know when when you're done making the final touches. I'd be happy to incorporate the rest of the intensities
logic afterwards.
OK, the final touches are done :)
I've added examples of binned version to the FeI2 and powder averaging tutorials. Some of the plotting is a bit rough around the edges.
There's a remaining question about the exact behavior of the "partial bins" at the far edge of the histogram, sometimes the behavior is possibly not quite correct, but the issue is that it's a bit under-specified what should be done in that case. Mantid has a bunch of options on like every function call that specify what to do with them, seems like. It shouldn't be a big issue for most use cases.
Remaining items for @ddahlbom and co:
integrated_kernel
could be improved, as per discussion in slackget_coordinates
function which generalizes both covectors *
and norm(q)
. Relatedly: The covectors
are in Q space by default, but Mantid is in k space (absolute units). It may be advantageous to be in k space by default for that reason.Thanks @Lazersmoke ! This is a great addition. I'll take a closer look at the code soon. If there are any changes that seem like they should be made before merging, I will comment here.
(I suppose we may want to merge sooner rather than later to keep things in sync. I can complete the remaining tasks after this is integrated.)
This isn't finished/polished yet (opening this PR to track progress).
This is the alternative interface to
intensities
which is designed to more closely align with what Mantid/Horace output. In particular, the new typeBinningParameters
is supposed to be isomorphic to the data Mantid uses to describe an "MDHistoWorkspace".Example usage:
The resulting intensity histogram looks something like this: