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

Automatically use available fieldmaps if IntendedFor is missing #2312

Open
celstark opened this issue Oct 20, 2017 · 13 comments
Open

Automatically use available fieldmaps if IntendedFor is missing #2312

celstark opened this issue Oct 20, 2017 · 13 comments

Comments

@celstark
Copy link

The BIDS spec lists that the IntendedFor field is optional:
"The IntendedFor field is optional and in case the fieldmaps do not correspond to any particular scans it does not have to be filled."

Whenever I don't include it in the fmap's .json files, I get "No fieldmaps found or they were ignored". It would seem as if any found fmap's should, by default, be applied to anything in the func folder. Or, am I misreading that in the spec?

Craig

@chrisgorgo
Copy link
Contributor

The spec is not clear how fieldmaps should exactly be used for processing since there are many opinions what is the best way to do this. It's up to the software to decide what to do when IntendedFor field is missing.

Having said that I agree with you that FMRIPREP should consider different default than skipping fieldmaps if IntendedFor is missing. The situation is tricky when the field is missing for some maps, but not others (or only some BOLD files are covered), but those cases are more likely issues with incomplete metadata. The scheme I described here: #774 (comment) might be best for FMRIPREP.

@oesteban
Copy link
Member

Hi Craig, that would be the other facet of the discussion in #747. As Chis mentioned, we will soon rethink the treatment of fieldmaps in FMRIPREP.

@chrisgorgo chrisgorgo changed the title IntendedFor is required, but should be optional for fmaps Automatically use available fieldmaps if IntendedFor is missing Jan 10, 2018
@oesteban oesteban transferred this issue from nipreps/fmriprep Nov 27, 2019
@oesteban
Copy link
Member

@mattcieslak
Copy link

Maybe it would make sense to only automatically assign fieldmaps to images if the shim settings match?

@oesteban
Copy link
Member

oesteban commented Nov 28, 2019

That's a good idea, but how would you check that from the BIDS input?

@mattcieslak
Copy link

Heudiconv/PyDicom both will write out a "ShimSetting" field with a series if values like

	"ShimSetting": [
		1716,
		-3891,
		-210,
		392,
		14,
		-78,
		161,
		4	],

In my experience, this gets configured at the scanner console. The shim settings used when acquiring the fieldmap are copied for the scans the fieldmap is intended to correct. I'm not sure if this is siemens specific, but it's been useful for sanity-checking the quality of distortion correction. Often the person scanning will forget to copy the shim settings from the fieldmap into their dwi or bold and the distortion correction is a bit off.

The other problem is that "ShimSetting" is not part of BIDS..

@debyee
Copy link

debyee commented Mar 25, 2020

Quick question about this thread, has there been any updates on this feature? I saw that it got moved to pipelines, but wanted to make sure there was something I wasn't missing since this thread was initiated 2 years ago

I'm currently trying to apply gradient echo field maps (I have 1 phasediff image and 2 magnitude images) that I'm trying to apply fieldmap correction to several functional runs, without the "IntendedFor" argument in the .json files for the fieldmaps.

I've been trying to follow along with the documentation in sdcflows (https://fmriprep.readthedocs.io/en/stable/sdc.html), and tried using the --force-syn argument, but for some reason, it's still not automatically detecting my fieldmaps. I looked at the function that @oesteban posted above with the "fieldmap_wrangler" function that seems to address this, but It's not working clearly for me. Fwiw, According to my logs, the bidsvalidator does detect the fieldmaps, so I know that they are detected. However, they don't seem to be applied.

Below is a snippet of code I used to call fmriprep. Please let me know if I'm posting this in the wrong thread -- since it seems that SDC seemed to have migrated to a different workflow.

#SBATCH -t 08:00:00
#SBATCH --mem=128GB
#SBATCH -N 1
#SBATCH -n 32
#SBATCH --account=carney-ashenhav-condo  
#SBATCH -J TCB-fmriprep-fieldmap
#SBATCH --output slurm-logs/TCB-fmriprep-log-%J.txt
#SBATCH --error slurm-logs/TCB-fmriprep-error-%J.txt

#--------- CONFIGURE THESE VARIABLES ---------

#bids_root_dir=/gpfs/data/ashenhav/mri-data     # based on oscar path
bids_root_dir=/gpfs/data/bnc/scratch
participant_labels=("tcb2004")
investigator=shenhav                            # investigator
study_label=201226                              # study label 
fmriprep_version=20.0.1                         # check /gpfs/data/bnc/simgs for the latest version
#nthreads=64 				                    # heuristic: 2x number of cores 

#--------- FMRIPREP ---------
singularity run --cleanenv                                              	\
	--bind ${bids_root_dir}/${investigator}/study-${study_label}:/data      \
	--bind /gpfs/scratch/dyee7:/scratch                                     \
	--bind /gpfs/data/bnc/licenses:/licenses                                \
	/gpfs/data/bnc/simgs/fmriprep-${fmriprep_version}.sif                   \
	/data/bids /data/bids/derivatives/fmriprep-${fmriprep_version}-nofs     \
	participant 															\
	--participant-label ${participant_labels} 								\
	--fs-license-file /licenses/freesurfer-license.txt 						\
	-w /scratch/fmriprep 													\
	--stop-on-first-crash                            						\
	--nthreads 64 															\
	--write-graph 															\
	--force-syn																\
	#--use-syn-sdc 															\
	#--fs-no-reconall   		

@effigies
Copy link
Member

No, fMRIPrep does not use fieldmaps that don't use IntendedFor at this point.

@oesteban
Copy link
Member

Yep, the problem is that the heuristics to assign fieldmaps and target data without the IntendedFor are quite complex and subject to opinion (e.g. the comments about shimming above).

This will take a while to implement if ever done. I only can see a push in the context of diffusion MRI, where the BIDS specs fall short in some cases. And even there, the most straightforward path would be to extend the possible uses of IntendedFor in BIDS.

@oesteban
Copy link
Member

After thinking this through, and having bids-standard/bids-specification#622 in the works, I think this issue can be moved back to fMRIPrep.

@oesteban oesteban transferred this issue from nipreps/sdcflows Oct 23, 2020
@yarikoptic
Copy link
Contributor

my 1c: I would just say "explicit better than implicit" and thus do not bother about some automagic pickup of fieldmaps and just close this issue as wontfix. If desired, use heudiconv's --command populate-fieldmaps to populate them based on your liking (match shim or just geometry?). May be validator (if it doesn't yet) should error out whenever some subj/sessions have IntendedFor and some don't to make present heterogeneity explicit as well.

@WillForan
Copy link

Would something like --guess-sdc [name|time] as an explicit option for fmriprep be welcome?
I can image there are some easy cases (--guess should error out if not easily determined?)

  • only one fmap and one bold
  • sorting and matching by json AcquisitionTime (fmap before bold, equal number of each) or
  • names fmap acq matches task name? fmap/*acq-rest* <=> func/*task-rest*

For completeness, here's heudiconv's intended-for docs

@effigies
Copy link
Member

effigies commented Feb 7, 2025

At this point, I have no intention of implementing this kind of logic in fMRIPrep. If we want to add something along these lines, it should:

  1. Have a well-defined logic that can be implemented on any BIDS dataset with any indexing tool. A document should clearly state this logic and get public feedback.
  2. Get implemented in PyBIDS (because that's what we currently use).
  3. Get a PR that gives users the ability to opt in to that behavior.

The approach that has felt cleanest to me is:

But if someone puts together a spec document for Heudiconv's approach and implements it in PyBIDS, I'd be willing to add those calls to fMRIPrep.


Apologies if the above comes off as grouchy. fMRIPrep development has slowed, and I think realistically this needs to live somewhere else in order to be maintainable.

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

8 participants