search

clMathLibraries / clBLAS

a software library containing BLAS functions written in OpenCL
Apache License 2.0
839 stars 240 forks source link

fix hard-coding of opencl version to 2.0; fix 1d initialization of 2d… #173

Closed hughperkins closed 8 years ago

hughperkins commented 8 years ago

… arrays.

Fixes the two bugs identified in https://github.com/clMathLibraries/clBLAS/issues/172

This means that the test code runs to completion for hte first iteration, before failing with an error -38 now. Error -38 will be fixed in a pull request for 2111bb03936 , once I have reproduced the issue, and demonstrated that 2111bb03936 fixes it.

TimmyLiu commented 8 years ago

Please make pull request to develop branch.

TimmyLiu commented 8 years ago

currently master branch is ahead of develop branch with pull requests #158 , #163 and #170. we are going to bring develop branch up to date with master branch. Can you make a pull request against develop branch after that? If master branch is proven not stable and develop branch contains fixes. We can then merge develop branch to master branch and make another release.

hughperkins commented 8 years ago

In my opinion you should have two branches really:

That way 2.8.x will be relativley stable. develop can contain bleeding edge work.

kknox commented 8 years ago

The opencl math library projects follow the 'git flow' scheme, in which all 'new' code should be committed into 'develop'. For critical bug-fixes, a we can cherry-pick commits and push out a new 'patch' (like 2.8.1) release. When making your commits into 'develop', it would help us if you isolate each bug fix into a single commit, so that we can cleanly cherry-pick each fix into 'master'. If your fork has many commits as you worked on bug-fixes, you can squash the commits into a minimal set first before you issue a PR with git rebase --interactive

The problem with accepting code into 'master' branch is that code from 'master' never flows into 'develop' (it's a one-way valve). We risk losing changes the next time 'develop' merges into 'master', because if the merge is done carelessly the changes to 'master' will be overwritten by 'develop' which never had the fix.

hughperkins commented 8 years ago

For critical bug-fixes, a we can cherry-pick commits and push out a new 'patch' (like 2.8.1) release.

Hmmm. That sounds reasonable, and when I read it, I thought it was perfectly 100% reasonable. However, there seem to be two potential issues with this:

  1. once your development branch has gone through a certain number of revisions, the bugfixes will start to take place on code that looked totally different in master, or didnt even fully exist yet, and therefore the cherry-pick is impossible
  2. the cherry-picking approach is the sort of thing that never happens I reckon. The approach will plausibly become "well, you should just run off develop" :-)
hughperkins commented 8 years ago

Addendum: for other projects, eg torch, there are no branches at all: everything runs from master. However, for torch, everyone runs in basically the exact same configuration: Ubuntu 14.04, with probably a CUDA card.

This is not really the case for clBLAS, where core team are using only AMD cards, but there are a lot more cards out there, so it's not really sufficient to run a few CI server tests and reckon it works: you wont know about all the bugs until people on different hardware/os combinations try them.

So, concretely, I'm not going to base my own library off your develop, because it's too unstable, rarely works for me, since mostly only tested on AMD (in my experience). ... and at the same time I cant run from your master, since missing bugfixes. For now, what I've been doing is making my own bugfix branch for this, currently 2.8-bugfixes1.