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

Draft MEDIC dynamic distortion correction method #435

Closed
wants to merge 26 commits into from

Conversation

tsalo
Copy link
Contributor

@tsalo tsalo commented Apr 7, 2024

Closes #36.

This currently runs on some test data, but I haven't evaluated the results.

Changes proposed:

  • Add sdcflows.workflows.fit.medic module with MEDIC workflow.

Still to do:

  • Figure out how to make EnforceTemporalConsistency more memory-efficient.
    • warpkit is much faster.
  • Compare SDCFlows results to MEDIC implementation in warpkit.
    • The current results look really bad compared to warpkit, but I'm confident I can pin down where they diverge.
  • Convert functions used as nodes into Nipype interfaces.
  • Add MEDIC option to the field map wrangler.
  • Documentation
  • Tests

Copy link

codecov bot commented Apr 7, 2024

Codecov Report

Attention: Patch coverage is 19.83871% with 497 lines in your changes are missing coverage. Please review.

Project coverage is 66.28%. Comparing base (73d717a) to head (ed0f46c).

Files Patch % Lines
sdcflows/utils/misc.py 7.14% 234 Missing ⚠️
sdcflows/interfaces/utils.py 24.51% 117 Missing ⚠️
sdcflows/workflows/fit/medic.py 12.62% 90 Missing ⚠️
sdcflows/interfaces/fmap.py 55.31% 42 Missing ⚠️
sdcflows/utils/phasemanip.py 6.66% 14 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master     #435       +/-   ##
===========================================
- Coverage   76.43%   66.28%   -10.16%     
===========================================
  Files          32       33        +1     
  Lines        2835     3455      +620     
  Branches      376      416       +40     
===========================================
+ Hits         2167     2290      +123     
- Misses        600     1097      +497     
  Partials       68       68               
Flag Coverage Δ
unittests ?

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

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

@tsalo
Copy link
Contributor Author

tsalo commented Apr 10, 2024

I still need to figure out why the results don't look good, but I was thinking an alternative might be for me to simply integrate warpkit as a dependency and run that directly. Would that be preferable to a translated Nipype workflow?

@mgxd
Copy link
Contributor

mgxd commented Apr 11, 2024

If using warpkit significantly improves performance, all the more reason to wrap it rather than reimplement - it'll come with the added benefit of reducing longterm maintenance 😄

One hurdle that would add is a more complex installation - however if/when vanandrew/warpkit#6 is resolved it would be a cinch

@tsalo
Copy link
Contributor Author

tsalo commented May 6, 2024

@mgxd I did what you proposed in #438, so I'll close this PR now.

@tsalo tsalo closed this May 6, 2024
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.

Support dynamic distortion correction with DOCMA
2 participants