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

speedup distance computations #637

Closed
wants to merge 3 commits into from
Closed

Conversation

dfolch
Copy link
Member

@dfolch dfolch commented Jun 19, 2015

This PR addresses #621.

By turning off generatetree() the time to compute a distance matrix is cut by approximately 90% (ymmv). After turning this off, the code was tested using Network Usage.ipynb. The notebook tests most of the network module including NetworkG, NetworkK and Moran, and ran without problems.

@jlaura
Copy link
Member

jlaura commented Jun 20, 2015

(1) Tests are failing at line 103 because the tree is being used there. It appears that the tree is not used in the code (only the test). We should confirm that the test coverage is what we want even when the tree is not generated (I think it is).

(2) This PR needs to rebase from master after merging the snapping code.

@dfolch
Copy link
Member Author

dfolch commented Jun 20, 2015

  1. Thanks for catching that test failure.
  2. Is it enough to do a git pull upstream master into my local branch, or is something more sophisticated needed?

@jlaura
Copy link
Member

jlaura commented Jun 20, 2015

Git fetch upstream master
Git checkout master
Git merge upstream/master

On Saturday, June 20, 2015, David C. Folch notifications@github.com wrote:

  1. Thanks for catching that test failure.
    1. Is it enough to do a git pull upstream master into my local
      branch, or is something more sophisticated needed?


Reply to this email directly or view it on GitHub
#637 (comment).

@dfolch
Copy link
Member Author

dfolch commented Jun 20, 2015

I will reopen this under a new PR since my branch is a total mess.

@dfolch dfolch closed this Jun 20, 2015
@dfolch dfolch mentioned this pull request Jun 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants