Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dwipreproc: Compatibility with FSL6.0.0 #1509

Merged
merged 2 commits into from Dec 12, 2018

Conversation

Projects
None yet
2 participants
@Lestropie
Copy link
Member

Lestropie commented Dec 8, 2018

My guess is that somewhere in the topup code, when provided with the --fout option to output the estimated field, it generates a deep copy of the input b=0 series, and then overwrites the contents of the first volume with the estimated field. However because they erroneously don't reduce the template image to 3 axes when generating said copy, the output "field map" image contains copies of all but the first input b=0 volume.

Would expect to be fixed reasonably quickly at their end, but need to catch and handle here nonetheless.

Closes #1493.

dwipreproc: Compatibility with FSL6.0.0
In this particular version of FSL, the image output by the --fout option of the topup command generates a 4D image, where the first volume is the expected field map image, but the other volumes are intensity-scaled versions of the uncorrected input images. While this is certainly an FSL bug, the dwipreproc script nevertheless needs to be compatible with such data. Currently it causes an issue only when explicit DWI volume recombination is to be performed. This change detects the erroneous topup output, and extracts just the first volume.
Closes #1493.

@Lestropie Lestropie self-assigned this Dec 8, 2018

@Lestropie Lestropie requested a review from bjeurissen Dec 8, 2018

@Lestropie

This comment has been minimized.

Copy link
Member Author

Lestropie commented Dec 9, 2018

TravisCI tests currently failing due to BitBucket having disabled TLS1.0 for security reasons, and we're using it to pull Eigen. However the versions of Python installed on TravisCI should be new enough to have the newer versions of TLS available. So not sure if something remains internally misconfigured at TravisCI's end, or what's going on.

@jdtournier

This comment has been minimized.

Copy link
Member

jdtournier commented Dec 11, 2018

Ok, might be time to use Eigen's new official git repo and ditch mercurial...?

@jdtournier
Copy link
Member

jdtournier left a comment

Seems good to me. Note the fix for TravisCI, now using their official git repo rather than Mercurial. Seems to work - feel free to merge if happy with that.

@Lestropie Lestropie merged commit e890d83 into master Dec 12, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Lestropie Lestropie deleted the dwipreproc_fsl6_volcombine_fix branch Dec 12, 2018

@Lestropie Lestropie referenced this pull request Dec 12, 2018

Merged

Hotfix to dev #1512

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.