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

Parallelized local tracking branch so now you can actually look at my code :) #666

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@amitvakula

amitvakula commented Jun 16, 2015

No description provided.

amitvakula added some commits Jun 3, 2015

- Added the initial draft of the optiized local tracking class. This …
…class attempts to take the LocalTracking class (a deterministic streamline generation/fiber tracking algorithm in dipy/tracking/local) and to parallelize its streamline generation

- Implemented __reduce__ methods for ThresholdTissueClassifer and for PeaksAndMetricsDirectionsGetters. I also implemented a __setstate__ method for PeaksAndMetricsDirectionsGetters They compiled and I did not modify any other functions so it shouldn't crash anything. I implemented these methods so these extension times are serializable (which is require for many parallelization frameworks)
- sucessfully serialized PeaksAndMetrics objects by implementing __re…
…duce__ method and __setstate__ method

- will add tests verifying correct serialization later
streamline = np.concatenate(parts, axis=0)
return streamline
class OptimizedLocalTracking(object):

This comment has been minimized.

@MrBago

MrBago Jun 26, 2015

Contributor

This class seems like it's mostly a copy of the LocalTracking class. You should either change the class directly or use inheritance. We're going to need you to do that before we merge it, plus it makes it much easier for me to see what you've changed :).

args.append(maxlen)
args.append(fixed)
args.append(return_all)
return args

This comment has been minimized.

@MrBago

MrBago Jun 26, 2015

Contributor

this is easier to read if you right:

args = [inv_A, lin, offset, ...,
        return_all]`
current_state = state["current_state"]
self._qa = current_state["_qa"]
self._ind = current_state["_ind"]
self._odf_vertices = current_state["_odf_vertices"]

This comment has been minimized.

@MrBago

MrBago Jun 26, 2015

Contributor

These assignments are already done in self._initialize() so do we need to do them again?

@MrBago

This comment has been minimized.

Contributor

MrBago commented Jun 26, 2015

We should meet and talk about the set_state stuff, but here are my thoughts in summery.

  1. We don't need __setstate__, we can do everything using __init__ (I think). That should make the code cleaner.
  2. We should make the code more object oriented. That means either a) only defining each method once, either in the parent or child as needed or b) having the child method call the parent method as needed.
@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Aug 1, 2015

Ping! @amitvakula and @MrBago Any progress with this? Oh and thank you @amitvakula for sharing your code. Groovy!

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Aug 2, 2015

@MrBago one of the reasons why tracking is sometimes slow is because some of the algorithms like max odf needs to calculate the ODF on the fly. Could we use some different representation to speedup the process? Do we really need those large matrix multiplications or we can do this with some smaller matrix multiplications like in Cholesky?

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Dec 30, 2015

What about this PR? Is something better on the way? Otherwise we need to close it.

@MrBago

This comment has been minimized.

Contributor

MrBago commented Dec 31, 2015

I think this is on hold, lets close this and we'll make a new PR if need be.

@MrBago MrBago closed this Dec 31, 2015

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