[bug-fix] When agent isn't training, don't clear update buffer #5205
+59
−20
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chriselion
reviewed
Mar 31, 2021
| @@ -104,7 +104,9 @@ def save_replay_buffer(self) -> None: | |||
| Save the training buffer's update buffer to a pickle file. | |||
| """ | |||
| filename = os.path.join(self.artifact_path, "last_replay_buffer.hdf5") | |||
| logger.info(f"Saving Experience Replay Buffer to {filename}") | |||
| logger.info( | |||
| f"Saving Experience Replay Buffer to {filename} ({os.path.getsize(filename)} bytes)" | |||
There was a problem hiding this comment.
nit: Isn't this the old size? Maybe better to print after saving.
There was a problem hiding this comment.
This shouldn't have been merged - let me revert. It will actually crash since initially there is no last_replay_buffer.hdf5
There was a problem hiding this comment.
Updated to two messages:
2021-03-31 14:28:45 INFO [trainer.py:107] Saving Experience Replay Buffer to results/ppo/PushBlock/last_replay_buffer.hdf5...
2021-03-31 14:28:45 INFO [trainer.py:112] Saved Experience Replay Buffer (1591310 bytes).
So that users won't be wondering why the program is stuck (if the replay buffer is gigs in size).
chriselion
reviewed
Mar 31, 2021
| trainer._append_to_update_buffer(agentbuffer_trajectory) | ||
| assert trainer.update_buffer.num_experiences == (i + 1) * time_horizon | ||
|
|
||
| # Check fhat if we append after stopping training, nothing happens. |
There was a problem hiding this comment.
Suggested change
| # Check fhat if we append after stopping training, nothing happens. | |
| # Check that if we append after stopping training, nothing happens. |
chriselion
approved these changes
Mar 31, 2021
ervteng
pushed a commit
that referenced
this pull request
Apr 8, 2021
* Don't clear update buffer, but don't append to it either * Update changelog * Address comments * Make experience replay buffer saving more verbose (cherry picked from commit 63e7ad4)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed change(s)
Previously, we cleared the replay buffer at each timestep if the trainer was done training. This was to avoid a large memory leak if one trainer was done, but the other wasn't. However, this meant that SAC would just not have a replay buffer at the end of the run.
We now don't clear the buffer, but rather just don't append to it.
Useful links (Github issues, JIRA tickets, ML-Agents forum threads etc.)
JIRA MLA-1251
Verified with pushblock:
Types of change(s)
Checklist
Other comments