uncomplicate / bayadera

High-performance Bayesian Data Analysis on the GPU in Clojure
Eclipse Public License 1.0
365 stars 24 forks source link

Fixing a minor error #6

Closed gurvesh closed 5 years ago

gurvesh commented 5 years ago

This error prevents custom size sampling on nvidia cuda and amd. The variable passed is currently ignored, which shouldn't happen. By correcting this - custom size sampling works correctly, even on cuda.

blueberry commented 5 years ago

I would have to look at this in detail, but as far as I remember, this was by design, since the algorithm assumes that all computing units are employed in searching the space.

Before taking the sample, the walkers need to search the space. Fewer walkers mean worse and longer search. Additionally, leaving computing units idle does not gain anything.

The idea is to always run all walkers, and if you need fewer samples, just take as many as you need. Once the space is well explored, you can freely take one, a handful, or up to walker-count samples. If you need more, just take additional step.

gurvesh commented 5 years ago

Sorry for not explaining properly. Currently what's happening is that no matter what sample size you give to the sample function, it returns a standard size array back. That's the case whether you ask for 1 sample or a million. The request is ignored. Due to this, even the code on the DBDA examples you've provided doesn't run on a cuda card.

If you look at the code I've tried to correct, it basically ignores the parameter n-or-res. I understand the logic of what's trying to be achieved, and rectifying it as I have gets there.

gurvesh commented 5 years ago

Here's an example of the error in the DBDA Chap 9 code (single coin). I haven't changed it - just showing the relevant lines:

Current behavior: (let [walker-count ( 256 44 32) sample-count ( 16 walker-count) ...... prior-sample (sample prior-sampler sample-count)

->> The code below fails
:prior {:sample (native (submatrix (p/data prior-sample) 0 0 2 walker-count) It fails because the prior-sample size is smaller than the submatrix being requested.

Requested submatrix is out of bounds. {:i 0, :j 0, :k 2, :l 360448, :mrows 2, :ncols 57344, :i+k 2, :j+l 360448}

This remains the case, no matter what sample-count you put into this line (sample prior-sampler sample-count). That simply gets ignored.

However, after fixing the code - everything works fine, and I verified that the "sample-count" value (passed as the n-or-res parameter) is being used correctly.

Hope that helps!

blueberry commented 5 years ago

Thank you for finding this! I'll have to look at it in detail, which will take a while since I have other stuff to work on, and this code is not fresh in my mind right now. It seems you are right, but I have to be careful to not introduce another bug by fixing this one!

blueberry commented 5 years ago

This was incorporated in latest changes. Thank you for discovering it.