Speed up slowest CI tests - #3426
Open
Lestropie wants to merge 2 commits into
Open
Conversation
The FOD FMLS segmenter previously constructed a hardcoded 1281-direction set in every command that uses it, dominating the runtime of the commands that segment FODs. A new FMLSDirectionSet configuration file entry, together with an -fmls_directions command-line option on fod2fixel, now selects either a built-in set by integer count or an external file of directions; resolution follows the precedence of explicit option, then configuration entry, then the unchanged hard-coded default. All commands that segment FODs — afdconnectivity, fod2fixel, tckgen, tcksift, tcksift2, and the dynamic seeding path — obtain the set through a shared accessor rather than constructing it directly, and the reference documentation is regenerated accordingly. Control of the direction set outside fod2fixel is deferred exclusively to the configuration entry, so no other command gains a new option. The tests of those commands now request 321 directions to cut execution time, which required the regression reference data of only fod2fixel tests to be regenerated. Other slow tests were modified to use reduced data. Prompts: 1. > Build the software in debug mode with tests enabled. Run all tests with the "binary" label and no parallel execution. Sort tests by execution time, longest to shortest. Find a suitable bifurcation point in this distribution to select a group of tests for which modifications to reduce execution time is warranted. For each test, analyse the input data consumed by the test. Within the clone of the test data repository that resides within the build directory, propose a modification to the test data that would reduce the computational expense of the test. Some input test data may be consumed by multiple tests; where this occurs, the test itself can be modified to select a subset of the input data before executing the command under evaluation. Regenerate any reference output data for regression testing within the clone of the test data repository within the build directory. 2. > Recently (GitHub #3419) the quadrature for FOD integration was changed from householderQr() to completeOrthogonalDecomposition(), with a reduced lmax. This may be contributing to slow execution of [afdconnectivity, tcksift, tcksift2, tckgen_seed_dynamic]. Testing during the Claude session that produced that change proved that QR decomposition was the wrong choice in that context, as was the use of a large lmax. This worktree has now been rebased to include those changes. Investigate (from prior Claude session content & online) whether an alternative decomposition would yield comparable results to completeOrthogonalDecomposition() but be substantially faster. Speed test the rebased code against prior results in this session. If this does not have more than a 50% effect on test execution time, implement a new MRtrix config file entry that controls which of the built-in direction sets is utilised by default by the FOD FMLS segmenter, and update command documentation of those commands in this list offering an explicit command-line option to choose this set / provide an external text file containing a set to state that in the absence of that command-line option it falls back to that config file entry, or the hard-coded default if that is also absent. 3. > Only add the -fmls_directions option to the fod2fixel command; for other relevant commands, control over this direction set will be deferred exclusively to the config file entry. For relevant tests, use either this option where available or the -config option where not available to utilise the 513 direction set. Indicate which existing tests now fail and therefore require regeneration of reference data for regression testing. Generated-by: Claude Opus 5 <noreply@anthropic.com>
Lestropie
commented
Aug 3, 2026
Co-authored-by: Robert Smith <robert.smith@florey.edu.au>
| // clang-format on | ||
|
|
||
| DWI::Directions::FastLookupSet get_directions() { | ||
| auto opt = App::get_options("fmls_directions"); |
There was a problem hiding this comment.
warning: no header providing "MR::App::get_options" is directly included [misc-include-cleaner]
cpp/core/dwi/fmls.cpp:24:
- #include "dwi/directions/set.h"
+ #include "app.h"
+ #include "dwi/directions/set.h"| if (!opt.empty()) { | ||
| try { | ||
| return DWI::Directions::FastLookupSet(static_cast<std::filesystem::path>(opt[0][0])); | ||
| } catch (Exception &) { |
There was a problem hiding this comment.
warning: no header providing "MR::Exception" is directly included [misc-include-cleaner]
cpp/core/dwi/fmls.cpp:25:
- #include "file/config.h"
+ #include "exception.h"
+ #include "file/config.h"
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Running the entire test suite when compiled in debug mode can be quite slow. When you have multiple Claude sessions working in parallel, and each decides that it needs to run the full test suite in case of unexpected regressions, it can be quite wasteful. So I identified the slowest tests, and targeted modifications that would preserve the utility of the tests but speed up execution.
The main change implementation-wise is being able to use less than the default 1281 directions for FOD segmentation. The matrix solve for quadrature weights is drastically faster with a reduced direction set.
This should more than halve runtime of
ctest -L binary.