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

Parallelize affine registration #1612

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@RicciWoo
Copy link
Contributor

RicciWoo commented Aug 9, 2018

Parallelize affine registration using prange and local function:

  1. cythonized _gradient_* in vector_fields.pyx;
  2. typed and made nogil on _compute_pdfs_* in parzenhist.pyx;
  3. parallelized _gradient_3d using prange and local function.

To specify number of threads: give num_threads to MutualInformationMetric

metric = MutualInformationMetric(nbins, sampling_prop, num_threads=4)

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Aug 9, 2018

Codecov Report

Merging #1612 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1612      +/-   ##
==========================================
- Coverage   87.34%   87.32%   -0.02%     
==========================================
  Files         246      246              
  Lines       31811    31812       +1     
  Branches     3451     3451              
==========================================
- Hits        27785    27781       -4     
- Misses       3204     3207       +3     
- Partials      822      824       +2
Impacted Files Coverage Δ
dipy/align/imaffine.py 91.82% <100%> (+0.02%) ⬆️
dipy/reconst/forecast.py 90.15% <0%> (-2.08%) ⬇️
dipy/core/graph.py 73.8% <0%> (-1.2%) ⬇️

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 5a6aa5a...a780788. Read the comment docs.

@Borda
Copy link
Contributor

Borda left a comment

It is a nice code simplification

@@ -440,7 +440,7 @@ def transform_inverse(self, image, interp='linear', image_grid2world=None,

class MutualInformationMetric(object):

def __init__(self, nbins=32, sampling_proportion=None):
def __init__(self, nbins=32, sampling_proportion=None, num_threads=None):

This comment has been minimized.

@Borda

Borda Jan 17, 2019

Contributor

sklean rather use -1 as unlimited/default value so the type check from doc does not raise an issue

the buffer in which to store the flags indicating whether the sample
point lies inside (=1) or outside (=0) the image grid
num_threads : int
Number of threads. If None (default) then all available threads

This comment has been minimized.

@Borda

Borda Jan 17, 2019

Contributor

there is no None in the functions declaration but num_threads=0


for i in range(nrows):
for j in range(ncols):
inside[k, i, j] = 1

This comment has been minimized.

@Borda

Borda Jan 17, 2019

Contributor

there should be a check that k is really in the range of nslices as the k moved to be input parameter and not determined inside the function

int all_cores = openmp.omp_get_num_procs()
int threads_to_use = -1

if num_threads != 0:

This comment has been minimized.

@Borda

Borda Jan 17, 2019

Contributor

I would rather see if num_threads > 0:

double[:] img_spacing,
double[:, :] sample_points,
floating[:, :] out, int[:] inside):
int i, j, k, in_flag, p

This comment has been minimized.

@Borda

Borda Jan 17, 2019

Contributor

k seems to be unused

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment