Tractogram scripts part7: segmentation scripts #993

EmmaRenauld · 2024-05-22T15:52:39Z

Quick description

The four segmentation scripts. (Renaming = made changes in many files). Improved doc everywhere to reference each other.

scil_tractogram_segment_bundles_with_bundleseg.py:
- Was ok! Clarified verifications.
scil_tractogram_segment_connections_from_labels.py:
- A lot of the lines have changed because I moved most of the processing to scilpy.tractanalysis.connectivity_segmentation! Will be difficult to review, sorry.
- Added an option to save the final streamlines in out_dir. Previously, they were saved as soon as you added option --out_dir.
- Added a lot of prints at debug level.
- Only saving intermediate files if that step was processed.
- Reusing the same variable (current_sft) rather than a separate sft name at each step to avoid multiplying data in memory. Using current_sft.streamlines rather than a separate streamlines = current_sft.streamlines, for the same reason.
- Separating the method remove_loops_and_sharp_turns into two: remove_loops, remove_sharp turns for more clarification; the "remove_loops" part was used twice (option args.loop_max_angle), once at the remove loops step, once at the remove_sharp_turn steps. Unnecessary processing removed.
- Encapsulating the "remove overlapping points" section in scilpy.tractograms.streamline_operations
- Using the existing function "filter_streamlines_by_length" rather than the local _prune_length method.
scil_tractogram_tractogram_segment_with_recobundles.py:
- Fixed the verbose. Still had a if args.verbose.
- Explained more the space and origin management. Added a to_rasmm, to_center to clarify, but it's the default when loading.
- Used new scilpy's save_tractogram, that checks if empty first.
scil_tractogram_tractogram_segment_with_ROI_and_score.py:
- Considering that I already have a 30-script PR, let's do it in another PR!!

Type of change

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

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

pep8speaks · 2024-05-22T15:52:57Z

Hello @EmmaRenauld, Thank you for updating !

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-05-31 00:18:40 UTC

codecov · 2024-05-24T15:59:28Z

Codecov Report

Attention: Patch coverage is 79.34509% with 82 lines in your changes are missing coverage. Please review.

Project coverage is 68.23%. Comparing base (17b9a4a) to head (dd02451).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #993      +/-   ##
==========================================
+ Coverage   68.14%   68.23%   +0.09%     
==========================================
  Files         419      419              
  Lines       21369    21388      +19     
  Branches     3216     3206      -10     
==========================================
+ Hits        14561    14594      +33     
+ Misses       5533     5527       -6     
+ Partials     1275     1267       -8

Components	Coverage Δ
Scripts	`69.19% <78.24%> (+0.05%)`	⬆️
Library	`66.85% <80.66%> (+0.18%)`	⬆️

karanphil

Looks good from my limited knowledge of these scripts! Only a small comment. Also, do you know why Github doesn't understand some of the scripts that were simply renamed, instead of thinking you deleted/added a file...

scilpy/tractanalysis/bundle_operations.py

arnaudbore

A couple of questions for this review.

scilpy/tractograms/streamline_and_mask_operations.py

scripts/scil_tractogram_segment_with_recobundles.py

arnaudbore · 2024-05-30T15:40:32Z

scripts/scil_tractogram_segment_connections_from_labels.py

+        formatter_class=argparse.RawTextHelpFormatter,
+        description=__doc__)
+    p.add_argument('in_tractograms', nargs='+',
+                   help='Tractogram filename (s). Format must be one of \n'


do we keep saying trk, tck, fib, vtk ...

I agree, it's heavy. Could belong in a PR removing it everywhere.

arnaudbore · 2024-05-30T15:44:07Z

scripts/scil_tractogram_segment_connections_from_labels.py

+        remove_loops = not args.no_remove_loops
+        remove_outliers = not args.no_remove_outliers
+        remove_curv_dev = not args.no_remove_curv_dev
+        construct_hdf5_from_connectivity(


Pretty sure @frheault can answer this question but why so much post processing here instead of using already cleaned tractograms ?

I wondered the same. We are writing in our draft scilpy paper that we DON'T do that, and people can combine scripts themselves.

It is because each time you clean a tractogram with one script you save a copy of the tractogram and you save it on IO, so it is often required to do it all at once for key steps in the pipeline.

Also, since 30-40% of streamlines don't even touch the labels, you save some processing by computing these on a filtered tractogram (touching labels). Outliers removal is bundle-based, so it has to happen while segmenting (we cant save 10 000 TRK to run 10 000 outliers removal)

We have the scripts to do it yourself and turn it off if you want. But if you wanted to avoid doing inside the script you would have to:
binarize the labels, remove all streamlines not touching the labels (to save 30-40% of computation for loop), run remove loop, then run that script.
Then run that script, it not that complex, but its a function call and 1-2 parameters in that script, so I am not sure if it is worth it to remove it

PS: This script also cut streamline if entering/exiting gray matter multiple time, so this affect the results of postprocessing (because we change the streamline during the script.

Would it make sense to have enough information to be able to replicate the preproc of this script and to do it outside of the script if the user wants and then use it without any preproc ? @frheault

That would require 3-4 scripts to either support .h5 as input and output or to duplicate scripts (one version for trk/tck and one for h5).

That's a complexity that is not so worth it I think.

CHrlS98

I don't know much about segmentation script but I scrolled through the code and, as far as I'm concerned, it seems alright.

EmmaRenauld changed the title ~~[WIP] Tractogram scripts part7: segmentation scripts~~ Tractogram scripts part7: segmentation scripts May 22, 2024

EmmaRenauld requested review from arnaudbore and frheault May 22, 2024 15:55

EmmaRenauld force-pushed the tractogram_scripts_part7 branch from 5f05e88 to 4af9c3b Compare May 24, 2024 15:12

karanphil approved these changes May 30, 2024

View reviewed changes

scilpy/tractanalysis/bundle_operations.py Outdated Show resolved Hide resolved

arnaudbore reviewed May 30, 2024

View reviewed changes

CHrlS98 approved these changes May 30, 2024

View reviewed changes

EmmaRenauld added 11 commits May 30, 2024 19:59

Rename the four segment scripts, and improve doc

27f9b79

recobundles: fix verbose option

fa0cf95

recobundles: Clarify space and origin

5ff0adb

recobundles: simplify sft management

0b7d35f

recobundles: fix tests.

27fc4c4

bundleseg: ok. Only adding comments

84eea5c

connectivity: Encapsulate all processing

35209e8

connectivity: tests pass

d7bd191

connectivity: Replace _prune_length by existing filter_by_length

0661204

Fix pep8

4f636cb

Fix test

61c729a

EmmaRenauld force-pushed the tractogram_scripts_part7 branch from 4af9c3b to 61c729a Compare May 31, 2024 00:11

Answer Arnaud's comments

dd02451

arnaudbore approved these changes Jun 18, 2024

View reviewed changes

arnaudbore merged commit 070d713 into scilus:master Jun 18, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tractogram scripts part7: segmentation scripts #993

Tractogram scripts part7: segmentation scripts #993

EmmaRenauld commented May 22, 2024 •

edited

Loading

pep8speaks commented May 22, 2024 •

edited

Loading

codecov bot commented May 24, 2024 •

edited

Loading

karanphil left a comment •

edited

Loading

arnaudbore left a comment

arnaudbore May 30, 2024

EmmaRenauld May 31, 2024 •

edited

Loading

arnaudbore May 30, 2024

EmmaRenauld May 31, 2024

frheault May 31, 2024 •

edited

Loading

arnaudbore May 31, 2024 •

edited

Loading

frheault Jun 2, 2024

CHrlS98 left a comment

Tractogram scripts part7: segmentation scripts #993

Tractogram scripts part7: segmentation scripts #993

Conversation

EmmaRenauld commented May 22, 2024 • edited Loading

Quick description

Type of change

Checklist

pep8speaks commented May 22, 2024 • edited Loading

Comment last updated at 2024-05-31 00:18:40 UTC

codecov bot commented May 24, 2024 • edited Loading

Codecov Report

karanphil left a comment • edited Loading

Choose a reason for hiding this comment

arnaudbore left a comment

Choose a reason for hiding this comment

arnaudbore May 30, 2024

Choose a reason for hiding this comment

EmmaRenauld May 31, 2024 • edited Loading

Choose a reason for hiding this comment

arnaudbore May 30, 2024

Choose a reason for hiding this comment

EmmaRenauld May 31, 2024

Choose a reason for hiding this comment

frheault May 31, 2024 • edited Loading

Choose a reason for hiding this comment

arnaudbore May 31, 2024 • edited Loading

Choose a reason for hiding this comment

frheault Jun 2, 2024

Choose a reason for hiding this comment

CHrlS98 left a comment

Choose a reason for hiding this comment

EmmaRenauld commented May 22, 2024 •

edited

Loading

pep8speaks commented May 22, 2024 •

edited

Loading

codecov bot commented May 24, 2024 •

edited

Loading

karanphil left a comment •

edited

Loading

EmmaRenauld May 31, 2024 •

edited

Loading

frheault May 31, 2024 •

edited

Loading

arnaudbore May 31, 2024 •

edited

Loading