search

nipreps / dmriprep

dMRIPrep is a robust and easy-to-use pipeline for preprocessing of diverse dMRI data. The transparent workflow dispenses of manual intervention, thereby ensuring the reproducibility of the results.
https://www.nipreps.org/dmriprep
Apache License 2.0
63 stars 24 forks source link

ENH: Connect fieldmap estimation to preprocessing pipeline of individual DWI runs #140

Closed josephmje closed 3 years ago

pep8speaks commented 3 years ago

Hello @josephmje, Thank you for updating!

Cheers! There are no style issues detected in this Pull Request. :beers: To test for issues locally, pip install flake8 and then run flake8 dmriprep.

Comment last updated at 2020-12-11 14:50:57 UTC
josephmje commented 3 years ago

@oesteban Thanks for the refactor to the base workflow. Reading it now and it looks a lot more intuitive.

oesteban commented 3 years ago

@josephmje can you make sure you have pushed anything you were working on and hold off of pushing while I finish the SDCFlows updates? I'll update this branch when SDCFlows is ready.

josephmje commented 3 years ago

@josephmje can you make sure you have pushed anything you were working on and hold off of pushing while I finish the SDCFlows updates? I'll update this branch when SDCFlows is ready.

Yup everything I've been working on has been pushed.

oesteban commented 3 years ago

Okay, if tests pass then I think we can go ahead and merge.

It still won't do anything with the fieldmaps because our test dataset with fieldmaps (ds001771) does not have IntendedFor and therefore, the registration and unwarping is disabled at https://github.com/nipreps/dmriprep/pull/140/files#diff-3f08760032797b48bfababab775f0a5f1852b0c33a7a0915122bc68d1bd52133R90

But I think that's a different issue (probably pertaining to SDCFlows, and namely the problem of what to do when no IntendedFor are given for a potential EPI target). And, as currently implemented, the behavior is similar to fMRIPrep's (which does nothing if IntendedFor is not set).

josephmje commented 3 years ago

Should we update the docstring for these workflows so that the fieldmap workflows get visualized? Currently, it refers to ds000206, which doesn't have any fieldmaps.

oesteban commented 3 years ago

Should we update the docstring for these workflows so that the fieldmap workflows get visualized? Currently, it refers to ds000206, which doesn't have any fieldmaps.

Good luck with that :p

It's going to be a rabbit hole of massive dimensions. Plus, SDCFlows has ~94% coverage. Let's trust that.