Mackenzie/include alternate modalities

[mackenziesnyder]

Completed:

• added the t2w modality as an option for autoafids
• this includes the synthSR program to convert t2w nifti files to t1w nifti files

Still To do:

• verify correct test data is used - SynthSR does not produce a high quality t1w image from the t2w image upon initial visual inspection of the files

[Dhananjhay]

@mackenziesnyder Can you please update the DAG in the README with the new rule synthsr, thanks!

[mackenziesnyder]

@mackenziesnyder Can you please update the DAG in the README with the new rule synthsr, thanks!

@Dhananjhay I added the DAG in the latest commit

[Dhananjhay]

Perfect, looks great!

Still To do:

verify correct test data is used - SynthSR does not produce a high quality t1w image from the t2w image upon initial visual inspection of the files

Once you are done with this remaining task I think we'd be in a good shape to merge the branch. @ataha24 What do you think?

[ataha24]

I would say either t1w or alternative (SynthSR conversion). This is mainly because SynthSR would support any MRI not just t2w. What do you guys think? Open to finding a better name too!

[mackenziesnyder]

Ok I think 'alternative' makes the most sense - I think I have to do a few other changes because in rule SynthSR I hard coded the suffix to look for t2w since I previously thought we needed a template for each and we only had t1w and t2w. Is this something you want implemented as well @ataha24? I will just have to add these to the config - what would the suffixes for the other modalities be?

[ataha24]

Here are some related qs that arise that we can think about together. Probably best to dicuss more on a quick call!

• What should the behaviour for this rule be when there are multiple files BUT no T1w image (e.g., T2w, Flair, or even CT)?
• What if the dataset is heterogenous (i.e., some subjects have T1w others have T2w). Is that something we want to handle or even possible to handle?

I am taking a look at some of the SynthSR outputs and I think they are indeed in the same "space" but there are some local mophological differences.

[ataha24]

Perfect, looks great!

Still To do:
verify correct test data is used - SynthSR does not produce a high quality t1w image from the t2w image upon initial visual inspection of the files

Once you are done with this remaining task I think we'd be in a good shape to merge the branch. @ataha24 What do you think?

I am happy with this so far as it is the best we can do to support other modelities. Ok to merge this, just left a small comment regarding how to invoke SynthSR during a run

[Dhananjhay]

I'm getting an error when I try running autoafids on T2w modality

Command:

autoafids data/ test-data participant --participant-label 719222 --cores all --modality T2w

Error log:

Config file config/snakebids.yml is extended by additional config specified via the command line.
..... fast preprocessing configration .....
 the folowing squence will be triggered ['n4_bias_correction', 'norm_im', 'resample_im']
KeyError in file /local/scratch/autoafids/autoafids/workflow/rules/cnn.smk, line 29:
'T1w'
  File "/local/scratch/autoafids/autoafids/workflow/Snakefile", line 267, in <module>
  File "/local/scratch/autoafids/autoafids/workflow/rules/cnn.smk", line 29, in <module>
  File "/local/scratch/autoafids/.venv/lib/python3.10/site-packages/snakebids/core/datasets.py", line 627, in __getitem__

It might be because the pipeline is bypassing running synthsr rule first before looking for T1w image.

[mackenziesnyder]

I'm getting an error when I try running autoafids on T2w modality

Command:

autoafids data/ test-data participant --participant-label 719222 --cores all --modality T2w

Error log:

Config file config/snakebids.yml is extended by additional config specified via the command line.
..... fast preprocessing configration .....
 the folowing squence will be triggered ['n4_bias_correction', 'norm_im', 'resample_im']
KeyError in file /local/scratch/autoafids/autoafids/workflow/rules/cnn.smk, line 29:
'T1w'
  File "/local/scratch/autoafids/autoafids/workflow/Snakefile", line 267, in <module>
  File "/local/scratch/autoafids/autoafids/workflow/rules/cnn.smk", line 29, in <module>
  File "/local/scratch/autoafids/.venv/lib/python3.10/site-packages/snakebids/core/datasets.py", line 627, in __getitem__

It might be because the pipeline is bypassing running synthsr rule first before looking for T1w image.

Ah this is because I specified to only load the pybids_inputs of the user specified modality - just going to change 'T1w' to config['modality'] in cnn.smk and I believe that should fix this. It works for dry runs so I'm just gonna test and then push!

[mackenziesnyder]

• I added functionality for T2w and FLAIR as per synthsr compatibility.
• I also added a warning message for when t2w or flair are chosen - indicates to the user that the predictions are only as good as sythsr's conversion from those modalities to a t1w image
• I've tested it on my end - if you guys want to take a look that would be great! I believe if all is good it should be ready to merge

[ataha24]

I tested --modality T2w on 20 MRIs (using --cores all). It ran perfectly fine for some subjects but failed on others because of memory. This may be something we should control at the synthsr rule level? I think its hard for the workflow to interpret how much memory is needed when parallelizing across subjects because of calling singularity so it is probably trying to fit as many subjects as there are cpus then later running out of memory. Below is image to help with that:

Otherwise, the workflow ran perfectly. One other minor comment is re how synthSR floods the terminal with outputs. Is there a nice way to gently suppressing that to make it user friendly to parse? Makes sense if that output is dumped into some log file (it may be already) but it may be too much to have everything be on terminal. What do you guys think?

Summary:
Issue A: need to allocate some memory for the rule so it does not overuse memory.
Enhancement A: limit terminal outputs so user is not overwhelmed with outputs.

Big picture questions:

1. are we using the appropriate synthSR model?
2. can sythSR support ct images and if so, could be cool feature to add later

[mackenziesnyder]

I tested --modality T2w on 20 MRIs (using --cores all). It ran perfectly fine for some subjects but failed on others because of memory. This may be something we should control at the synthsr rule level? I think its hard for the workflow to interpret how much memory is needed when parallelizing across subjects because of calling singularity so it is probably trying to fit as many subjects as there are cpus then later running out of memory. Below is image to help with that:

Otherwise, the workflow ran perfectly. One other minor comment is re how synthSR floods the terminal with outputs. Is there a nice way to gently suppressing that to make it user friendly to parse? Makes sense if that output is dumped into some log file (it may be already) but it may be too much to have everything be on terminal. What do you guys think? Summary: Issue A: need to allocate some memory for the rule so it does not overuse memory. Enhancement A: limit terminal outputs so user is not overwhelmed with outputs.

Big picture questions:

1. are we using the appropriate synthSR model?
2. can sythSR support ct images and if so, could be cool feature to add later

Hi @ataha24 , I will try and test the workflow on a larger dataset to see if I can reproduce the error. I'll also look into putting the terminal output in a log file.

to answer the big picture questions:

1. I just dockerized the code here: https://github.com/BBillot/SynthSR and am running it with the command it gave in the readme - so I think so? Please correct me if this is incorrect.
2. It has ct built into it, we just need to give it the --ct flag for the shell command. We could specify an autoafids flag to correspond to the --ct flag for synthsr so autoafids can be run on ct images - I think that might be good to include as its own pr tho!

[mackenziesnyder]

@ataha24 I have done a lot of testing today - Synthsr uses a TON of memory when it runs (over 59% of my memory at its highest when only running one subject) so I believe it would have significant issues when trying to paralyze it. I can specify in the rule that it needs to be run independently and cannot be paralyzed through resource allocations. I tried memory allocation for the rule so it doesn't overuse memory, but it was still consistently crashing unfortunately. I used cbs basic and it would crash pretty much immediately when trying to run more than 1 subject at a time - I'm not sure if you were running it on cbs heavy previously to have some subjects run ok.