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

ENH: Allow unwarp to apply only to references #385

Closed
wants to merge 5 commits into from

Conversation

effigies
Copy link
Member

@effigies effigies commented Aug 4, 2023

This PR adds a ref_only mode to init_unwarp_wf that allows the same workflow to be used only on reference volumes.

This also changes the output reference volume to be derived from the input reference volume, rather than the average of the corrected time series.

@effigies effigies requested a review from oesteban August 4, 2023 19:17
@effigies
Copy link
Member Author

effigies commented Aug 4, 2023

This is intended to reduce the footprint of the EPI fit workflow. The corrected reference image will be used for T1w coregistration.

@effigies effigies enabled auto-merge August 4, 2023 19:18
@effigies effigies disabled auto-merge August 4, 2023 19:18
@codecov
Copy link

codecov bot commented Aug 4, 2023

Codecov Report

Patch coverage: 85.71% and project coverage change: -0.03% ⚠️

Comparison is base (375dd7a) 81.99% compared to head (661ca8f) 81.97%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #385      +/-   ##
==========================================
- Coverage   81.99%   81.97%   -0.03%     
==========================================
  Files          26       26              
  Lines        2271     2274       +3     
  Branches      281      282       +1     
==========================================
+ Hits         1862     1864       +2     
  Misses        362      362              
- Partials       47       48       +1     
Flag Coverage Δ
unittests ∅ <ø> (∅)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
sdcflows/workflows/apply/correction.py 81.81% <85.71%> (-1.52%) ⬇️

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

oesteban
oesteban previously approved these changes Aug 7, 2023
Copy link
Member

@oesteban oesteban left a comment

Choose a reason for hiding this comment

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

This changes the idea that a new reference is generated with the unwarped data, but that is probably is good overall, as the programmer will be required to generate the new reference explicitly with the undistorted data, if they don't want to directly use the unwarped reference.

Looks great to me :)

@oesteban
Copy link
Member

oesteban commented Aug 7, 2023

I came back to check one thing - the distorted_ref should be a realigned file if it has more than 1 timepoints (I am currently working with sbrefs having 4 volumes)

@effigies
Copy link
Member Author

effigies commented Aug 7, 2023

Sorry, you mean distorted_ref should be averaged, in case it has multiple time points? I thought by the time we got to this, sbrefs had already been merged to a single volume.

@oesteban
Copy link
Member

oesteban commented Aug 7, 2023

Sorry, you mean distorted_ref should be averaged, in case it has multiple time points? I thought by the time we got to this, sbrefs had already been merged to a single volume.

If that is the expectation, we should make it very clear that it has been averaged. However, resample would handle that just fine, allowing less data munging before correcting.

@effigies
Copy link
Member Author

Closing in favor of #346. We will probably want to make averaging and brain extraction optional in a future PR, to avoid duplicating effort when only a timeseries is needed.

@effigies effigies closed this Aug 16, 2023
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.

2 participants