Updates the default value of rm_small_clusters variable in slr_with_qbx function

[BramshQamar]

This PR addresses issue #2530.

It changes the default value of rm_small_clusters from 50 to 0 in slr_with_qbx function.

[skoudoro]

Hi @BramshQamar,

Thank you for this. However, I am not sure you really fixing the issue here.

The function removes clusters of less than 50 streamlines before performing the registration. In case someone has a very thin bundle or bundle with only one streamline, this function breaks.

Where and why does it break? This is what we need to understand and fix in the code and add tests. We also need to document this.

Please, can you provide more information? I think, we can not merge this for now. thank you for your feedback

[BramshQamar]

Hi @BramshQamar,

Thank you for this. However, I am not sure you really fixing the issue here.

The function removes clusters of less than 50 streamlines before performing the registration. In case someone has a very thin bundle or bundle with only one streamline, this function breaks.

Where and why does it break? This is what we need to understand and fix in the code and add tests. We also need to document this.

Please, can you provide more information? I think, we can not merge this for now. thank you for your feedback

Hi @skoudoro

The default value of the parameter rm_small_clusters was the issue! I changed the default value.

Previously, it was set to 50 and the function would remove clusters that have less than 50 streamlines in them. So if you give an input bundle with only one streamline or a bundle with less than 50 streamlines in it, the function gives an error that there's nothing to register as it removes all the streamlines because the input bundle has less than 50 streamlines in it. In order to perform registration between two bundles, we need those two bundles to have at least one streamline in them. The default value of rm_small_clusters=50 would make the bundle an empty bundle in case it has less than 50 streamlines in it. I changed the value of rm_small_clusters to 0 so that when the input bundle has less than 50 streamlines, the method would still work and perform the registration (as it would not remove clusters/streamlines with 0 default value). I added in the documentation that if a user has a whole brain tractogram, they should set rm_small_clusters to 50.

I hope it clarifies the purpose of this PR. Let me know if you have any questions/concerns about it. Thank you!

[BramshQamar]

And you are right, we should add a test in case the user sets the rm_small_clusters parameter to 50 and one of the input bundles has less than 50 streamlines. Maybe a warning when this happens.

[skoudoro]

yes, you should add a warning or something. because changing this default value does not solve the problem.

Even if you put this default value to 0, if someone changes it to 25 for example, and uses a small bundle, he will not understand why it fails.

I think the issue here is not rm_small_clusters variable value. Everybody can change it at any time. The issue is: Catching the right problem and sending the right message for the user and asking him to change rm_small_clusters

Not sure I am clear. Let me know if you need more information.

[BramshQamar]

yes, you should add a warning or something. because changing this default value does not solve the problem.

Even if you put this default value to 0, if someone changes it to 25 for example, and uses a small bundle, he will not understand why it fails.

I think the issue here is not rm_small_clusters variable value. Everybody can change it at any time. The issue is: Catching the right problem and sending the right message for the user and asking him to change rm_small_clusters

Not sure I am clear. Let me know if you need more information.

Yes, you are right! I will add warnings in the function.

[skoudoro]

Do you have an update for this @BramshQamar?

[skoudoro]

Hi @BramshQamar,

I just updated your PR to fix the issue with small clusters. This is ready to be merged. I am just waiting for the CI's.

If you could also test from your side, it would be great. Thanks !