Skip to content
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

Add global_counter to Trainer as optional argument #271

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

wielandbrendel
Copy link
Contributor

This PR allows to use tf.train.exponential_decay together with Trainer. It adds an optional argument to pass a global_counter to Trainer, and to hence initialise the global_counter before initializing Trainer. This is necessary if the parameter to be decayed is part of the loss function.

else:
self.global_step = tf.Variable(0., name='Global_Step',
trainable=False)
self.global_step = global_step
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line shouldn't be necessary as it would have already been set. This conflicts with my other comment but either way we can make this more simple in one way or the other.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last line is a stupid mistake - sorry!

@@ -87,8 +87,11 @@ def __init__(self, train_ops, graph=None, clip_gradients=5.0,
self.validate_trainop_names()

self.global_loss = None
self.global_step = tf.Variable(0., name='Global_Step',
trainable=False)
if global_step:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you need to set:

if global_step is None:
    global_step = tf.Variable(0., name='Global_Step', trainable=False)
self.global_step = global_step

because 'if Tensor' raises an exception in TensorFlow.

@aymericdamien
Copy link
Member

That looks good @wielandbrendel! I just added a comment to use:

if global_step is None:
    global_step = tf.Variable(0., name='Global_Step', trainable=False)
self.global_step = global_step

as @braddengross suggested, because the other way will raise a TensorFlow error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants