-
Notifications
You must be signed in to change notification settings - Fork 331
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
Better scaling for reward plot + axis labels #216
Conversation
Looks good. Which would you prefer, the actual reward, or the moving average? Could you possible add an argument to |
Yes, that's what I was thinking. Would it be ok to use a |
Yeah, that's fine. We may want to remove the pandas dependency in the future, but for now it's okay. |
if self.reward_im is None and self.reward_ax is None: | ||
self.reward_im, self.reward_ax = plt.subplots() | ||
self.reward_ax.set_title('Accumulated reward') | ||
self.reward_ax.set_xlabel('Episode') | ||
self.reward_ax.set_ylabel('Reward') | ||
self.reward_plot, = self.reward_ax.plot(self.reward_list) | ||
self.reward_plot, = self.reward_ax.plot(reward_list_) |
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.
If self.reward_window
is None
, is reward_list_
created?
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.
Yes, that is done in the above if-else statement. If self.reward_window
is None
, reward_list_ = self.reward_list[:]
.
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.
Nice, I missed that for some reason.
if self.reward_im is None and self.reward_ax is None: | ||
self.reward_im, self.reward_ax = plt.subplots() | ||
self.reward_ax.set_title('Accumulated reward') | ||
self.reward_ax.set_xlabel('Episode') | ||
self.reward_ax.set_ylabel('Reward') | ||
self.reward_plot, = self.reward_ax.plot(self.reward_list) | ||
self.reward_plot, = self.reward_ax.plot(reward_list_) |
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.
Nice, I missed that for some reason.
In case accumulated rewards are constant for some episodes, the horizontal line is obscured since it's right at the figures edge when using
min
andmax
forset_ylim
. Instead, we could do automatic scaling withrelim
andautoscale_view
, which works well and leaves margins. See for example https://stackoverflow.com/questions/10984085/automatically-rescale-ylim-and-xlim-in-matplotlib. If interested, I could also add the option to plot a moving average of reward instead of the actual rewards, since those tend to be very shaky.