Add tractoflow subworkflow #82

AlexVCaron · 2024-12-17T20:18:46Z

Describe your changes

new subworkflow for tractoflow. I will lint and add tests soon

List test packages used by your subworkflow

Checklist before requesting a review

Create the tool:
- Edit ./subworkflows/nf-neuro/<category>/<tool>/main.nf
- Edit ./subworkflows/nf-neuro/<category>/<tool>/meta.yml
Generate the tests:
- Edit ./subworkflows/nf-neuro/<category>/<tool>/tests/main.nf.test
- Run the tests to generate the main.nf.test.snap snapshots
Ensure the syntax is correct :
- Run prettier and editorconfig-checker to fix common syntax issues
- Run nf-core subworkflows lint and fix all errors
- Ensure your variables have good, clear names

arnaudbore

We are getting there !! We may need a discussion on teams to follow up with this discussion about params names and apart from a couple of comments everything seems ready.

arnaudbore · 2025-01-14T20:33:05Z

subworkflows/nf-neuro/tractoflow/main.nf

+        RECONST_FRF( ch_reconst_frf )
+        ch_versions = ch_versions.mix(RECONST_FRF.out.versions.first())
+
+        /* Run fiber response aeraging over subjects */


Suggested change

/* Run fiber response aeraging over subjects */

/* Run fiber response averaging over subjects */

arnaudbore · 2025-01-14T20:37:43Z

subworkflows/nf-neuro/tractoflow/meta.yml

+  acquisition biases, align anatomical and diffusion space, compute DTI and fODF metrics and generate whole brain
+  tractograms.
+  ----------  Configuration  ----------
+  - nextflow.config : contains an example configuration for the subworkflow. Includes :


Remove Includes :

arnaudbore · 2025-01-14T20:43:57Z

subworkflows/nf-neuro/tractoflow/nextflow.config

+    dwi_fodf_fit_peaks_ventricle_min_md             = 0.003
+
+    //**PFT tracking**//
+    fodf_fit_pft                                    = true


run_pft_tracking makes more sense to me. BUT I didn't see anywhere else.

arnaudbore · 2025-01-14T20:46:19Z

subworkflows/nf-neuro/tractoflow/nextflow.config

+    fodf_fit_pft                                    = true
+    fodf_pft_fit_random_seed                        = 0
+    fodf_pft_fit_algorithm                          = "prob"
+    fodf_pft_fit_step_size                          = 0.5


These names are imo too long why not using: pft_tracking_step_size etc... same for local_tracking_

I think we we could, as much as possible and it still needs to make sense, keep the same var name we had with the original tractoflow.

I agree with this, the simpler the parameters' names, the easier the user will understand how to use them. I think those are only for example purposes; when the subworkflow will be included in a pipeline, the nextflow.config and modules.config will need to be either copied or rewritten so they can be changed in the pipeline implementation of tractoflow.

arnaudbore · 2025-01-16T20:14:16Z

subworkflows/nf-neuro/tractoflow/meta.yml

+    on the preprocessed DWI image and extract relevant metrics.
+  PFT TRACKING (dipy)
+    Perform Particle Filtering Tractography (PFT) on the FODF to generate whole brain tractograms.
+  LOCAL TRACKING (dipy)


GPU implementation if 100% scilpy

gagnonanthony

Amazing job! 💯 I wrote some comments, but they shouldn't be hard to solve!

gagnonanthony · 2025-01-17T14:04:25Z

subworkflows/nf-neuro/tractoflow/main.nf

+            ch_dwi, ch_rev_dwi,
+            ch_sbref, ch_rev_sbref,
+            ch_topup_config


Is there a specific reason why you wrote the inputs as a pair rather than a vertical list? With the reverse images, it does make sense, but I think I would make it homogeneous with how we call the other subworkflows.

gagnonanthony · 2025-01-17T14:08:15Z

subworkflows/nf-neuro/tractoflow/main.nf

+        ch_registration_fa = PREPROC_DWI.out.dwi_resample
+            .join(PREPROC_DWI.out.bval)
+            .join(PREPROC_DWI.out.bvec)
+            .join(PREPROC_DWI.out.b0_mask)
+
+        REGISTRATION_FA( ch_registration_fa )
+        ch_versions = ch_versions.mix(REGISTRATION_FA.out.versions.first())


I think this adds a process that could be avoided. Unless I am missing something, the complete DTI process (lines 137-143) could be placed here, and the resulting FA map could be used for registration. This would avoid computing the FA twice and reduce runtime.

gagnonanthony · 2025-01-17T14:13:23Z

subworkflows/nf-neuro/tractoflow/main.nf

+        ch_pft_tracking = ANATOMICAL_SEGMENTATION.out.wm_mask
+            .join(ANATOMICAL_SEGMENTATION.out.gm_mask)
+            .join(ANATOMICAL_SEGMENTATION.out.csf_mask)
+            .join(RECONST_FODF.out.fodf)
+            .join(RECONST_DTIMETRICS.out.fa)
+        TRACKING_PFTTRACKING( ch_pft_tracking )
+        ch_versions = ch_versions.mix(TRACKING_PFTTRACKING.out.versions.first())
+
+        //
+        // MODULE: Run TRACKING/LOCALTRACKING
+        //
+        ch_local_tracking = ANATOMICAL_SEGMENTATION.out.wm_mask
+            .join(RECONST_FODF.out.fodf)
+            .join(RECONST_DTIMETRICS.out.fa)
+        TRACKING_LOCALTRACKING( ch_local_tracking )
+        ch_versions = ch_versions.mix(TRACKING_LOCALTRACKING.out.versions.first())


Do we expect to run both tracking methods in a single run? We should use parameters to select which one we want to run.

gagnonanthony · 2025-01-17T14:13:49Z

subworkflows/nf-neuro/tractoflow/meta.yml

@@ -0,0 +1,347 @@
+# yaml-language-server: $schema=https://raw.githubusercontent.com/nf-core/modules/master/subworkflows/yaml-schema.json


I think you can drop this line.

gagnonanthony · 2025-01-17T14:19:06Z

subworkflows/nf-neuro/tractoflow/nextflow.config

+    fodf_fit_pft                                    = true
+    fodf_pft_fit_random_seed                        = 0
+    fodf_pft_fit_algorithm                          = "prob"
+    fodf_pft_fit_step_size                          = 0.5


I agree with this, the simpler the parameters' names, the easier the user will understand how to use them. I think those are only for example purposes; when the subworkflow will be included in a pipeline, the nextflow.config and modules.config will need to be either copied or rewritten so they can be changed in the pipeline implementation of tractoflow.

ThoumyreStanislas

Excellent work Alex! It's crazy to see it all finished. For my part, I don't have many comments, I agree with Arnaud and Anthony about the input nomenclature for FRF. Anthony's comment is relevant to the DTI_metrics module, which could be earlier to avoid calculating fa twice. But that's all, well done again! 🚀 🥇

ThoumyreStanislas · 2025-01-21T09:33:46Z

subworkflows/nf-neuro/tractoflow/meta.yml

+      description: |
+        (Optional) The input channel containing the brain probability mask for antsBET,
+        with intensity range 1 (definitely brain) to 0 (definitely background).
+        Structure: [ val(meta), path(probability_map) ]


For anatomical segmentation, you can also add a lesion mask as an input to complete the segmentation. It's optional.

pull-request-management bot requested review from gagnonanthony and Manonedde December 17, 2024 20:18

AlexVCaron mentioned this pull request Dec 18, 2024

[FIX] Test data caching and unzipping #83

Merged

4 tasks

AlexVCaron force-pushed the feat/tractoflow branch from 1e37018 to 8c9a0d5 Compare January 14, 2025 19:19

AlexVCaron added 5 commits January 14, 2025 19:19

first implementation and metadata

e1b5609

more metadata, added test, fix to modules and more configuration

ab51483

prettier and lint (just a bit)

c53ca31

finish lint

60ee8a0

will have to open an issue for more options for eddy in scipy

eaeca16

AlexVCaron force-pushed the feat/tractoflow branch from 8c9a0d5 to eaeca16 Compare January 14, 2025 19:20

arnaudbore requested review from arnaudbore and ThoumyreStanislas January 16, 2025 19:16

arnaudbore reviewed Jan 16, 2025

View reviewed changes

gagnonanthony requested changes Jan 17, 2025

View reviewed changes

ThoumyreStanislas reviewed Jan 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tractoflow subworkflow #82

Add tractoflow subworkflow #82

AlexVCaron commented Dec 17, 2024

arnaudbore left a comment

arnaudbore Jan 14, 2025

arnaudbore Jan 14, 2025

arnaudbore Jan 14, 2025

arnaudbore Jan 14, 2025

arnaudbore Jan 16, 2025

gagnonanthony Jan 17, 2025

arnaudbore Jan 16, 2025

gagnonanthony left a comment

gagnonanthony Jan 17, 2025

gagnonanthony Jan 17, 2025

gagnonanthony Jan 17, 2025

gagnonanthony Jan 17, 2025

gagnonanthony Jan 17, 2025

ThoumyreStanislas left a comment

ThoumyreStanislas Jan 21, 2025

	/* Run fiber response aeraging over subjects */
	/* Run fiber response averaging over subjects */

		@@ -0,0 +1,347 @@
		# yaml-language-server: $schema=https://raw.githubusercontent.com/nf-core/modules/master/subworkflows/yaml-schema.json

Add tractoflow subworkflow #82

Are you sure you want to change the base?

Add tractoflow subworkflow #82

Conversation

AlexVCaron commented Dec 17, 2024

Describe your changes

List test packages used by your subworkflow

Checklist before requesting a review

arnaudbore left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gagnonanthony left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThoumyreStanislas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment