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

Collect and apply field maps #79

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Collect and apply field maps #79

wants to merge 14 commits into from

Conversation

tsalo
Copy link
Collaborator

@tsalo tsalo commented Sep 25, 2024

Closes #63. Once this is working, I'll copy over the changes to fmripost_template.

Changes proposed in this pull request

  • Collect field maps from the fMRIPrep derivatives.
  • Warp the preprocessed field map to BOLD reference space.
  • Add a new version of the BOLD volumetric transform workflow that uses the preprocessed field map directly instead of hte coefficient and reference files.

Copy link

codecov bot commented Sep 25, 2024

Codecov Report

Attention: Patch coverage is 24.13793% with 44 lines in your changes missing coverage. Please review.

Project coverage is 30.56%. Comparing base (999b3f5) to head (1205e86).

Files with missing lines Patch % Lines
src/fmripost_aroma/workflows/base.py 0.00% 22 Missing ⚠️
src/fmripost_aroma/workflows/resample.py 27.58% 21 Missing ⚠️
src/fmripost_aroma/utils/bids.py 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #79      +/-   ##
==========================================
- Coverage   30.59%   30.56%   -0.04%     
==========================================
  Files          26       27       +1     
  Lines        2363     2405      +42     
  Branches      286      291       +5     
==========================================
+ Hits          723      735      +12     
- Misses       1617     1646      +29     
- Partials       23       24       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tsalo tsalo marked this pull request as ready for review January 6, 2025 15:16
@tsalo tsalo requested a review from effigies January 6, 2025 15:16
src/fmripost_aroma/workflows/resample.py Outdated Show resolved Hide resolved
Comment on lines 489 to 494
# Warp desc-preproc_fieldmap to boldref space
# Warp the mask as well
fieldmap_to_boldref = pe.Node(
ApplyTransforms(),
name='fieldmap_to_boldref',
)
Copy link
Member

Choose a reason for hiding this comment

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

This node needs to go into the bold_MNI6_wf because you want to resample from fmap to the MNI space.

Comment on lines 153 to 170
distortion_params = pe.Node(
DistortionParameters(metadata=metadata),
name='distortion_params',
run_without_submitting=True,
)
workflow.connect([
(inputnode, fmap_select, [
('fmap', 'fmap_ref'),
('fmap_id', 'keys'),
]),
(inputnode, distortion_params, [('bold_file', 'in_file')]),
# Inject fieldmap correction into resample node
(fmap_select, resample, [('fmap', 'fieldmap')]),
(distortion_params, resample, [
('readout_time', 'ro_time'),
('pe_direction', 'pe_dir'),
]),
]) # fmt:skip
Copy link
Member

Choose a reason for hiding this comment

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

Here is the section where you need to resample the fieldmap from fmap space to target space, using the inverse boldref2fmap transform, in addition to the boldref2anat and anat2MNI transforms. Then you pass in the fieldmap in MNI space to resample.

Copy link
Member

@effigies effigies left a comment

Choose a reason for hiding this comment

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

Pushed what I was thinking for resample.py. Feel free to revert.

src/fmripost_aroma/workflows/base.py Outdated Show resolved Hide resolved
@tsalo
Copy link
Collaborator Author

tsalo commented Jan 15, 2025

@effigies thanks for intervening directly on this. I started addressing your review last weekend but got drawn into other things. I'm hoping to test out the changes here soon.

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.

Field maps not being collected or applied
2 participants