-
Notifications
You must be signed in to change notification settings - Fork 28
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
[FIX] force_fmapless on top of self-correcting pepolar bold #465
base: master
Are you sure you want to change the base?
Conversation
Also, it seems there was no test for |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #465 +/- ##
=======================================
Coverage 83.75% 83.75%
=======================================
Files 30 30
Lines 2826 2826
Branches 367 367
=======================================
Hits 2367 2367
Misses 387 387
Partials 72 72 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
b48366a
to
dc6d2f7
Compare
Not sure how that will behave with multi-subject cases, seee #455 . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how I feel about this, now that I've implemented application of pre-computed fieldmaps. In practice, the way to discover the relevant fieldmaps was to re-run the SDCFlows fieldmap wrangling function, and then look for precomputed fieldmaps of the relevant IDs.
This will change that ID, making a hard break between pre- and post-change derivatives. One of the ostensible advantages to the fit-transform split was that we can run an improved transform stage on any previous derivatives, so long as the derivatives are queryable.
We would need to strategize an alternative method for finding the appropriate fieldmaps from both new and old derivatives to make this change.
I am not sure to fully grasp the challenges here, and I wiped a lot of my memory of why I submitted that PR.
|
When force_fmapless is used on top of self-correcting pepolar bold, the fmapless estimator will get it's name from B0FieldIdentifier, creating a duplicate.
For now I just created a test to replicate that behavior and triggering the error that this PR will try to fix.