BayesianLogic / blog

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

Get finite support #295

Closed datang1992 closed 10 years ago

datang1992 commented 10 years ago

@lileicc @cberzan Here I add the methods getFiniteSupport() to each distribution. This will be used for implementing Gibbs Sampling.

cberzan commented 10 years ago

Please try to fix any warnings in new code. Use List<type> when you know the type, e.g. List<int> in UniformInt. Where you don't know the type, e.g. in Categorical, use List<Object>.

datang1992 commented 10 years ago

@cberzan Related warnings fixed. Also the comment fixed. Please review this pull request first since I need to add getFiniteSupport() to the GEM distribution. But I could not add this method know since CondProbDistrib.java has not been changed in that branch.

chrisgioia64 commented 10 years ago

@cberzan @datang1992 So, I'm unfamiliar with Gibbs Sampling, but for many of the getFiniteSupport methods, (e.g. Geometric) you return null. Do you intend to fill these in later or is the getFiniteSupport unapplicable in these circumstances? I'm assuming for distributions such as geometric, they haven't yet been implemented, but don't know about continuous distributions.

If all distributions support getFiniteSupport then I suppose it might make sense to implement as many of them as possible before merging the pull requests. Unless it's not worth your time. Constantin, what are your thoughts on this? If there are some distributions that don't support getFiniteSupport (e.g. maybe continuous distributions) maybe make an interface that extends CondProbDistrib to be used for Gibbs Sampling.

lileicc commented 10 years ago

@cberzan @chrisgioia64 @datang1992 Many of them return null because they are distributions on continuous values. They do not have finite support.

datang1992 commented 10 years ago

@chrisgioia64 Please help me to check and test these modifications. Thanks!

chrisgioia64 commented 10 years ago

@datang1992 Please see updated commit. For multinomial, the finite support wasn't copying properly.

datang1992 commented 10 years ago

@lileicc @chrisgioia64 I have changed the return type to Object[] since it may be faster.

lileicc commented 10 years ago

@datang1992 A general comment, instead of computing finiteSupport during setParam(), you may do it lazily. During setParam(), set finiteSupport to null. During getFiniteSupport(), check if hasParam and null, then calculate finitesupport, otherwise just return finitesupport (could be null).

datang1992 commented 10 years ago

@lileicc I have moved the calculation parts outside setParams(), also have computed finiteSupport for the tree infinite discrete distributions.

datang1992 commented 10 years ago

@chrisgioia64 It seems that we don't have a JUnit Test Class for UniformChoice. Could you add one? Thanks!

datang1992 commented 10 years ago

@lileicc Could this branch be merged?

cberzan commented 10 years ago

@datang1992 Please merge master into this branch so that we can see if the travis build passes.

lileicc commented 10 years ago

LGTM. merge after passing the test.