New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Fix for #74 #119
[WIP] Fix for #74 #119
Conversation
Thanks! Looks like there's still an error for that issue though:
|
@dustinvtran could you please share code to reproduce it? On my machine both |
Here's the source for the jupyter notebook I tried. If I run the code block twice I get the error with your fix. (Without the fix I get the error reported in #74).
|
@dustinvtran It seems it is Python3 issue (it works fine on Python2). I will look into it |
@dustinvtran So it seems it is stability issue, for instance seed=100 works well. So to fix it, clearly we need:
About seeding: currently code is globally seeded, which is often considered a bad practice (for instance user can accidentally reseed code), but I assume this should be left unchanged for now. The core of the problem is that when single session is shared, seeding of
prints different number each time we are in the same session (for more information see https://www.tensorflow.org/versions/r0.8/api_docs/python/constant_op.html#set_random_seed). This is used in https://github.com/blei-lab/edward/blob/master/edward/util.py#L214. So best solution would be to create new session each time in inference.initialize. There are many hacks around it, like storing initial values in a separate dict in Btw. sorry for not adhering to the PR formatting policy - noticed it just now. |
Thanks for spotting this. So it sounds like there are a few issues. They all revolve around the fact that when a notebook runs a code block again, the Python environment is the same, and currently as a product of Edward's internals, the TensorFlow session is the same. This causes downstream issues such as
Having the same TensorFlow session is ideal across the same Python environment and inference algorithms so we shouldn't change this. We could have |
Thanks for the reply.
Yup, this is more or less what I meant. I will try to come up with something, but just to warn, I am afraid there is no "elegant" solution here :) Minor comment:
This is just matter of instability - so it is a separate issue as well. I am not sure how to fix it. For instance for seed=0 even the first run prodcues Domain error (caused by |
Sounds great. I'm not sure I understand the instability comment though (or the Python 3 comment). For example, with the Python script #!/usr/bin/env python
import edward as ed
import tensorflow as tf
from edward.models import Variational, Beta
from edward.stats import bernoulli, beta
class BetaBernoulli:
"""
p(x, z) = Bernoulli(x | z) * Beta(z | 1, 1)
"""
def log_prob(self, xs, zs):
log_prior = beta.logpdf(zs, a=1.0, b=1.0)
log_lik = tf.pack([tf.reduce_sum(bernoulli.logpmf(xs, z))
for z in tf.unpack(zs)])
return log_lik + log_prior
ed.set_seed(0)
model = BetaBernoulli()
variational = Variational()
variational.add(Beta())
data = ed.Data(tf.constant((0, 1, 0, 0, 0, 0, 0, 0, 0, 1), dtype=tf.float32))
inference = ed.MFVI(model, variational, data)
inference.run(n_iter=500) I run it in on Python 2.7.10 and TensorFlow 0.8.0, and it always produces the same output (only the address for the function initializer changes): INFO:tensorflow:Created variable alpha:0 with shape (1,) and init <function _initializer at 0x101344b18>
INFO:tensorflow:Created variable alpha:0 with shape (1,) and init <function _initializer at 0x101344b18>
INFO:tensorflow:Created variable beta:0 with shape (1,) and init <function _initializer at 0x101344b18>
INFO:tensorflow:Created variable beta:0 with shape (1,) and init <function _initializer at 0x101344b18>
iter 0 loss -6.56
shape:
[ 0.28207624]
scale:
[ 0.96621323]
iter 100 loss -8.06
shape:
[ 0.39867195]
scale:
[ 1.25165844]
iter 200 loss -6.52
shape:
[ 0.67361927]
scale:
[ 1.5545224]
iter 300 loss -5.40
shape:
[ 0.69744116]
scale:
[ 2.3602047]
iter 400 loss -6.04
shape:
[ 0.95770347]
scale:
[ 2.53179455] |
Epsilon-bounds to stay within open intervals (e.g., |
Yup, the Python3 comment wasn't really accurate. It just didn't happen for that seed under Python2.
Ups, sorry. I had a slight difference in my script. Changing the seed to 12 in your code makes it diverge (in Python3, not Python2). |
Okay gotcha; with |
Okay I was able to fix the internal copy of the layers object across classes in 09bf835. (see also related stackoverflow) from edward.models import Variational
from edward.models import Normal
print(Variational().layers)
Variational().add(Normal(2))
print(Variational().layers) prints
as expected.
So this fixes 1. The latter two are still open. |
Sorry, I had a really busy week! I will do my best to look at this during the weekend. Btw, this PR already introduced fix for 1 :) |
No problem. Also yes, the commit above was based on yours. :) |
f276bef
to
a42627d
Compare
Otherwise this happens :) :
prints