bids-apps / MRtrix3_connectome

Generate subject connectomes from raw BIDS data & perform inter-subject connection density normalisation, using the MRtrix3 software package.
http://www.mrtrix.org/
Apache License 2.0
49 stars 26 forks source link

parcellation none followed by --preprocessed run #55

Closed dkp closed 3 years ago

dkp commented 5 years ago

Hi Robert,

I was trying the following: Run Mrtrix3_connectome using parcellation none in order to complete the preprocessing. The goal was to use the resulting directory in subsequent commands with the --preprocessed flag.

docker run -i --rm -v ${PWD}:/bids_dataset -v ${PWD}/derivatives:/outputs bids/mrtrix3_connectome /bids_dataset /outputs participant --participant_label 102 --parcellation none --output_verbosity 3 --debug

followed by this (run from the derivatives dir)

docker run -i --rm -v ${PWD}:/bids_dataset -v ${PWD}/desikan:/outputs bids/mrtrix3_connectome /bids_dataset /outputs participant --participant_label 102 --parcellation desikan --preprocessed --output_verbosity 3 --debug

This almost works. But at the very end of the 2nd docker command it fails:

mrtrix3_connectome.py: Changing back to original directory (/)
mrtrix3_connectome.py: Script failed while executing the command: connectome2tck tractogram.tck assignments.csv exemplars.tck -tck_weights_in weights.csv -exemplars parc.mif -files single

If I go to the temp directory, I can run the step that killed it using -force (just calling MRtrix on my mac):

$ connectome2tck tractogram.tck assignments.csv exemplars.tck -tck_weights_in weights.csv -exemplars parc.mif -files single
connectome2tck: [done] reading streamline assignments file
connectome2tck: [WARNING] Parcellation image "parc.mif" provided via -exemplars option contains more nodes (84) than are present in input assignments file "assignments.csv" (83)
connectome2tck: [100%] generating exemplars for connectome
connectome2tck: [100%] finalizing exemplars
connectome2tck: [ERROR] output file "exemplars.tck" already exists (use -force option to force overwrite)
$ connectome2tck tractogram.tck assignments.csv exemplars.tck -tck_weights_in weights.csv -exemplars parc.mif -files single -force
connectome2tck: [WARNING] existing output files will be overwritten
connectome2tck: [done] reading streamline assignments file
connectome2tck: [WARNING] Parcellation image "parc.mif" provided via -exemplars option contains more nodes (84) than are present in input assignments file "assignments.csv" (83)
connectome2tck: [100%] generating exemplars for connectome
connectome2tck: [100%] finalizing exemplars

Any idea what is happening here? What is parcellation none for? Should this work?

Thanks,

Dianne

Lestropie commented 5 years ago

Run Mrtrix3_connectome using --parcellation none in order to complete the preprocessing. The goal was to use the resulting directory in subsequent commands with the --preprocessed flag.

I haven't actually thought about that kind of usage. My suspicion is that if it weren't for the connectome2tck issue it would stumble later nevertheless: the second run with --preprocessed will try to overwrite output files that will have already been written by the first run with --parcellation none.

The main purpose of the --preprocessed flag is actually to enable CI testing, which times out if attempting to run dwipreproc :-/

I should try to support this kind of usage in the future. In part I've been holding off on such capabilities because I want a better definition of BIDS Derivatives in order to be able to determine what has and has not been generated / written to the output directory, in a way that is not specific to just this script.


connectome2tck: [WARNING] Parcellation image "parc.mif" provided via -exemplars option contains more nodes (84) than are present in input assignments file "assignments.csv" (83)

Given the Desikan parcellation contains 84 nodes, I would hypothesize that the parcellation image is correct, but the assignments file is not. My best guess is that node 84 (right cerebellar cortex) does not have any streamlines assigned to it, and therefore never appears in the assignments file (command connectome2tck relies on the streamline-to-node assignments data to determine (guess) how many nodes are in the parcellation). So maybe take a look at the tractogram / connectome matrix and see if this is the case?

This is however only a warning; I don't see why this should have terminated execution. There should be a file error.txt in the script scratch directory that gives the output of the failed command?

dkp commented 5 years ago

Thank you for your thoughtful reply. I found error.txt as you suggested, and have attached it. error.txt

Of course, you are absolutely right that the main goal is really to not lose all the work if MRtrix3_connectome crashes.
(On the HPC, if the job gets killed, the temp directory gets removed...which is especially painful) ; ( ...so we are all dreaming of a time when MRtrix3_connectome can pick up where it left off. I will stop trying to use none..bad idea apparently.

-Dianne

Lestropie commented 5 years ago

I think there's a discrepancy between what --parcellation none performs, and what you're looking for. The former will still do e.g. CSD, and may even generate a tractogram depending on command-line options; it just doesn't generate a connectome. Whereas what you're looking for is to strictly perform "pre-processing" only. That would require its own dedicated command-line option. You could add that feature request as a new issue if you'd like.


From error.txt:

double free or corruption (out)

Okay, so something is failing pretty catastrophically. My guess is that the code for finalising the exemplar trajectories is making an assumption of having at least one streamline intersecting each node, whereas from the aforementioned warning from reading the assignments file, it seems that there is a disconnected node. This will actually require rectification in MRtrix3 code, not here.

If you are able to send me the relevant data for reproducing the connectome2tck crash, that would make my life much easier .Chances are there was a memory / segmentation issue when you ran it on your local system as well, it just wasn't detected at the time, but I can run it in debug mode to isolate the fault.

dkp commented 5 years ago

Okay, I need to deface the T1 and it has been copied various places, so I'm going to rerun that way and produce you a nice package... -Dianne

dkp commented 5 years ago

Your package of data is here on Box (1.8 GB zipped): https://arizona.box.com/s/8iu18zcl7r0om5zv05be1ff4hy62s05e

As before I ran this to get the intial derivatives from the parcellation=none:

docker run -i --rm -v ${PWD}:/bids_dataset -v ${PWD}/derivatives:/outputs bids/mrtrix3_connectome /bids_dataset /outputs participant --participant_label 102 --parcellation none --output_verbosity 3 --debug

I started the desikan run from the derivatives dir:

docker run -i --rm -v ${PWD}:/bids_dataset -v ${PWD}/desikan:/outputs bids/mrtrix3_connectome /bids_dataset /outputs participant --participant_label 102 --parcellation desikan --preprocessed --output_verbosity 3 --debug 

The failure is consistent and repeatable. This is Docker 2.1.0.0: with 8 cpus, 32 GB RAM Running on Mac Pro 2013 with Mojave and 64 GB of ram. The scan data is from a Siemens Skyra 3T with Syngo MR VE11c software.

Let me know if I can provide any other info, if the box link fails, or perhaps I could break something else ; )

-Dianne 

Lestropie commented 5 years ago

Okay, it turns out I've actually already fixed this: https://github.com/MRtrix3/mrtrix3/issues/1581

The problem is that the fix at the MRtrix3 end is not yet incorporated into this app.

So what you could do to get this working without too much effort is:

The warning regarding the discordance between number of parcels in assignments file and parcellation image still applies (that warning was the result of the changes in https://github.com/MRtrix3/mrtrix3/issues/1581).

dkp commented 5 years ago

Thanks Robert! I'm not in a terrible hurry though....and I hate to generate one-off weird versions of the container if I don't have to. Any idea when you might have an official fix?

Thanks so much,

Dianne

araikes commented 5 years ago

@Lestropie,

As I have built a version of this container to allow for multi-session runs, if I wanted to incorporate the above fix (for exactly the reason that @dkp is doing it), do you know what the ID for the checkout would need to be in the Dockerfile?

Lestropie commented 5 years ago

It would need to be 6e3b94f or newer; though when revising such it's usually easiest to just grab whatever master is pointing to (currently 266a5b4), since that will contain all latest bug fixes on the most recent tag but not any behaviour-altering modifications (which for MRtrix3 are all accumulated on the dev branch and only get merged to master in conjunction with a tag update).

Alternatively if your change is backwards-compatible you could create a pull request and I could integrate it into a 0.4.3 tag?

araikes commented 5 years ago

I can definitely create a PR. It's definitely not the prettiest fix for having longitudinal data or preprocessed T1s from fMRIPrep, but it does work (as best I can tell). I still plan to work on a PyBIDS grabber and DataSink interface for writing derivatives but that's a bit out for me.

Lestropie commented 3 years ago

--preprocessed is no longer present in 0.5.0 (#62), as there are now separate preproc and participant analysis levels.