[FIX] correctly calculate min / max sl length, update step_size docs #905
Conversation
|
The old default min and max sl lengths were so lenient that this would not be a problem in previous runs. Only with the recent merge has this become a problem. |
|
Just to finish off the VOF/pARC addition, we need to make the cleaning parameters stronger. |
|
This is heading towards fixing master, however the problem is some bundles, like the IFOF, cannot be found even with max streamline length of 250 mm. So we may need to do some testing on bundle length, or maybe there is some bug in converting max/min sl length to step size that I don't yet understand. I am going to pause on this for now to focus on other things (GRFP , IN-BIC meeting) |
|
With min SL 20mm, max SL 250mm |
|
Nevermind, tweaked defaults/example reqs and now it has a chance of passing the docs test. Still, we should revisit the min/max bundle lengths stuff in the future. |
|
@arokem The broken master has to do with changes to min/max SL length in the recent PR. I fixed a bug in this code (which was not previously relevant as we had lenient min/max) but it seems like some bundles (FA/FP) require a small min sl length. I think we should make min/max sl length a bundle-specific parameter in the future, and then merge this PR to fix master. |
|
I also changed the cleaning parameter reasonable default. This helps with VOF/pAF. This we should also make bundle-specific in the future. |
| if len_sl > self.min_length * self.step_size\ | ||
| and len_sl < self.max_length * self.step_size\ | ||
| and len_sl > 1: | ||
| if len_sl > self.min_length: |
There was a problem hiding this comment.
Doesn't this change mean we are no longer enforcing max_length?
There was a problem hiding this comment.
I think that DIPY already uses a max_length. its enforced via the size of F and the function _tracker .
Previously, min_length, max_length, and step_size were all in voxel space units but we docs said they were in mm. Now, min_length and max_length are input in mm and docs clarify that step_size is in voxel space units. Then, min and max length are converted to step size units. I think this makes the most sense.