uclnlp / jack

Jack the Reader
MIT License
257 stars 82 forks source link

Wercker fails #251

Closed dirkweissenborn closed 7 years ago

dirkweissenborn commented 7 years ago

I removed a redundant initialization statement (we were calling it once in reader.setup_from_data and in after that directly again in reader.train).

Now overfitting fails, which doesn't make a lot of sense, because I thought calling initializers multiple times is idempotent.

Any ideas @pminervini ?

pminervini commented 7 years ago

I'm not sure that calling initializers multiple times is idempotent .. you are now initializing the parameters with different values, and the training log can be different from the one it used to be.

I can fix this by updating the training log in the tests - give me a few minutes.

dirkweissenborn commented 7 years ago

Well it was at least in earlier TF versions. Anyway, this commit is necessary to not duplicate initialization especially if it is not idempotent.

Thanks!

pminervini commented 7 years ago

I'm not totally sure, let's quickly check:

In [7]: z = tf.get_variable(shape=[5], name='z')
In [8]: init_op = tf.global_variables_initializer()
In [9]: session = tf.InteractiveSession()
In [10]: session.run(init_op)
In [11]: session.run(z)
Out[11]: array([ 0.01121974, -0.20893741,  0.36991453,  0.48889792, -0.07987314], dtype=float32)
In [12]: session.run(init_op)
In [13]: session.run(z)
Out[13]: array([ 0.33691907, -0.73696464, -0.52026761, -0.72819018,  0.40745521], dtype=float32)
dirkweissenborn commented 7 years ago

Interesting, seems that this has changed :) Can you fix the tests?

dirkweissenborn commented 7 years ago

oh you did