ALEKernel masks data after convolution

[chrisgorgo]

This means that signal for coordinates outside of the mask will be included. Was this intended?

[tsalo]

We should probably add in a check to remove coordinates from outside the mask before convolution. That can be added as a function in the diagnostics module (#34).

[chrisgorgo]

This in contrast to GingerALE which masks (removes) coordinates outside of the mask prior to convolution.

[tsalo]

I think that's the way to go, so we can add coordinate removal in all of the kernel estimators.

[tsalo]

With #232 merged, this will be pretty easy to implement. I haven't reviewed GingerALE's code though, and the MATLAB implementation I've read shifted foci outside the brain to be just inside it. I just want to be extra-double sure that this is how it's supposed to work. @mriedel56 can you confirm?

[mriedel56]

Just looking at the MATLAB code now, it looks like coordinates are changed not during the ALE stage, but rather, when coordinates are imported. But, coordinates are changed to fit image dimensions, not necessarily the mask. So, given a standard space of 91x109x91, if a foci had image-space coordinates (45,61,104), it would be changed to (45,61,91). Since the edge of images are likely lost anyway during masking, seems like a non-issue.

Cant say what GingerALE does either, sorry.

[tsalo]

Thanks @mriedel56. I guess we'll move forward with masking, and we can change to moving coordinates if we find out it's the way GingerALE does it.

[angielaird]

GingerALE's implementation does not do anything to the coordinates that fall outside the mask. Yes, users receive a warning about these coordinates, but it's just letting them know that they're outside the mask, not that any steps are taken to address this. The coordinates are used in the analysis, but if they are too far out they likely don't contribute much. That is, the peak of their Gaussian is outside the mask but some part of the tail can be included.

[tsalo]

Thank you @angielaird! I guess we should keep it as-is, then. I will close this issue, as well as #234.

[tsalo]

A note about this: #320 heavily invests in the masking-after-convolution approach, since MA maps saved to files in the Dataset will be completely unmasked.