Skip to content

Change ThreadedLoop to default to 2 inner axes#3293

Open
Lestropie wants to merge 3 commits intodevfrom
threaded_loop_inner_axes
Open

Change ThreadedLoop to default to 2 inner axes#3293
Lestropie wants to merge 3 commits intodevfrom
threaded_loop_inner_axes

Conversation

@Lestropie
Copy link
Copy Markdown
Member

Extension of #3280.

Closes #3284.


As proven in #3280, where an image processing operation is comparatively cheap, it can be substantially faster to place 2 image axes rather than 1 within the inner loop of the multi-threading mechanism, as there is less time spent achieving synchronisation across threads.
Conversely, I believe the purpose of processing 1D stripes of image data by default rather than slices is that, for a large number of threads relative to the number of slices, a substantial fraction of the runtime could be spent waiting for the last slices to complete, during which time not all threads available are utilised.
As such, my expectation is that fastest execution will be achieved using:

  • 1 axis in the inner loop for very expensive voxel-wise operations
  • 2 axes in the inner loop for inexpensive operations

Exactly where the threshold lies I do not know.
I did however find after a first attempt at refining that it is likely a minority of multi-threaded image operations that are sufficiently expensive to warrant the use of one axis in the inner loop. Currently this is exclusively:

  • dwi2fod
  • dwi2tensor
  • dwidenoise
  • FOD reorientation

If there's contention about what should & shouldn't be in this list we can do speed tests; but it needs to be with realistic data, not the CI test data.

(@MRtrix3/mrtrix3-devs note also use of std::optional for function arguments; facilitates differentiation between explicit and default values)

@Lestropie Lestropie requested a review from jdtournier April 1, 2026 06:26
@Lestropie Lestropie self-assigned this Apr 1, 2026
github-actions[bot]

This comment was marked as outdated.

Original code compiled and ran in debug mode locally, but failed CI tests due to assertion failure in std::optional usage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant