BayesianLogic / blog

The BLOG programming language
http://bayesianlogic.github.io/
BSD 4-Clause "Original" or "Old" License
98 stars 31 forks source link

change multinomial return type to be Integer[] (previously RealMatrix) #317

Closed lileicc closed 9 years ago

lileicc commented 9 years ago

@jxwuyi Please review

  1. added spec for int array, real array
  2. fixed array sub resolution issue
  3. fixed histogram for array values
  4. fixed multinomial value type to be ArrayList

@datang1992 Please check if the getFiniteSupport is correct.

@cberzan FYI.

lileicc commented 9 years ago

@datang1992 please fix testing for Multinomial. tks!

lileicc commented 9 years ago

@jxwuyi bird model works in this branch.

datang1992 commented 9 years ago

@lileicc I am wondering why are you using ArrayList instead of Integer[]. I think Integer[] is better for us to handle, and it should be more efficient. Even if you want to use a list, I think LinkedList is a better choice since it will be more convenient for computing the FiniteSupport. In fact, if we use any kind of list, we need to modify a lot of code, especially in the test code.

lileicc commented 9 years ago

@datang1992 @jxwuyi Because we need to store the value in historgram, which is using hashCode. hashCode of Integer[] will be just object address, while hashCode of ArrayList will be based on the content of array. i.e. [1, 2, 3] is treated same as [1, 2, 3] even if you have different array instance.

lileicc commented 9 years ago

@jxwuyi Since both Da and I worked on this branch, it is best if you can review this pr.

jxwuyi commented 9 years ago

I will review now

On Sun, Oct 5, 2014 at 5:36 PM, lileicc notifications@github.com wrote:

@jxwuyi https://github.com/jxwuyi Since both Da and I worked on this branch, it is best if you can review this pr.

— Reply to this email directly or view it on GitHub https://github.com/BayesianLogic/blog/pull/317#issuecomment-57958308.