-
Notifications
You must be signed in to change notification settings - Fork 81
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
FIX: lm pre-training #58
Conversation
finetune/base.py
Outdated
@@ -489,6 +493,8 @@ def _build_model(self, n_updates_total, target_dim, train=True): | |||
elif not self.is_trained: | |||
self._load_base_model() | |||
|
|||
guarantee_initialized_variables(self.sess) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain the thought process behind this? Can't we explicitly initialize the variables under model/clf instead? Or add some logs for the variables that this call initializes. I can foresee a future where we accidentally mess up the base model and this could hide problems?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specify model/clf as scope
finetune/base.py
Outdated
self.do_dropout = tf.placeholder(tf.float32) # 1 for do dropout and 0 to not do dropout | ||
if self.target_type == SEQUENCE_LABELING: | ||
self.Y = tf.placeholder(tf.int32, [None, self.config.max_length]) # classification targets | ||
else: | ||
self.Y = tf.placeholder(tf.float32, [None, self.target_dim]) # classification targets | ||
self.Y = tf.stop_gradient(tf.placeholder(tf.float32, [None, self.target_dim or 1])) # classification targets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gradients already do not flow into placeholders The tf.stop_gradient
should be on the loss function to protect against changes to self.Y
finetune/utils.py
Outdated
@@ -95,6 +95,14 @@ def np_init(w): | |||
return partial(_np_init, w=w) | |||
|
|||
|
|||
def guarantee_initialized_variables(sess): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this.
finetune/utils.py
Outdated
@@ -95,6 +95,14 @@ def np_init(w): | |||
return partial(_np_init, w=w) | |||
|
|||
|
|||
def guarantee_initialized_variables(sess): | |||
global_vars = tf.global_variables() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Credit stackoverflow
0a7f620
to
ae3823d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
0ab0fa0
to
0988082
Compare
Resolves #56