get_roi_mask vs. get_roi_masks

[alexhuth]

Per a discussion today, it's silly to have two (extremely similarly named) functions that return ROI masks in volume space. Clearly get_roi_mask has fewer features and is pretty lame because it relies on the backwards mapper. But on the other hand, I think that get_roi_masks is extremely bloated and over-complicated (90 lines of code! this function should be ~10, tops).

I think we should replace both with a function that is clear and powerful (and doesn't have "poop" in the doc string). What should that function do? I propose that to convert ROIs from vertex-space to voxel-space we should create a separate floating-point voxel-space mask for each ROI. The value for each voxel should be the fraction of that voxel that overlaps with the ROI. This should be easy to do using the volume mapper & freesurfer surfaces. These fractions could then be compared or thresholded to produce binary maps.

It seems to me like this new function would be very efficient. It would require perhaps re-normalizing the volume mapper matrix, and then 10-ish lines of code to get the ROI weights.

Ideas? Thoughts?

[jamesgao]

I support the use of get_roi_mask with a simplified codebase for the "backwards" function. I'll look into that over the next few days, but I suspect that better documentation would benefit the package way more than the better roi_mask function. You two should decide on which one ships with the initial version...

[marklescroart]

First: I think "poop" should be in more docstrings.

I don't think it's horrible to have one function for single rois and one function for all rois. The plural may seem redundant, but the difference is extremely easy to interpret. Why make people write their own wrapper for a common use case?

One of the benefits of get_roi_masks is that it distinguishes the left and right hemispheres (R has positive indices, L has negative). I don't think there is any other function that does it; a fair bit of the complexity in the code deals with this. Another aspect of the code is that it assigns each voxel to only one roi. I don't think that get_roi_mask does that. And yes, this could be accomplished with some thresholding on continuous values, but we don't have those continuous values yet.

Another benefit of get_roi_masks is that the output format is compact and easy to work with. This could be my familiarity bias, but this is also how Shinji formatted his rois (which is where I picked up the format). I honestly think this is what many users will want.

[alexhuth]

I don't think it's horrible to have one function for single rois and one function for all rois. The plural may seem redundant, but the difference is extremely easy to interpret. Why make people write their own wrapper for a common use case?

get_roi_mask actually much of the functionality of get_roi_masks, despite the name. It returns the masks for all rois, but doesn’t make sure that they’re non-overlapping.

One of the benefits of get_roi_masks is that it distinguishes the left and right hemispheres (R has positive indices, L has negative). I don't think there is any other function that does it; a fair bit of the complexity in the code deals with this. Another aspect of the code is that it assigns each voxel to only one roi. I don't think that get_roi_mask does that.

I agree that this is the one big benefit of get_roi_masks: it divides voxels into non-overlapping sets. But I think if we can produce a continuous-valued array across voxels and ROIs, a simple argmax over this array would produce the same result. The whole function could be fewer than 10 lines of code: get the mapper, get the ROI vertices, run the vertices through the new “backwards” mapper, then optionally package and binarize the result. And yes, this could be accomplished with some thresholding on continuous values, but we don't have those continuous values yet.

Agreed, let’s make this happen. Another benefit of get_roi_masks is that the output format is compact and easy to work with. This could be my familiarity bias, but this is also how Shinji formatted his rois (which is where I picked up the format). I honestly think this is what many users will want.

Really? Because I think the output format is pretty ugly. There is just no reason to pack the ROI definitions into a single volume. If you’re going to use ROI definitions, you’re almost certainly going to create a binary mask from this volume (something like “v1roi = rois == 4”, ugh). And then you need to pass around a variable containing the index for each ROI anyway. I think passing around either (1) a dictionary of {binary ROI mask : ROI name} or (2) a dictionary of {continuous ROI mask : ROI name} would be consistent and clean. Plus it would be super easy to convert to a single volume if you really wanted to, you could do it in one line of code.

But do you agree with the ROI vertex -> voxel definition that I proposed? Should it be based on the fraction of voxel volume inside an ROI?

[marklescroart]

I don't find the single-volume format cumbersome, and it makes other operations easier (early visual cortex is abs(roiMask)<6, e.g. ).

But whatever, the dictionary format you described is fine and more straightforward. Inputs / Options should be:

(1) binary or continuous output (as you describe it). (2) split L/R hemispheres or don't. Some people will want this for sure. and possibly: (3) list of ROIs for which to get masks (easy to do at the output; would any time savings from only computing SOME rois be worth it at all?) (4) optional distance measure for cortical thickness (from fiducial surface, or using pycortex' cortical mask). This adds ~2 lines of code. Weird shit (sulcal crossing) can happen if you set this to too high a value, though.

[jamesgao]

Hmm, I just realized there might be a very simple way to get around this. What if we just generated the roi masks in anatomical space, then downsampled them to epi space? We could select all the anatomical voxels that fall within the line between white matter and pial surface. This seems like an easier job than trying to do everything directly in epi space...