ME-ICA / mapca

A Python implementation of the moving average principal components analysis methods from GIFT
GNU General Public License v2.0
6 stars 8 forks source link

Use 3D fftconvolve instead of slice-wise 2D fftconvolve #15

Closed notZaki closed 3 years ago

notZaki commented 3 years ago

This PR replaces the multiple slice-wise 2D fftconvolve with a single 3D fftconvolve to make the code simpler. The original implementation and the PR produce a nearly identical array.

codecov-io commented 3 years ago

Codecov Report

Merging #15 (87527e8) into main (67ad715) will decrease coverage by 0.16%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #15      +/-   ##
==========================================
- Coverage   92.09%   91.92%   -0.17%     
==========================================
  Files           3        3              
  Lines         291      285       -6     
==========================================
- Hits          268      262       -6     
  Misses         23       23              
Impacted Files Coverage Δ
mapca/utils.py 98.54% <100.00%> (-0.07%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 67ad715...87527e8. Read the comment docs.

eurunuela commented 3 years ago

I would like to add an integration test before we merge PRs like this one. I was thinking of saving the results before and after maPCA in tedana and using those.

eurunuela commented 3 years ago

@tsalo feel free to merge this PR if you're happy with the changes.