qsiprep icon indicating copy to clipboard operation
qsiprep copied to clipboard

Brainmask pixel type to unsigned types

Open jhlegarreta opened this issue 1 year ago • 4 comments

While using the brainmask obtained from qsiprep's preprocessing run, I have run into issues using it, as the pixel type used is a float type: some software might refuse to use such pixel type for a mask, typically requiring unsigned integer (or similar) types.

I only remember very vaguely some conversation in NiBabel or BIDS about similar issues, but I am unable to find the appropriate threads.

Is changing the brainmask pixel type something that can be done or that maintainers consider beneficial? I had a quick look at the code, but I did not find the proper place to propose the change.

jhlegarreta avatar Sep 07 '23 15:09 jhlegarreta

I think as long as the QSIPrep outputs work with the QSIRecon workflows, there is no need to introduce a change. Have you run into any issues in this respect with using QSIPrep outputs outside of QSIRecon workflows? In either case, changing from float to int should be easy enough for users to do independently if they need to.

smeisler avatar Sep 09 '23 22:09 smeisler

Have you run into any issues in this respect with using QSIPrep outputs outside of QSIRecon workflows?

I have run into issues using UKFTractography (refuses to proceed with a mask that has float-type data) and I know other tools (e.g. SCILPY) that would refuse as well.

An optional flag would be enough for me (e.g. --convert_pixel_to_bool or whatever other appropriate name).

It is not a matter of how hard it is to do this on my end; I have already done it, it is a matter of making the process more convenient (I am forced to do the conversion myself, installing additional tools, which is not desirable), and using a data type that honors the content, as using a floating pixel type for a mask is meaningless, and requires more space. And as said, maybe more in line with other more prominent processing tools (I confess that I cannot demonstrate this since I am unable to retrace those vague memories about this issue with such prominent tools).

jhlegarreta avatar Sep 09 '23 22:09 jhlegarreta

I have no problem forcing the mask to be of type int. This wouldn't cause a problem for any of the other recon workflows and is a sensible default.

Would either of those other workflows be good candidates for officially supported recon workflows? Usually the little details like converting data types are handled in those workflows.

mattcieslak avatar Sep 09 '23 22:09 mattcieslak

Thanks for the follw-up @mattcieslak.

Would either of those other workflows be good candidates for officially supported recon workflows? Usually the little details like converting data types are handled in those workflows.

Not sure if I understand the question. I did not mention other workflows, just diffusion data analysis tools. As said, these tools assume the mask data is not a floating point data type, and refuse to perform any step involving mask data as long as it is not an e.g. integer type. These tools will not accept PRs that modify that behavior. Also, as said, strictly speaking, I also think it is meaningless to have a mask stored as a floating point data.

jhlegarreta avatar Sep 12 '23 12:09 jhlegarreta