HazyResearch / deepdive

DeepDive
deepdive.stanford.edu
1.96k stars 536 forks source link

Non-evidence values not being sampled during Learning phase #269

Closed brahmaneya closed 9 years ago

brahmaneya commented 9 years ago

Due to this line of code, https://github.com/HazyResearch/sampler/blob/master/src/app/gibbs/single_thread_sampler.cpp#L45.

non-evidence values of variables stay fixed during the sampling phase. This causes problems in cases where we don't have training data for some types of variables. For example: If we have variables A_i, B_i, C_i for i from 1 to 1000. And for each i, we have factors A_i = B_i (learnable weight w1 shared across all i's), and B_i = C_i (learnable weight w2 shared across all i's), and most A_i's and C_i's are labelled, such that A_i is always equal to C_i. Then the learning should assign a high positive value to both w1, w2, or a high negative value to both. And during inference where only one of A_i or C_i is fixed, it should set the other one to be equal to it. But this doesn't happen because it never samples B_i values, and thus the factors are not activated during learning.

chrismre commented 9 years ago

@zhangce take a look at this :)

feiranwang commented 9 years ago

@zhangce I'll fix this.

zhangce commented 9 years ago

@feiranwang can you check in the fix we did before for this and deploy the binary? (I think you have the latest version of the patch for this (~Feb~March))

Ce On Apr 28, 2015 6:47 PM, "chrismre" notifications@github.com wrote:

@zhangce https://github.com/zhangce take a look at this :)

— Reply to this email directly or view it on GitHub https://github.com/HazyResearch/deepdive/issues/269#issuecomment-97264253 .

feiranwang commented 9 years ago

@zhangce Sure, I'll take care of it. Yes, we did this before. I found there's a bug in update function which will cause problem if we sample non-evidence variables. I'll fix it as well.

feiranwang commented 9 years ago

Fixed in https://github.com/HazyResearch/sampler/commit/ca3e8d45ef1c1c7d8badd16f82eb02847118d744.

chrismre commented 9 years ago

@netj @feiranwang @zhangce Guys, this should have triggered a larger response... you should have a series of small regressions about learning parameters correctly (e.g., samples of 1000 coin flips, etc.) Where are these regression tests?

feiranwang commented 9 years ago

Currently we have biased coin test for boolean and CRF test for multinomial under https://github.com/HazyResearch/sampler/tree/master/test. @zhangce what other tests do you think can be added?

robinjia commented 9 years ago

Chiming in because this bug affected me too...why don't we create a test corresponding to brahmaneya's example? Or more generally some sort of latent variable model (maybe a simple HMM?) where every edge is connected to at least one node that is not observed.

zhangce commented 9 years ago

We need some graph where there are hidden variables that do not have evidence during learning.

A(evid) -- B(not evid) connected by factor (A==B)

You have training data 1 1 1 1 1 1 1 0

You should expect the factor has a weight ~1.09 (log 3) if B is sampled.

Otherwise, you should expect the factor has a very small negative weight if B is init'ed with 0.

Or, you should expect the factor has a very large positive weight if B is init'ed with 1.

Ce

On Thu, Apr 30, 2015 at 12:47 PM, Feiran Wang notifications@github.com wrote:

Currently we have biased coin test for boolean and CRF test for multinomial under https://github.com/HazyResearch/sampler/tree/master/test. @zhangce https://github.com/zhangce what other tests do you think can be added?

— Reply to this email directly or view it on GitHub https://github.com/HazyResearch/deepdive/issues/269#issuecomment-97895910 .

feiranwang commented 9 years ago

Added partially observed variable test in https://github.com/HazyResearch/sampler/commit/9296be214f40acda711d84ff4045154deccc5f2d.