Skip to content

Conversation

oesteban
Copy link
Member

A new interface is proposed, to be used in phase-difference based
workflows.

@Aksoo identified a potential issue with siemens2rads
(#30 (comment)),
although the re-centering did not seem to make a big difference.

This implementation of the rescaling uses percentiles to robustly
calculate the range of the input image (as recommended in the original
paper about FSL PRELUDE), and makes sure the output data type is
adequate (float32).

A new interface is proposed, to be used in phase-difference based
workflows.

@Aksoo identified a potential issue with siemens2rads
(nipreps#30 (comment)),
although the re-centering did not seem to make a big difference.

This implementation of the rescaling uses percentiles to robustly
calculate the range of the input image (as recommended in the original
paper about FSL PRELUDE), and makes sure the output data type is
adequate (float32).
@oesteban oesteban added the bug Something isn't working label Nov 18, 2019
@oesteban oesteban added this to the 1.0.0 milestone Nov 18, 2019
@oesteban oesteban requested a review from mattcieslak November 18, 2019 18:06
@oesteban
Copy link
Member Author

Okay, now I remember why we were not using the 0-6.28 range: it needs recentering (which is not the case for the 2 phases case).

I'll add a recentering node to this PR.

FSL PRELUDE will return some values unwrapped below zero, making it hard
to differenciate these pixels from the mask.
This commit addresses that particular issue.
@oesteban
Copy link
Member Author

oesteban added a commit to oesteban/sdcflows that referenced this pull request Nov 19, 2019
@kimsin98
Copy link
Contributor

kimsin98 commented Nov 19, 2019

Trimming wrapped phase images at 2nd and 98th percentiles seems odd, since the values outside that range are not really outliers in a wrapped image. They are the connecting values necessary for smooth unwrapping.

In the original PRELUDE paper, the percentile trimming is done on the magnitude image to derive a mask if no mask is specified by the user.

oesteban added a commit to oesteban/sdcflows that referenced this pull request Nov 19, 2019
@oesteban
Copy link
Member Author

Trimming wrapped phase images at 2nd and 98th percentiles seems odd, since the values outside that range are not really outliers in a wrapped image. They are the connecting values necessary for smooth unwrapping.

The intent of this was to deal correctly with phase images cast to float datatypes. Since it doesn't seem to solve any problem, and the particular change goes beyond the scope of this PR, I think you are right I should remove it 👍(done in the latest commit).

@oesteban oesteban merged commit be7058e into nipreps:master Nov 19, 2019
@oesteban oesteban deleted the fix/siemens2rads branch November 19, 2019 06:43
"""Convert the input phase difference map in arbitrary units (a.u.) to rads."""
from scipy.stats import mode
im = nb.load(in_file)
data = im.get_fdata(dtype='float32')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I'm thinking about this again, I wonder if it makes more sense to make it float64 for the calculation, given that it's only one volume. This would ensure that the rounding error of casting to float32 on write (the set_data_dtype below will still take care of that) dominates the arithmetic rounding errors.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels like splitting hairs... especially for images that originally were int16, DYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, probably. But it's cheap and reduces accumulated error. That accumulated error is almost certainly in the noise, but the extra guard bits also give you lots of room to have suboptimal order of operations without changing the output relative to an optimal (smallest induced error) algorithm, which feels useful for reproducibility/comparability with other tools.

This interface is equivalent to running the following steps:
#. Convert from rad to rad/s
(``niflow.nipype1.workflows.dmri.fsl.utils.rads2radsec``)
#. FUGUE execution: fsl.FUGUE(save_fmap=True)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#. FUGUE execution: fsl.FUGUE(save_fmap=True)
#. FUGUE execution: fsl.FUGUE(save_fmap=True).

oesteban added a commit that referenced this pull request Nov 23, 2019
oesteban added a commit to oesteban/sdcflows that referenced this pull request Nov 25, 2019
Putting together the lessons learned in nipreps#30, leveraging nipreps#52 and nipreps#53
(unfolded from nipreps#30 too), and utilizing nipreps#50 and nipreps#51, this workflow adds
the phase difference map calculation, considering it one use-case of the
general phase-difference fieldmap workflow.

On top of this PR, we can continue the discussions held in nipreps#30.
Probably, we will want to address nipreps#23 the first - the magnitude
segmentation is sometimes really bad (e.g. see the phase1/2 unit test).

Another discussion arisen in nipreps#30 is the spatial smoothing of the
fieldmap (nipreps#22).

Finally, the plan is to revise this implementation and determine whether
the subtraction should happen before or after PRELUDE, and whether the
arctan2 route is more interesting.
oesteban added a commit to oesteban/sdcflows that referenced this pull request Nov 25, 2019
Putting together the lessons learned in nipreps#30, leveraging nipreps#52 and nipreps#53
(unfolded from nipreps#30 too), and utilizing nipreps#50 and nipreps#51, this workflow adds
the phase difference map calculation, considering it one use-case of the
general phase-difference fieldmap workflow.

On top of this PR, we can continue the discussions held in nipreps#30.
Probably, we will want to address nipreps#23 the first - the magnitude
segmentation is sometimes really bad (e.g. see the phase1/2 unit test).

Another discussion arisen in nipreps#30 is the spatial smoothing of the
fieldmap (nipreps#22).

Finally, the plan is to revise this implementation and determine whether
the subtraction should happen before or after PRELUDE, and whether the
arctan2 route is more interesting.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants