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

Refactor peeling.cc and add tests associate with it #83

Closed
wants to merge 3 commits into from

Conversation

stevenhwu
Copy link
Collaborator

Issue #80
Refactor peeling.cc, forward only

  • add up_core() and to_parent_core() to simplify the code.
  • Add a few utility functions, such as sum_over_children(), multiply_upper_lower(). Some might change after refactor reverse functions.

Add 3 boost_test

  • test_sum_over_child
  • test_up_core
  • test_down

TODO:

  • Some of these might be better in struct workspace_t
  • Better check/testing for "family" can be parent-child or Dad-mom-child
  • Check reverse functions
  • Some debug codes might be redundant

@stevenhwu stevenhwu self-assigned this Jan 24, 2016
@dng-jenkins
Copy link

Can one of the admins verify this patch?

@reedacartwright
Copy link
Contributor

Jenkins, ok to test

@reedacartwright
Copy link
Contributor

@stevenhwu Have you tested the performance of your changes? Eigen does the best when more operations are on the same line because it can optimize them efficiently.

@stevenhwu
Copy link
Collaborator Author

Thanks, good to know that's how Eigen works. Ran some quick (incompletely) tests, the difference is like 2s in millions of calls. Push the new test case soon.
TODO:

  • Update them if these are the bottleneck
  • Make it Eigen fast and without redundancy

@stevenhwu
Copy link
Collaborator Author

Update peeling.cc to ensure performance.
Based on 10 Millions calls, the difference ~ +- 2%.
TODO: better test dir layout #85

@stevenhwu
Copy link
Collaborator Author

Update peeling.cc. Some eigen related functions are defined in macro to ensure speed.
Based on 10 Millions calls or randomly generated dataset, the difference ~ +- 2%.

stevenhwu added a commit to stevenhwu/denovogear that referenced this pull request Mar 8, 2016
@stevenhwu stevenhwu closed this Mar 9, 2016
@stevenhwu stevenhwu deleted the feature_peeling branch June 8, 2016 19:31
@stevenhwu stevenhwu restored the feature_peeling branch June 8, 2016 19:36
@stevenhwu stevenhwu deleted the feature_peeling branch August 12, 2016 19:44
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.

3 participants