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

RF/FIX: Prioritize sbref and shortest echo for SyN-SDC #364

Merged
merged 8 commits into from
Jun 8, 2023

Conversation

effigies
Copy link
Member

@effigies effigies commented Jun 7, 2023

This PR resolves the twin issues of either creating an estimator per echo or including all echos (with different contrast and dropout properties) in multi-echo SyN.

Because we want the shortest echo, the simplest way to do this was to sort all possible EPIs by suffix and echo time, to push the shortest echo sbref to the front of the line, or the shortest echo bold/dwi, failing that. We then set "IntendedFor" to the set of sbrefs/echos matching all other entities/path components. Keeping track of what files have been included in an "IntendedFor" avoids finding multiple estimators for a set.

New estimator results:

❯ sdcflows-find-estimators --subject 02 --fmapless . 
Estimation for </data/openneuro/ds000210> complete. Found:
	sub-02
		FieldmapEstimation(sources=<2 files>, method=<EstimatorType.ANAT: 5>, bids_id='auto_00000')
			None	anat/sub-02_T1w.nii.gz
			j	func/sub-02_task-cuedSGT_run-01_echo-1_bold.nii.gz
		FieldmapEstimation(sources=<2 files>, method=<EstimatorType.ANAT: 5>, bids_id='auto_00001')
			None	anat/sub-02_T1w.nii.gz
			j	func/sub-02_task-cuedSGT_run-02_echo-1_bold.nii.gz
		FieldmapEstimation(sources=<2 files>, method=<EstimatorType.ANAT: 5>, bids_id='auto_00002')
			None	anat/sub-02_T1w.nii.gz
			j	func/sub-02_task-cuedSGT_run-03_echo-1_bold.nii.gz
		FieldmapEstimation(sources=<2 files>, method=<EstimatorType.ANAT: 5>, bids_id='auto_00003')
			None	anat/sub-02_T1w.nii.gz
			j	func/sub-02_task-cuedSGT_run-04_echo-1_bold.nii.gz

Closes #363.

@effigies
Copy link
Member Author

effigies commented Jun 7, 2023

I should probably add similar tests for multiecho pepolar. Can do that in this or a separate PR.

@mgxd Sorry to be bugging you, but I'd really like to get fMRIPrep out this week if I can, and this is a blocker. If you have time, I'd appreciate a speedy review.

@effigies effigies added this to the 2.5.1 milestone Jun 7, 2023
@effigies effigies changed the title RF: Prioritize sbref and shortest echo for SyN-SDC RF/FIX: Prioritize sbref and shortest echo for SyN-SDC Jun 7, 2023
mgxd
mgxd previously approved these changes Jun 7, 2023
Copy link
Contributor

@mgxd mgxd left a comment

Choose a reason for hiding this comment

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

LGTM- this functionality seems self-contained enough to potentially be a separate function

sdcflows/utils/wrangler.py Show resolved Hide resolved
@effigies
Copy link
Member Author

effigies commented Jun 7, 2023

Not a massive refactor, but at least it encapsulated it. WDYT?

Copy link
Contributor

@mgxd mgxd left a comment

Choose a reason for hiding this comment

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

Looks clean

@effigies effigies enabled auto-merge June 7, 2023 21:45
@effigies effigies merged commit e84d838 into nipreps:master Jun 8, 2023
@effigies effigies deleted the fix/multiecho-syn branch June 8, 2023 01:28
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

Successfully merging this pull request may close these issues.

SyN-SDC includes multiple echos
2 participants