dwipreproc: Compatibility with FSL6.0.0#1509
Merged
Conversation
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.
Member
Author
|
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. |
Member
|
Ok, might be time to use Eigen's new official git repo and ditch mercurial...? |
jdtournier
approved these changes
Dec 11, 2018
Member
jdtournier
left a comment
There was a problem hiding this 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.
Merged
Lestropie
added a commit
that referenced
this pull request
Jul 21, 2023
In #1509, a CI failure was encountered where on Mac ARM64 the shared library would not link, complaining about the absence of __set_fetch_function() for unsigned long datatype. It is believed that this is being caused by the presence of MR::Math::load_matrix() with size_t template type being used for loading pre-defined permutation matrices. To address this, the new typedef MR::Math::Stats::index_type is being introduced to refer to any item being indexed into an array or matrix. A 32-bit unsigned integer should be sufficient for all cases of such (the only possible exception is the calculation of the maximal number of shuffles, for which 64-bit unsigned integers are used, just for the sake of potentially calculating and reporting that number, even if it turns out to be much larger than the number of shuffles actually utilised).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
My guess is that somewhere in the
topupcode, when provided with the--foutoption 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.