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

RFC: New heuristics table #101

Closed
oesteban opened this issue Apr 21, 2020 · 3 comments · Fixed by #115
Closed

RFC: New heuristics table #101

oesteban opened this issue Apr 21, 2020 · 3 comments · Fixed by #115
Labels
effort: high Estimated high effort task impact: high Estimated high impact task
Milestone

Comments

@oesteban
Copy link
Member

oesteban commented Apr 21, 2020

In the context of nipreps/dmriprep#97, I'm trying to design a more generalizable set of heuristics when deciding the SDC estimation strategy. This is also related to nipreps/dmriprep#70.

This is likely to replace the fieldmap_wrangler eventually:

def fieldmap_wrangler(layout, target_image, use_syn=False, force_syn=False):
"""Query the BIDSLayout for fieldmaps, and arrange them for the orchestration workflow."""
from collections import defaultdict
fmap_bids = layout.get_fieldmap(target_image, return_list=True)
fieldmaps = defaultdict(list)
for fmap in fmap_bids:
if fmap['suffix'] == 'epi':
fieldmaps['epi'].append((fmap['epi'], layout.get_metadata(fmap['epi'])))
if fmap['suffix'] == 'fieldmap':
fieldmaps['fieldmap'].append({
'magnitude': [(fmap['magnitude'], layout.get_metadata(fmap['magnitude']))],
'fieldmap': [(fmap['fieldmap'], layout.get_metadata(fmap['fieldmap']))],
})
if fmap['suffix'] == 'phasediff':
fieldmaps['phasediff'].append({
'magnitude': [(fmap[k], layout.get_metadata(fmap[k]))
for k in sorted(fmap.keys()) if k.startswith('magnitude')],
'phases': [(fmap['phasediff'], layout.get_metadata(fmap['phasediff']))],
})
if fmap['suffix'] == 'phase':
fieldmaps['phasediff'].append({
'magnitude': [(fmap[k], layout.get_metadata(fmap[k]))
for k in sorted(fmap.keys()) if k.startswith('magnitude')],
'phases': [(fmap[k], layout.get_metadata(fmap[k]))
for k in sorted(fmap.keys()) if k.startswith('phase')],
})
if fieldmaps and force_syn:
# syn: True -> Run SyN in addition to fieldmap-based SDC
fieldmaps['syn'] = True
elif not fieldmaps and (force_syn or use_syn):
# syn: False -> Run SyN as only SDC
fieldmaps['syn'] = False
return fieldmaps

The implementation will probably take the shape of a workflow, with an IdentityInterface for an outputnode where fields are the names of the EPI targets, for later selection.

Each of these fields will contain a list of fielmaps (which will be written out to the derivatives folder following the proposal in #26, with the adaptation of #26 (comment)), in the order and estimated as prescribed in the following table:

This heuristic table is to be applied per participant.

  • In the case of DWI, a target can be any dwi dataset under dwi/.
  • In the case of BOLD, a target will be first any sbref and then any bold in the absence of sbref with IntendedFor.
Priority Data Behavior
0 2+ EPIs with different PE but common IntendedFor under fmap/ i) Estimate fieldmap (pepolar); ii) Apply fieldmap to IntendedFor targets
1 1+ EPIs with common PE and IntendedFor under fmap/ with 1+ target in IntendedFor with different PE. i) Estimate fieldmap (pepolar), using IntendedFor targets as alternative PEs; ii) Apply fieldmap to IntendedFor targets
2 2+ EPIs with different PE but no IntendedFor under fmap/ Expand IntendedFor to all targets and apply 0?
3 1+ EPIs with common PE and no IntendedFor under fmap/ with 1+ target with different PE. Expand IntendedFor to all targets and apply 1?
4 Targets have several different PEs i) Hierarchically find most granular matches for estimation (i.e., first dir-, then run-, then acq-, etc. and their combinations); ii) Expand a IntendedFor to corresponding matches
5 Fieldmap images with IntendedFor. Priority of fieldmaps: _fieldmap, _phasediff, _phase{1,2}. Apply fieldmap regularization and conversion to Hz workflow
6 No data under fmap/ and targets do not have several PEs Fieldmap-less solutions

This is to be discussed in today's dMRIPrep meeting (cc/ @nipreps/dmriprep). Requesting for comments from @mattcieslak, @effigies, @satra, @mgxd.

@oesteban oesteban added impact: high Estimated high impact task effort: high Estimated high effort task labels Apr 21, 2020
@oesteban oesteban added this to the 1.3.0 milestone Apr 21, 2020
@mattcieslak
Copy link
Collaborator

One thing that will be important is picking which images from the epi fieldmaps, whether from fmaps/ or dwi/, to use. Siemens scanners add b>0 images to these, so it would be great to extend BIDS to allow for bval and bvec files in fmaps/ to prevent b>0 images from being used for distortion correction. It has also been helpful to use b=0 images from multiple timepoints in the scan if the fieldmaps are coming from long dwi scans.

Otherwise this looks great

oesteban added a commit to oesteban/dmriprep that referenced this issue Apr 22, 2020
Adds a new subworkflow based on FSL TOPUP to integrate SD estimation
for the ds001771 dataset.

- [x] Pin niworkflows to current master (while I release 1.2.0rc5
  containing nipreps/niworkflows#503, nipreps/niworkflows#504, which
  are used here).
- [x] Create a new sdc estimation workflow, with the expectation of
  upstreaming it to SDCFlows.
- [x] Implement the barebones of how nipreps/sdcflows#101 could look
  like. Also to be upstreamed to SDCFlows when mature.
- [x] Stick TOPUP from FSL 6.0.3 in the Docker image, since topup from
  FSL 5.0.x is really unstable (for instance, it fails with a
  segmentation fault on the workflow of ds001771)

Resolves: nipreps#92
@oesteban
Copy link
Member Author

Siemens scanners add b>0 images to these, so it would be great to extend BIDS to allow for bval and bvec files in fmaps/ to prevent b>0 images from being used for distortion correction.

I've given this some thought and realized this would be really easy to encode with valid BIDS: just dump those bvecs and bvals in the sidecar JSON.

I'll try to resurrect a bunch of stalled PRs and figure out whether a new PR would be appropriate. But this should be more explicit in BIDS (and that way, for instance, the validator can be instructed to check that the number of b-vecs/vals in the sidecar matches the size of the 4th dimension).

That also made me think about the RASB PR. We are already including tables in the metadata (e.g. the SliceTiming key) - what is it different in the case of the gradient table? wouldn't it be much more compact and easy to check by the validator if it were in the sidecar?

That said, in my first implementation of nipreps/dmriprep#97, I'm using b0s extracted from two datasets under dwi/, so those it is more of an implementation detail rather than a hole in the spec.

oesteban added a commit to oesteban/dmriprep that referenced this issue May 5, 2020
Adds a new subworkflow based on FSL TOPUP to integrate SD estimation
for the ds001771 dataset.

- [x] Pin niworkflows to current master (while I release 1.2.0rc5
  containing nipreps/niworkflows#503, nipreps/niworkflows#504, which
  are used here).
- [x] Create a new sdc estimation workflow, with the expectation of
  upstreaming it to SDCFlows.
- [x] Implement the barebones of how nipreps/sdcflows#101 could look
  like. Also to be upstreamed to SDCFlows when mature.
- [x] Stick TOPUP from FSL 6.0.3 in the Docker image, since topup from
  FSL 5.0.x is really unstable (for instance, it fails with a
  segmentation fault on the workflow of ds001771)

Resolves: nipreps#92
@oesteban oesteban pinned this issue May 7, 2020
oesteban added a commit to oesteban/dmriprep that referenced this issue Jun 2, 2020
Adds a new subworkflow based on FSL TOPUP to integrate SD estimation
for the ds001771 dataset.

- [x] Pin niworkflows to current master (while I release 1.2.0rc5
  containing nipreps/niworkflows#503, nipreps/niworkflows#504, which
  are used here).
- [x] Create a new sdc estimation workflow, with the expectation of
  upstreaming it to SDCFlows.
- [x] Implement the barebones of how nipreps/sdcflows#101 could look
  like. Also to be upstreamed to SDCFlows when mature.
- [x] Stick TOPUP from FSL 6.0.3 in the Docker image, since topup from
  FSL 5.0.x is really unstable (for instance, it fails with a
  segmentation fault on the workflow of ds001771)

Resolves: nipreps#92
oesteban added a commit to oesteban/dmriprep that referenced this issue Jun 2, 2020
Adds a new subworkflow based on FSL TOPUP to integrate SD estimation
for the ds001771 dataset.

- [x] Pin niworkflows to current master (while I release 1.2.0rc5
  containing nipreps/niworkflows#503, nipreps/niworkflows#504, which
  are used here).
- [x] Create a new sdc estimation workflow, with the expectation of
  upstreaming it to SDCFlows.
- [x] Implement the barebones of how nipreps/sdcflows#101 could look
  like. Also to be upstreamed to SDCFlows when mature.
- [x] Stick TOPUP from FSL 6.0.3 in the Docker image, since topup from
  FSL 5.0.x is really unstable (for instance, it fails with a
  segmentation fault on the workflow of ds001771)

Resolves: nipreps#92
oesteban added a commit to oesteban/dmriprep that referenced this issue Jun 2, 2020
Adds a new subworkflow based on FSL TOPUP to integrate SD estimation
for the ds001771 dataset.

- [x] Pin niworkflows to current master (while I release 1.2.0rc5
  containing nipreps/niworkflows#503, nipreps/niworkflows#504, which
  are used here).
- [x] Create a new sdc estimation workflow, with the expectation of
  upstreaming it to SDCFlows.
- [x] Implement the barebones of how nipreps/sdcflows#101 could look
  like. Also to be upstreamed to SDCFlows when mature.
- [x] Stick TOPUP from FSL 6.0.3 in the Docker image, since topup from
  FSL 5.0.x is really unstable (for instance, it fails with a
  segmentation fault on the workflow of ds001771)

Resolves: nipreps#92
oesteban added a commit to oesteban/dmriprep that referenced this issue Jun 2, 2020
Adds a new subworkflow based on FSL TOPUP to integrate SD estimation
for the ds001771 dataset.

- [x] Pin niworkflows to current master (while I release 1.2.0rc5
  containing nipreps/niworkflows#503, nipreps/niworkflows#504, which
  are used here).
- [x] Create a new sdc estimation workflow, with the expectation of
  upstreaming it to SDCFlows.
- [x] Implement the barebones of how nipreps/sdcflows#101 could look
  like. Also to be upstreamed to SDCFlows when mature.
- [x] Stick TOPUP from FSL 6.0.3 in the Docker image, since topup from
  FSL 5.0.x is really unstable (for instance, it fails with a
  segmentation fault on the workflow of ds001771)

Resolves: nipreps#92
oesteban added a commit to oesteban/dmriprep that referenced this issue Jun 2, 2020
Adds a new subworkflow based on FSL TOPUP to integrate SD estimation
for the ds001771 dataset.

- [x] Pin niworkflows to current master (while I release 1.2.0rc5
  containing nipreps/niworkflows#503, nipreps/niworkflows#504, which
  are used here).
- [x] Create a new sdc estimation workflow, with the expectation of
  upstreaming it to SDCFlows.
- [x] Implement the barebones of how nipreps/sdcflows#101 could look
  like. Also to be upstreamed to SDCFlows when mature.
- [x] Stick TOPUP from FSL 6.0.3 in the Docker image, since topup from
  FSL 5.0.x is really unstable (for instance, it fails with a
  segmentation fault on the workflow of ds001771)

Resolves: nipreps#92
@oesteban oesteban modified the milestones: 1.4.0, 1.5.0 Jun 12, 2020
oesteban added a commit to oesteban/sdcflows that referenced this issue Oct 23, 2020
This PR attempts to provide a more reliable framework to build
fieldmap estimation workflows.
Implicitly, it will help addressing issues regarding data conformity
(e.g., nipreps#63, nipreps#64, nipreps#65) and also ease larger refactors such as #20, nipreps#21,
 nipreps#26, and nipreps#101.
oesteban added a commit to oesteban/sdcflows that referenced this issue Oct 23, 2020
This PR attempts to provide a more reliable framework to build
fieldmap estimation workflows.
Implicitly, it will help addressing issues regarding data conformity
(e.g., nipreps#63, nipreps#64, nipreps#65) and also ease larger refactors such as #20, nipreps#21,
 nipreps#26, and nipreps#101.
oesteban added a commit to oesteban/sdcflows that referenced this issue Oct 26, 2020
This PR attempts to provide a more reliable framework to build
fieldmap estimation workflows.
Implicitly, it will help addressing issues regarding data conformity
(e.g., nipreps#63, nipreps#64, nipreps#65) and also ease larger refactors such as #20, nipreps#21,
 nipreps#26, and nipreps#101.
@oesteban oesteban linked a pull request Nov 12, 2020 that will close this issue
@oesteban
Copy link
Member Author

When bids-standard/bids-specification#622 goes in this heuristic table will be much easier, and should be defined by the workflows using sdcflows, so not a responsibility of this package anymore. Closing.

@oesteban oesteban unpinned this issue Nov 12, 2020
oesteban added a commit to oesteban/sdcflows that referenced this issue Nov 14, 2020
This PR attempts to provide a more reliable framework to build
fieldmap estimation workflows.
Implicitly, it will help addressing issues regarding data conformity
(e.g., nipreps#63, nipreps#64, nipreps#65) and also ease larger refactors such as #20, nipreps#21,
 nipreps#26, and nipreps#101.
oesteban added a commit to oesteban/sdcflows that referenced this issue Nov 18, 2020
This PR attempts to provide a more reliable framework to build
fieldmap estimation workflows.
Implicitly, it will help addressing issues regarding data conformity
(e.g., nipreps#63, nipreps#64, nipreps#65) and also ease larger refactors such as #20, nipreps#21,
 nipreps#26, and nipreps#101.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: high Estimated high effort task impact: high Estimated high impact task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants