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

IntendedFor is ignored by wrangler.find_estimators #266

Open
bpinsard opened this issue Apr 9, 2022 · 9 comments
Open

IntendedFor is ignored by wrangler.find_estimators #266

bpinsard opened this issue Apr 9, 2022 · 9 comments

Comments

@bpinsard
Copy link
Contributor

bpinsard commented Apr 9, 2022

I am trying the new fMRIPrep release 21.0.1, and I have never ending topup on pilot multi-echo data.
Trying to investigate why, I found out that SDCFlow ignored the IntendedFor and topup is run on 20+ incongruent volumes (echoes with different distortions level from all possible epi fieldmap in the session).

Looking at the code, if I am not mistaken, it seems that the IntendedFor will never be used as the following code will take all EPI and return a single estimator per session.

for ses, acq, ce in product(sessions or (None,), acqs, contrasts):
entities = base_entities.copy()
entities.update(
{"suffix": "epi", "session": ses, "acquisition": acq, "ceagent": ce}
)
dirs = layout.get_directions(**entities)
if len(dirs) > 1:
e = fm.FieldmapEstimation(
[
fm.FieldmapFile(fmap.path, metadata=fmap.get_metadata())
for fmap in layout.get(direction=dirs, **entities)
]
)
estimators.append(e)

and then IntendedFor are skipped because the epi fmap is already used above.
if epi_fmap.path in fm._estimators.sources:
continue # skip EPI images already considered above

I would say that this issue is not multi-echo specific as it did not only aggregate echoes from the same series. The BIDS might be faulty as IntendedFor in multi-echo fMRI and phase-reversed EPI fmap is not detailled in the doc (I set it such that the each echo index _epi.json be IntendedFor the same echo index _bold.nii.gz).
I could make changes to use the new B0FieldIdentifiers BIDS tags, but it would be good if that worked on legacy BIDS datasets using IntendedFor.
Let me know if that's clear or if you need more info on the dataset.

@bpinsard
Copy link
Contributor Author

Looking at the wrangler code, I can see that some options that were working in the previous version might no longer possible.
For instance, running fmriprep on a dataset including fieldmaps with --use-syn-sdc to use Syn as fallback when fieldmaps are missing for some of the runs/sessions.

In addition, from the code logic it seems that if a dataset includes fieldmaps for DWI but not for BOLD, it might require using --force-syn to enable Syn on BOLD. As the wrangler is generic for both DWI/BOLD and fmriprep checks if any estimator has been returned, it could interact with the workflow setup.

https://github.com/nipreps/fmriprep/blob/6ba171f78cc4ece177b68e4953c83bc60c2c9071/fmriprep/workflows/base.py#L335-L341

@bpinsard
Copy link
Contributor Author

bpinsard commented Nov 4, 2022

Any potential fix for that issue?
I am also puzzle by the pybids query to find the fmaps tagged with B0FieldIdentifier here

b0_entities["B0FieldIdentifier"] = b0_id
e = fm.FieldmapEstimation([
fm.FieldmapFile(fmap.path, metadata=fmap.get_metadata())
for fmap in layout.get(**b0_entities)

as I cannot get any results with that query in a simple pybids example.

In [7]: layout.get_B0FieldIdentifiers()
['myb0id']
In [8]: layout.get(B0FieldIdentifier='myb0id')
[]
In [9]: fmap=layout.get(datatype='fmap',direction='AP',run=1, extension='.nii.gz',acquisition='sbref')[0]
In [11]: fmap.entities['B0FieldIdentifier']
Out[11]: ['myb0id']

Is querying files with metadata functional in pybids?
Thank you in advance for any pointer.

@bpinsard
Copy link
Contributor Author

bpinsard commented Nov 4, 2022

I think I solved a part of the puzzle about pybids not finding the B0FieldIdentifier tagged files.
If B0FieldIdentifier is a list in the json (which is allowed in the BIDS spec https://bids-specification.readthedocs.io/en/stable/04-modality-specific-files/01-magnetic-resonance-imaging-data.html#echo-planar-imaging-and-b0-mapping ), then querying with a single B0FieldIdentifier string will of course not find any file exactly matching the query, which is what happens in

b0_entities["B0FieldIdentifier"] = b0_id
e = fm.FieldmapEstimation([
fm.FieldmapFile(fmap.path, metadata=fmap.get_metadata())
for fmap in layout.get(**b0_entities)
.
If I query layout.get(B0FieldIdentifier=bids.layout.Query.ANY) I do get the fieldmaps where that value is not empty.
In fact it doesn't seem possible to query that an entity with list value contains a string from the BIDSLayout.get if I am not mistaken, so that won't be an easy fix.
@mgxd @oesteban what do you think?

@oesteban
Copy link
Member

oesteban commented Nov 9, 2022

I agree

layout.get(B0FieldIdentifier='myb0id')

should work out

@bpinsard
Copy link
Contributor Author

bpinsard commented Nov 9, 2022

You mean it should work with list, getting a match if any value in the list matches?

@bpinsard
Copy link
Contributor Author

bpinsard commented Nov 9, 2022

layout.get(B0FieldIdentifier='"myb0id"',regex_search=True) is able to find the files with the value contained in the list.

@bpinsard
Copy link
Contributor Author

I can't wrap my head around how to make B0Field* work with sdcflows for pepolar with a single phase-reversed epi fmap. It seems that sdcflows is making assumptions about the scheme that are not set in the BIDS specs (which I convene is quite loose). https://bids-specification.readthedocs.io/en/stable/04-modality-specific-files/01-magnetic-resonance-imaging-data.html#echo-planar-imaging-and-b0-mapping

From the spec, one could set B0Fields in different ways to achieve the same goal which is for each bold find a pair of epis (_bold|_sbref|_epi) with opposite phase encoding direction (pedir):

  • use a bold specific ID that is set as B0FieldSource for the bold, and is set as B0FieldIdentifier in the bold(or sbref) and in the opposite pedir acquisition (_sbref/_bold if stored in func/ or _epi if stored in fmap/). However, this doesn't work when multiple bold tasks uses the same opposite pedir for corrections, due to limitations in pybids (partly addressed here FIX: Make lists and dicts hashable bids-standard/pybids#684), and in sdcflows ( https://github.com/nipreps/sdcflows/blob/master/sdcflows/fieldmaps.py#L409 cannot deal with lists).
  • use a "fieldmap" (as _epi|bold|_sbref) specific id, and have the _bold list both pepolar "fieldmaps" as B0FieldSource. It seems the recommended scheme in (using a B0FieldIdentifier derived from "fieldmaps" series dicom tags) but sdcflows assumes that a single B0FieldIdentifier identifies both files used for pepolar (and that id is used to name the workflow), raising "Insufficient sources to estimate a fieldmap." ( find_estimators list unique B0FieldIdentifiers and then for each, runs a fieldmap workflow) .

The first way seems easier to fix.
Let me know if that makes sense and what do you think about that issue.
Thanks

@effigies
Copy link
Member

@bpinsard did this get fully resolved?

@bpinsard
Copy link
Contributor Author

I think the B0Field issues are resolved, but the problems with IntendedFor are not. I will try to find some time to have a small test case to replicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants