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

Add fit_tensor iteration decorator #766

Merged
merged 1 commit into from Nov 6, 2015

Conversation

Projects
None yet
2 participants
@dimrozakis
Contributor

dimrozakis commented Nov 6, 2015

Splits data into a number of chunks of specified size and iterates the
decorated fit_tensor function over them. This is useful to counteract
the temporary but significant memory usage increase in fit_tensor
functions that use vectorized operations and need to store large
temporary arrays for their vectorized operations.

Add fit_tensor iteration decorator
Splits data into a number of chunks of specified size and iterates the
decorated fit_tensor function over them. This is useful to counteract
the temporary but significant memory usage increase in fit_tensor
functions that use vectorized operations and need to store large
temporary arrays for their vectorized operations.
@dimrozakis

This comment has been minimized.

Contributor

dimrozakis commented Nov 6, 2015

Closes #763

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Nov 6, 2015

Thank you for this! :)

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Nov 6, 2015

I am going to wait for travis and if all good will merge it. Keep it up!

@dimrozakis

This comment has been minimized.

Contributor

dimrozakis commented Nov 6, 2015

thanks

@@ -1163,6 +1165,42 @@ def predict(self, gtab, S0=1):
return tensor_prediction(self.model_params[..., 0:12], gtab, S0=S0)
def iter_fit_tensor(step=1e4):

This comment has been minimized.

@Garyfallidis

Garyfallidis Nov 6, 2015

Member

I am going to go ahead and merge this PR because master breaks execution and a lot of people are always using the master. Please add a Parameters section in another PR. And explain what step is. Is this the number of voxels for example?

This comment has been minimized.

@dimrozakis

dimrozakis Nov 6, 2015

Contributor

Yes step is the number of voxels in one chunk. I will push a PR putting a params section, probably during the weekend.

Garyfallidis added a commit that referenced this pull request Nov 6, 2015

Merge pull request #766 from tomotech/opt_tensor_fit_iter
Add fit_tensor iteration decorator

@Garyfallidis Garyfallidis merged commit 718af67 into nipy:master Nov 6, 2015

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Nov 6, 2015

Thx @dimrozakis. Check my last comment. 👍

@dimrozakis dimrozakis deleted the advantis-io:opt_tensor_fit_iter branch Nov 6, 2015

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Nov 6, 2015

Okay good. While you are on it maybe check also that when you parallelized reslicing there was not memory addition there too. Multiprocessing can duplicate memory at times.

@dimrozakis

This comment has been minimized.

Contributor

dimrozakis commented Nov 7, 2015

@Garyfallidis:

Please add a Parameters section in another PR. And explain what step is.

Done in #768

While you are on it maybe check also that when you parallelized reslicing there was not memory addition there too.

Well, there is some memory increase. I think that's inevitable when using multiprocessing and not using shared memory. However, I don't think it's much of an issue. Let me elaborate a little bit on this:

Multiprocessing support in dipy.align.reslice was added in advantis-io@7f7e8a7 and the implementation splits the 4D input array to a number of 3D arrays, which it sends to the workers of a multiprocessing pool. When sending over a 3D array to a worker, then that array is duplicated. If one has N workers and G gradients, this will take up at most N/G more memory. For 4 workers and 40 gradients, that's a 10% temporary memory increase over the size of the input array. When sending back the results, until they're saved in the output 4D array, each chunk will temporarily take up more memory (since the output param of affine_transform can't be used). So there's some more memory there. Note however that it was the previous commit that started using the output param and that the pool implementation is still better in this aspect than 2 commits before. Also by default reslice won't use a Pool. So I'd say there isn't something to fix there. Some things could further be improved by using shared memory, but that also has its own problems (for example if the input array isn't given in shared memory, copying it there will temporarily take up twice as much memory).

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Nov 7, 2015

Thank you for the detailed explanation.

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