-
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] Issue 22036: Client should handle concurrent episodes with one being training_enabled=False
.
#22076
[RLlib] Issue 22036: Client should handle concurrent episodes with one being training_enabled=False
.
#22076
Conversation
…e_22036_client_handles_concurrent_episodes
training_enabled=False
.training_enabled=False
.
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.
makes a lot of sense.
just a couple of suggestions.
@@ -172,7 +172,7 @@ def log_action( | |||
def log_returns( | |||
self, | |||
episode_id: str, | |||
reward: int, | |||
reward: float, |
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.
huh, interesting bug :)
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.
:)
@@ -0,0 +1,88 @@ | |||
#!/usr/bin/env python |
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.
should we add this as a unit test?
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.
Oh yes, I think I forgot this! Let me check. Great catch. It's working though, I checked on my laptop :D
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.
done
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.
thanks thanks :)
…e_22036_client_handles_concurrent_episodes
…e_22036_client_handles_concurrent_episodes
…e being `training_enabled=False`. (ray-project#22076)
Issue 22036: Client should handle concurrent episodes with one being
training_enabled=False
.Why are these changes needed?
Issue #22036
Related issue number
Closes #22036
Checks
scripts/format.sh
to lint the changes in this PR.