New upsampling method (PTT) for tractogram #818

frheault · 2023-11-28T20:15:52Z

Quick description

The previous version was simply adding zigzag to streamlines and was barely useful (even for data augmentation it was kind of weird). Now this version uses Parallel Transport to create new streamlines around the previous one using their real geometry. This is helpful for model creation, figures and data augmentation.

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

Provide data, screenshots, command line to test (if relevant)

(The input for that image for single centroids from each bundle)

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

…_tube

pep8speaks · 2023-11-28T20:16:01Z

Hello @frheault, Thank you for updating !

In the file scilpy/tractograms/streamline_operations.py:

Line 396:80: E501 line too long (80 > 79 characters)

In the file scripts/scil_tractogram_compress.py:

Line 21:80: E501 line too long (81 > 79 characters)

In the file scripts/scil_tractogram_resample.py:

Line 65:80: E501 line too long (84 > 79 characters)
Line 67:80: E501 line too long (82 > 79 characters)
Line 68:80: E501 line too long (81 > 79 characters)
Line 73:80: E501 line too long (80 > 79 characters)
Line 74:80: E501 line too long (83 > 79 characters)
Line 80:80: E501 line too long (81 > 79 characters)
Line 81:80: E501 line too long (81 > 79 characters)

Comment last updated at 2024-03-19 17:59:20 UTC

arnaudbore · 2023-11-28T20:54:11Z

Build passed ! Good Job 🍻 !

AntoineTheb

Some minor comments, otherwise LGTM

scilpy/tractograms/tractogram_operations.py

scilpy/utils/spatial_ops.py

arnaudbore · 2023-12-04T19:37:50Z

Build passed ! Good Job 🍻 !

AntoineTheb

Small possible but not required improvement. Otherwise LGTM. Of course, pep8 problems need to be fixed and unit tests would be nice, but can be added later.

scilpy/tractograms/tractogram_operations.py

…_tube

… new_upsampling_tube

arnaudbore · 2023-12-14T16:35:12Z

Build passed ! Good Job 🍻 !

…_tube

… new_upsampling_tube

arnaudbore · 2023-12-18T21:11:32Z

Build passed ! Good Job 🍻 !

…_tube

… new_upsampling_tube

arnaudbore · 2023-12-20T21:14:01Z

Build Failed 💥
https://jenkins.scil.usherbrooke.ca/job/scilpy/job/PR-818/7/

arnaudbore · 2023-12-20T22:16:51Z

Build passed ! Good Job 🍻 !

AntoineTheb

Last round of review. Couple of comments, only the args one is critical.

scripts/scil_tractogram_resample.py

scilpy/utils/spatial_ops.py

…_tube

arnaudbore · 2024-01-29T17:26:31Z

Build passed ! Good Job 🍻 !

AntoineTheb · 2024-01-30T15:31:10Z

@frheault do you need help closing this PR ? It has been open for a while and I would maybe like to use this feature in some work.

frheault · 2024-01-30T16:55:48Z

I just added tests so maybe just check that part? Then it would be ready to merge.

I think the tests are passing but they restarted and got stuck.

arnaudbore · 2024-01-30T17:25:15Z

Build passed ! Good Job 🍻 !

AntoineTheb · 2024-01-31T19:54:41Z

@frheault Tests look good to me ! Only thing remaining are pep8 errors then GTG @arnaudbore imo

arnaudbore · 2024-02-01T19:32:03Z

Build passed ! Good Job 🍻 !

…_tube

scilpy/tractograms/tractogram_operations.py

EmmaRenauld · 2024-02-22T13:55:50Z

scilpy/utils/spatial_ops.py

+import numpy as np
+
+
+def parallel_transport_streamline(streamline, nb_streamlines, radius, rng=None):


After discussion with Arnaud, we would put this in tractogram.streamline_operations.

Can you also move the r(vec, theta) method into utils.util, and name it something like def rotation_around_vector_matrix?

scilpy/utils/spatial_ops.py

scilpy/tractograms/tractogram_operations.py

arnaudbore

A couple of comments but we are really close !

scilpy/tractograms/tractogram_operations.py

scilpy/utils/spatial_ops.py

scilpy/tractograms/streamline_operations.py

scilpy/tractograms/tests/test_streamline_operations.py

…_tube

arnaudbore

Small comment - almost GTG

scilpy/tractanalysis/reproducibility_measures.py

scilpy/tractograms/tractogram_operations.py

EmmaRenauld

Thanks! Much clearer than the first time I read it!

EmmaRenauld · 2024-03-06T14:41:35Z

scilpy/tractograms/tractogram_operations.py

+
+    Parameters
+    ----------
+    tractogram: TrkFile, TckFile, ArraySequence, list


If I understand, this method would work too if tractogram is a StatefulTractogram, no?

scilpy/tractograms/tractogram_operations.py

scripts/scil_tractogram_resample.py

EmmaRenauld · 2024-03-19T17:26:13Z

scripts/scil_tractogram_resample.py

+                                       'points to generate new ones [%(default)s].')
+    upsampling_group.add_argument('--tube_radius', type=float, default=1,
+                                  help='Maximum distance to generate streamlines '
+                                       ' around the original ones [%(default)s].')


two spaces between streamlines and around.

EmmaRenauld · 2024-03-19T17:26:36Z

scripts/scil_tractogram_resample.py

+                                  help='Sigma for smoothing. Use the value of '
+                                       'surrounding X,Y,Z points on the '
+                                       'streamline to blur the streamlines.\n'
+                                       'A good sigma choice would around 5.')


EmmaRenauld · 2024-03-19T17:27:09Z

scripts/scil_tractogram_resample.py

-            (args.streamline_wise_std is not None and
-             args.streamline_wise_std <= 0):
-        parser.error('STD needs to be above 0.')
+    if (args.point_wise_std is not None and args.point_wise_std <= 0):


parenthesis not needed

EmmaRenauld · 2024-03-19T17:27:24Z

scripts/scil_tractogram_resample.py

-        parser.error('STD needs to be above 0.')
+    if (args.point_wise_std is not None and args.point_wise_std <= 0):
+        parser.error('argument --point_wise_std: must be > 0')
+    if (args.tube_radius is not None and args.tube_radius <= 0):


frheault and others added 4 commits November 23, 2023 07:15

Working version, no new script

8f83a61

Fix

a9f62be

working ptt

4c00772

Merge branch 'master' of github.com:scilus/scilpy into new_upsampling…

05d3d9b

…_tube

frheault requested a review from AntoineTheb November 28, 2023 20:15

arnaudbore self-requested a review November 28, 2023 20:54

AntoineTheb requested changes Nov 29, 2023

View reviewed changes

scilpy/tractograms/tractogram_operations.py Outdated Show resolved Hide resolved

scilpy/utils/spatial_ops.py Outdated Show resolved Hide resolved

scilpy/utils/spatial_ops.py Outdated Show resolved Hide resolved

AntoineTheb added the enhancement label Nov 29, 2023

Fix Antoine comment, fix compress, optimize

3ae2cb8

AntoineTheb self-requested a review November 30, 2023 14:59

Fix conflict new scripts

0962d2a

AntoineTheb approved these changes Dec 5, 2023

View reviewed changes

scilpy/tractograms/tractogram_operations.py Outdated Show resolved Hide resolved

frheault added 2 commits December 14, 2023 10:40

Merge branch 'master' of github.com:scilus/scilpy into new_upsampling…

94fe97b

…_tube

Merge branch 'new_upsampling_tube' of github.com:frheault/scilpy into…

4504055

… new_upsampling_tube

frheault added 3 commits December 18, 2023 15:02

Merge branch 'master' of github.com:scilus/scilpy into new_upsampling…

ed7e4a1

…_tube

Merge branch 'json_rename_scripts' of github.com:frheault/scilpy into…

8e6249c

… new_upsampling_tube

Change RNG to generator

f5168ad

frheault added 3 commits December 20, 2023 16:01

Merge branch 'master' of github.com:scilus/scilpy into new_upsampling…

c73fd70

…_tube

Merge branch 'new_upsampling_tube' of github.com:frheault/scilpy into…

255d7cf

… new_upsampling_tube

Fix Pep8

a4f38e7

fix test

7a5fcd9

AntoineTheb requested changes Jan 10, 2024

View reviewed changes

scripts/scil_tractogram_resample.py Outdated Show resolved Hide resolved

scripts/scil_tractogram_resample.py Outdated Show resolved Hide resolved

scilpy/utils/spatial_ops.py Outdated Show resolved Hide resolved

Merge branch 'master' of github.com:scilus/scilpy into new_upsampling…

84fd8f4

…_tube

Merge branch 'master' of github.com:scilus/scilpy into new_upsampling…

9f83929

…_tube

added newline

fba8fcf

frheault added 2 commits February 20, 2024 08:39

Merge branch 'master' of github.com:scilus/scilpy into new_upsampling…

508ce0e

…_tube

Fix conflict

bac39ef

EmmaRenauld reviewed Feb 22, 2024

View reviewed changes

scilpy/tractograms/tractogram_operations.py Show resolved Hide resolved

EmmaRenauld reviewed Feb 22, 2024

View reviewed changes

scilpy/utils/spatial_ops.py Outdated Show resolved Hide resolved

EmmaRenauld reviewed Feb 22, 2024

View reviewed changes

scilpy/tractograms/tractogram_operations.py Show resolved Hide resolved

Move functions and add tests, emmanuel comments

86e8f5b

arnaudbore reviewed Feb 26, 2024

View reviewed changes

frheault and others added 6 commits February 27, 2024 13:44

Merge branch 'master' of github.com:scilus/scilpy into new_upsampling…

c386b56

…_tube

Move function, add tests, pep8

521acab

Fix import

6765016

Fix kdtree type error

5d26160

Revert to sklearn

b32e164

Merge branch 'master' of github.com:scilus/scilpy into new_upsampling…

ebc9205

…_tube

arnaudbore approved these changes Mar 5, 2024

View reviewed changes

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

scilpy/tractograms/tractogram_operations.py Show resolved Hide resolved

EmmaRenauld reviewed Mar 6, 2024

View reviewed changes

frheault added 3 commits March 17, 2024 07:25

address comments

c492232

Fix conflict

c50a518

Fix error for pytest

de85fd9

EmmaRenauld approved these changes Mar 19, 2024

View reviewed changes

Fix Emmanuelle comments in argparse

aa8038c

arnaudbore merged commit c584c7e into scilus:master Mar 20, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New upsampling method (PTT) for tractogram #818

New upsampling method (PTT) for tractogram #818

frheault commented Nov 28, 2023 •

edited

Loading

pep8speaks commented Nov 28, 2023 •

edited

Loading

arnaudbore commented Nov 28, 2023

AntoineTheb left a comment

arnaudbore commented Dec 4, 2023

AntoineTheb left a comment

arnaudbore commented Dec 14, 2023

arnaudbore commented Dec 18, 2023

arnaudbore commented Dec 20, 2023

arnaudbore commented Dec 20, 2023

AntoineTheb left a comment

arnaudbore commented Jan 29, 2024

AntoineTheb commented Jan 30, 2024

frheault commented Jan 30, 2024

arnaudbore commented Jan 30, 2024

AntoineTheb commented Jan 31, 2024

arnaudbore commented Feb 1, 2024

EmmaRenauld Feb 22, 2024

arnaudbore left a comment

arnaudbore left a comment •

edited

Loading

EmmaRenauld left a comment

EmmaRenauld Mar 6, 2024

EmmaRenauld Mar 19, 2024

EmmaRenauld Mar 19, 2024

EmmaRenauld Mar 19, 2024

EmmaRenauld Mar 19, 2024

		import numpy as np


		def parallel_transport_streamline(streamline, nb_streamlines, radius, rng=None):

New upsampling method (PTT) for tractogram #818

New upsampling method (PTT) for tractogram #818

Conversation

frheault commented Nov 28, 2023 • edited Loading

Quick description

Type of change

Provide data, screenshots, command line to test (if relevant)

Checklist

pep8speaks commented Nov 28, 2023 • edited Loading

Comment last updated at 2024-03-19 17:59:20 UTC

arnaudbore commented Nov 28, 2023

AntoineTheb left a comment

Choose a reason for hiding this comment

arnaudbore commented Dec 4, 2023

AntoineTheb left a comment

Choose a reason for hiding this comment

arnaudbore commented Dec 14, 2023

arnaudbore commented Dec 18, 2023

arnaudbore commented Dec 20, 2023

arnaudbore commented Dec 20, 2023

AntoineTheb left a comment

Choose a reason for hiding this comment

arnaudbore commented Jan 29, 2024

AntoineTheb commented Jan 30, 2024

frheault commented Jan 30, 2024

arnaudbore commented Jan 30, 2024

AntoineTheb commented Jan 31, 2024

arnaudbore commented Feb 1, 2024

EmmaRenauld Feb 22, 2024

Choose a reason for hiding this comment

arnaudbore left a comment

Choose a reason for hiding this comment

arnaudbore left a comment • edited Loading

Choose a reason for hiding this comment

EmmaRenauld left a comment

Choose a reason for hiding this comment

EmmaRenauld Mar 6, 2024

Choose a reason for hiding this comment

EmmaRenauld Mar 19, 2024

Choose a reason for hiding this comment

EmmaRenauld Mar 19, 2024

Choose a reason for hiding this comment

EmmaRenauld Mar 19, 2024

Choose a reason for hiding this comment

EmmaRenauld Mar 19, 2024

Choose a reason for hiding this comment

frheault commented Nov 28, 2023 •

edited

Loading

pep8speaks commented Nov 28, 2023 •

edited

Loading

arnaudbore left a comment •

edited

Loading