-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
[RLlib] Minor cleanups in Trainer
; better tf/tf2 info messages about possible tracing speedups.
#20109
Conversation
Trainer
; metter tf/tf2 info messages about possible tracing speedups.Trainer
; better tf/tf2 info messages about possible tracing speedups.
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.
A couple of nit picking comments. Looks nice overall.
rllib/agents/trainer.py
Outdated
self.env_creator = _global_registry.get(ENV_CREATOR, env) | ||
# A class specifier. | ||
elif "." in env: | ||
if _global_registry.contains(ENV_CREATOR, self._env_id): |
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.
maybe worth breaking everything in this if statement into a little util function, so things don't look so nested here.
"(framework='tf')") | ||
# Tf-static-graph (framework=tf): Recommend upgrading to tf2 and | ||
# enabling eager tracing for similar speed. | ||
elif tf1 and self.config["framework"] == "tf": |
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.
haha, maybe we can say this in a more "recommendation" manner, since this user is specifically asking for tf1 here? something like:
You are using tensorflow static-graph mode. Consider setting framework='tf2' to upgrade to tf2.x eager execution, and eager_tracing=True to reach similar execution speed as static-graph mode.
Just a suggestion.
Minor cleanups in
Trainer
; metter tf/tf2 info messages about possible tracing speedups.Why are these changes needed?
Related issue number
Checks
scripts/format.sh
to lint the changes in this PR.