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

D4PG: Missing specification of critic variables in update operation? #1

Open
DevMMI opened this issue Apr 12, 2019 · 1 comment
Open

Comments

@DevMMI
Copy link

DevMMI commented Apr 12, 2019

Hey, thanks for this amazing repository! I was wondering where exactly the critic variables are being specified when building the critic update operation, and whether this was a possible accidental omission.

` # Take the mean loss on the batch
critic_loss = tf.negative(critic_loss)
critic_loss = tf.reduce_mean(critic_loss)
critic_loss += l2_regularization(self.critic_vars)

    # Gradient descent
    critic_trainer = tf.train.AdamOptimizer(Settings.CRITIC_LEARNING_RATE)

self.critic_train_op = critic_trainer.minimize(critic_loss)`

shouldn't it be

self.critic_train_op = critic_trainer.minimize(critic_loss, var_list=self.critic_vars)

in order to apply the loss function to minimize the variables? Thank you and please let me know if I'm incorrect.

@Valentin-Guillet
Copy link
Member

Hey, thanks for your interest !
The argument var_list=self.critic_vars is not necessary as by default, tensorflow uses the variables in GraphKeys.TRAINABLE_VARIABLES to update (see here).
Here, the trainable variables are the critic's and the actor's ones but the gradient of the critic loss on the actor variables is null, so it is not necessary to remove them.
I hope that this is clear, and feel free to ask any other question !

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

No branches or pull requests

2 participants