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

Distributed searchlight #148

Merged
merged 11 commits into from Nov 23, 2016
Merged

Conversation

@mjanderson09
Copy link
Contributor

@mjanderson09 mjanderson09 commented Nov 21, 2016

No description provided.

@yidawang
Copy link
Member

@yidawang yidawang commented Nov 21, 2016

Please provide more comments to describe at least the public methods. I found it a bit confused when reading the code now. For example, I am lost when trying to understand how the ownership is distributed.

from brainiak.searchlight.searchlight import Searchlight

"""Distributed Searchlight Example
"""

This comment has been minimized.

@mihaic

mihaic Nov 21, 2016
Contributor

Document the way this code supposed is supposed to be executed.

self.max_blk_edge = max_blk_edge

def _get_ownership(self, data):
"""Determine which processes own each subject

This comment has been minimized.

@mihaic

mihaic Nov 23, 2016
Contributor

"process owns"?

In the case of a 3D array, a 3D subarray at the block location
In the case of a 4D array, a 4D subarray at the block location,
including the entire 0th dimension.

This comment has been minimized.

@mihaic

mihaic Nov 23, 2016
Contributor

Isn't time the last dimension?

----------
sl_rad: radius, in voxels, of the sphere inscribed in the
searchlight cube

This comment has been minimized.

@mihaic

mihaic Nov 23, 2016
Contributor

Does this include the center or not?

subj: list of 4D arrays containing data for one or more subjects.
Each entry of the list must contaion a 4D array on only one
MPI rank, and "None" on all other ranks.

This comment has been minimized.

@mihaic

mihaic Nov 23, 2016
Contributor

Aren't we allowing all data to be loaded on a single rank?

Each entry of the list must contaion a 4D array on only one
MPI rank, and "None" on all other ranks.
mask: 3D array with nonzero values at active vertices

This comment has been minimized.

@mihaic

mihaic Nov 23, 2016
Contributor

s/nonzero/true


# Get/set ownership
ownership = self._get_ownership(subj)
full_pts = self._get_blocks(mask) if rank == 0 else None

This comment has been minimized.

@mihaic

mihaic Nov 23, 2016
Contributor

What is a point?

"""

comm = MPI.COMM_WORLD

This comment has been minimized.

@mihaic

mihaic Nov 23, 2016
Contributor

How about making comm an instance attribute?

Parameters
subj: list of 4D arrays containing subset of subject data

This comment has been minimized.

@mihaic

mihaic Nov 23, 2016
Contributor

s/subj/subjs?

Parameters
subj: list of 4D arrays containing subset of subject data

This comment has been minimized.

@mihaic

mihaic Nov 23, 2016
Contributor

The padding should be documented here.


return outmat

def searchlight_voxel(self, voxel_fn, pool_size=None):

This comment has been minimized.

@mihaic

mihaic Nov 23, 2016
Contributor

How about run_searchlight? And run_block_function for searchlight_block?

submasks = self._split_volume(mask, all_blocks)

# Scatter points, data, and mask
self.pts = self._scatter_list(all_blocks, 0)

This comment has been minimized.

@mihaic

mihaic Nov 23, 2016
Contributor

What is a point? See pt in the example as well.

mat: a 3D or 4D volume
block: a tuple containing block information:
- a triple containing the top left point of the block and

This comment has been minimized.

@mihaic

mihaic Nov 23, 2016
Contributor

Top left front or top left back? Also, wouldn't it be better to remove direction altogether and say "the voxel with the lowest coordinates"?

Parameters
----------
subj: list of 4D arrays containing data for one or more subjects.

This comment has been minimized.

@mihaic

mihaic Nov 23, 2016
Contributor

How about subjects?

@mihaic
mihaic approved these changes Nov 23, 2016
@mihaic mihaic merged commit 1895ec0 into brainiak:master Nov 23, 2016
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
linux Build finished.
Details
macos Build finished.
Details
@mjanderson09 mjanderson09 deleted the mjanderson09:distributed_searchlight branch Apr 26, 2017
danielsuo pushed a commit that referenced this pull request Nov 16, 2017
danielsuo pushed a commit that referenced this pull request Nov 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants