[RTM] RF: Assume phase-encoding direction is A-P unless specified L-R #740
Conversation
effigies
force-pushed
the
original_syn
branch
2 times, most recently
from
3ff6c3a to
ad2ec5d
Compare
|
To review this PR, separately review #741, and then look at effigies/fmriprep@fsfix_common...original_syn. |
effigies
force-pushed
the
original_syn
branch
4 times, most recently
from
7c29adf to
465f751
Compare
effigies
changed the title
fmriprep/interfaces/reports.py
Outdated
| @@ -33,6 +33,7 @@ | |||
|
|
|||
| FUNCTIONAL_TEMPLATE = """\t\t<h3 class="elem-title">Summary</h3> | |||
| \t\t<ul class="elem-desc"> | |||
| \t\t\t<li>Detected phase-encoding (PE) direction: {pedir}</li> | |||
There was a problem hiding this comment.
Remove "Detected"? since now it is the default AP or manually set
fmriprep/workflows/bold.py
Outdated
| bold_pe = None | ||
| else: | ||
| bold_pe = layout.get_metadata(bold_file).get("PhaseEncodingDirection") | ||
|
|
||
| restrict_i = [[1, 0, 0], [1, 0, 0]] |
There was a problem hiding this comment.
restrict = [[int(bold_pe[0] == 'i'), int(bold_pe[0] == 'j'), 0]] * 2And then use restrict_deformation=restrict down in L1371
|
One more thing: the sdc-syn method is one method to estimate the fieldmap. Could we move the code to its proper module under the fieldmap/ folder? |
|
Yeah, I can do that. Can do that here or in a separate PR. Up to you. |
|
Actually I'll go ahead an say: Let's do that in a separate PR. We can move a lot of logic that's currently in Will merge on tests passing. |
As discussed in #714, the simplest way to build in a prior of A-P phase-encoding is to always use A-P unless otherwise specified. When this produces a discernible error, the correct response would be to update the dataset metadata.
With this change, we can eliminate the cost function machinery from the BBR workflows, which should simplify #694, if this goes in first.
Closes #714.