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

Clean up GPU branch [10] #61

Closed
bcumming opened this issue Nov 4, 2016 · 5 comments
Closed

Clean up GPU branch [10] #61

bcumming opened this issue Nov 4, 2016 · 5 comments
Assignees

Comments

@bcumming
Copy link
Member

bcumming commented Nov 4, 2016

point estimate: 10 points
feature: #67

The GPU branch github.com/bcumming/nestmc-proto/tree/gpu needs to be merged with master. There are issues that make this difficult

  1. its "diff" is overwhelming relative to the master
  2. the quality of the code in this branch needs to be improved (types, comments, design, documentation)
  3. more work is required to get it ready for final merge: namely validation of the gpu results

This task aims to target steps 1 and 2 in a gpu branch of the main repository. Once the state of the branch is satisfactory, the validation in step 3 will be addressed in a separate project task.

Acceptance criteria

  • all tests pass and validation pass for the CPU back end
  • the CPU back end has unchanged performance relative to current master
  • gpu version of tests and miniapp compiles and runs without crashing
  • @halfflat approves the design and implementation of the cpu and gpu implementations

Steps

  1. address @halfflat's comments in Part 1 of the big GPU merge #45 [3]
  2. add changes since Part 1 of the big GPU merge #45 to the PR [4]
  3. address @halfflat's comments on step (2) [3]
@bcumming bcumming added this to the sprint 2016.11.07 milestone Nov 4, 2016
@halfflat
Copy link
Contributor

halfflat commented Nov 4, 2016

Looks good to me!

@bcumming
Copy link
Member Author

bcumming commented Nov 4, 2016

Finished the estimates. I went through your first code review and just made a quick tally of the small and large fixes, to come up with 6 points for me to make fixes, comment on the review, and then make further changes based on follow up review.

@bcumming bcumming changed the title Clean up GPU branch Clean up GPU branch [10] Nov 7, 2016
@bcumming bcumming self-assigned this Nov 7, 2016
@bcumming
Copy link
Member Author

Step 1 has been finished, and step 2 mostly finished. The backend implementation hasn't been fully polished, but I think that its structure is quite clear and fit for purpose.

@bcumming
Copy link
Member Author

Step 2 has been finished, the PR #45 is ready for review.

@halfflat
Copy link
Contributor

PR #45 merged!

noraabiakar pushed a commit to noraabiakar/arbor that referenced this issue Nov 26, 2019
* Coerce `ncell` parameter to int. This fix was missed in an earlier commit/merge.
* Remove temporary directory shenanigans for CoreNEURON implementation as we can pass it an output directory to use.
akuesters pushed a commit to akuesters/arbor that referenced this issue Aug 16, 2021
* ImPlot Yeah.

* Fix acc suffix.

* Fix PVS warnings.
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

No branches or pull requests

2 participants