Unclear difference between b0_treshold and tol

[EmmaRenauld]

Description

In many cases, there seems to be some unclear processing of the variable tol when using a gtab that was created using a b0_treshold.

Way to reproduce

Ex:

from dipy.core.gradients import gradient_table
from dipy.reconst.mcsd import response_from_mask_msmt

gtab = gradient_table(bvals, bvecs, b0_threshold=30)
response_from_mask_msmt(gtab, ..., tol=20)

In this case (and probably many others), the gtab.b0s_mask seems to be ignored: b0_indices = get_bval_indices(bvals, list_bvals[0], tol). There should maybe be at least a check that with both options, we get the same b0s?

This seems like major work in many functions and I wouldn't want to break anything. I'm not sure I'm able to prepare a pull request for you.

Thanks!

[skoudoro]

Thank you for this information @EmmaRenauld,

We will look deeper into it and I agree, we have to be careful with this.

[pjsjongsung]

The main issue seems to be using both the term tol and b0_threshold. The problem is that tol is also used in unique_bvals_tolerance, which could change the b0_mask gradient table had.

I believe we should go in either direction:

1. Include tol or replace b0_threshold with tol in the gradient table class so that bval clustering can be an option (default or not default) when creating the class
2. Make users do bval clustering if they want to instead of having it as a default behavior in some functions

Regardless I think there should be some clean up of how we are using the gradient table so that this kind of overlapping behavior can be prevented. I can create a PR if either of the above options looks good, or if there is another valid option.

[EmmaRenauld]

In our scripts in scilpy, we were also not sure how to proceed. We think there might be use cases where people would have a different threshold for the b0 than their tolerance of other shells. It is difficult to imagine all the use cases. We tried to keep both values as much as possible.

[pjsjongsung]

I agree, but I do not think those cases need to be solved inside each function. If someone needs a different threshold, I think they should do it before passing the gradient table to the function. The problem is that the tol parameter causes a confusion to users who have no intention of using a different tolerance threshold or bval clustering.

We could pass bval clustering as a option in the gradient table class, with an optional parameter tol. However, I am a little bit worried that someone could have been using the tolerance parameter with an actual effect without acknowledging it, which could break their code if this is changed.