[CSimon-A]

DO NOT MERGE UNTIL PR #1373 IS MERGED!!

Quick description

Fixes #1372. Builds on #1373: until it is merged, this PR also shows its two commits. Please review only the last two commits.

BundleSeg methods passed data to each other through self (model_streamlines, model_centroids, neighb_centroids, neighb_indices), so following recognize() meant tracking which method last wrote each attribute. This is what caused #1371. Each step now takes its inputs as arguments and returns its outputs, so the whole data flow is visible in recognize(). Only the data shared by all bundles (tractogram memmaps, clusters, centroids, rng) stays on the object.

Type of change

Refactor: results are unchanged.

• bundleseg.py:
  • _cluster_model_bundle, _reduce_search_space and _register_model_to_neighb return their results; _register_model_to_neighb returns the SLR matrix.
  • recognize() applies the SLR matrix to both the model streamlines and centroids in one place, so the two can no longer get out of sync (BundleSeg: second _reduce_search_space ignores the local SLR #1371).
  • prune_far_from_model is now private (_prune_far_from_model) like the other steps, and takes the registered model and the candidates as arguments.
  • Removed cleanup(), which was never called and did not free anything.
  • Docstrings fixed (several were out of date).
  • The error message now says which search space reduction found no neighbors (16 mm, or 14 mm after the local SLR).
• voting_scheme.py: the transformation is passed to single_recognize instead of being stored on BundleSeg, which never used it.
• test_bundleseg.py: tests of _reduce_search_space and _register_model_to_neighb, now possible because they no longer depend on hidden state.

Interface changes are internal to BundleSeg: the constructor no longer takes transformation, prune_far_from_model is now private, and cleanup() is removed. In scilpy, BundleSeg is only created by VotingScheme. The interfaces of VotingScheme and scil_tractogram_segment_with_bundleseg are unchanged. scil_tractogram_segment_with_recobundles is not affected as it uses dipy's RecoBundles directly and doesn't import voting_scheme.py or bundleseg.py.

Check the relevant options.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

Validation

Results compared exactly (recognized indices and scores) with the code before the refactor (#1373):

• 56 cases, running both versions in the same process: synthetic bundles (model shifts from 0 to 18 mm including the case where nothing is found, 5 seeds, all SLR types, several pruning and clustering thresholds, a model with more than 250 centroids, 3 bundles in a row with the same object) and the fibercup test data (21 models, 2 seeds).
• scil_tractogram_segment_with_bundleseg, seed 0, identical results.json:
  • fibercup: 5 bundles, 2,333 streamlines;
  • HCP sub-111312: 51 bundles, 458,308 streamlines;
  • HCP sub-111312_re: 51 bundles, 445,312 streamlines (includes a bundle with no neighbors at 16 mm).
• pytest src/scilpy/segment and test_tractogram_segment_with_bundleseg.py pass.

Noticed while working on this, out of scope (separate issues): slr_transform_type='scaling' raises IndexError (#1374), and single_recognize_parallel in voting_scheme.py is unused and broken (#1375).

Checklist

• My code follows the style guidelines of this project (run autopep8)
• I added relevant citations to scripts, modules and functions docstrings and descriptions
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I moved all functions from the script file (except the argparser and main) to scilpy modules
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[codecov]

Codecov Report

❌ Patch coverage is 90.24390% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.87%. Comparing base (c787973) to head (d5593b4).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1376      +/-   ##
==========================================
+ Coverage   72.57%   72.87%   +0.29%     
==========================================
  Files         301      301              
  Lines       26279    26278       -1     
  Branches     3702     3700       -2     
==========================================
+ Hits        19073    19150      +77     
+ Misses       5655     5568      -87     
- Partials     1551     1560       +9     

Flag	Coverage Δ
smoketests	69.72% <17.07%> (-0.02%)	⬇️
unittests	14.70% <87.80%> (+0.47%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
Scripts	75.28% <ø> (ø)
Library	69.98% <90.24%> (+0.65%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.