Skip to content

Conversation

@ervteng
Copy link
Contributor

@ervteng ervteng commented Feb 7, 2020

In a recent change, the Agent ID now changes after every episode reset. This caused the dictionary of keys to grow inside the AgentProcessor. This PR cleans up the adding/deletion of items on reset boundaries, so that the dict doesn't grow.

@ervteng ervteng requested a review from vincentpierre February 7, 2020 02:51
Copy link
Contributor

@vincentpierre vincentpierre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have some training results on bouncer and hallway to see if there are regressions ?

if not self.last_step_result[_gid][0].done:
if "action" in take_action_outputs:
self.policy.save_previous_action(
[global_id], take_action_outputs["action"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_gid ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

self.policy.save_previous_action(
previous_action.agent_ids, take_action_outputs["action"]
# Index is needed to grab from last_take_action_outputs
self.last_step_result[global_id] = (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_gid ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one needs to be global_id, its in a different loop

@chriselion
Copy link
Contributor

How do you know it works without a test?

@ervteng
Copy link
Contributor Author

ervteng commented Feb 8, 2020

How do you know it works without a test?

Tracking the size of the dicts using example envs (Hallway, Bouncer). Still need to think of a good unit test for this; we need to emulate On-demand decisions

@ervteng ervteng marked this pull request as ready for review February 10, 2020 23:40
@ervteng
Copy link
Contributor Author

ervteng commented Feb 11, 2020

image
Bouncer
image
Hallway

@ervteng
Copy link
Contributor Author

ervteng commented Feb 11, 2020

@chriselion

How do you know it works without a test?

Added a unit test that checks the basic deleting mechanism between resets.

@chriselion
Copy link
Contributor

Great, just wanted to make sure we had some checks on it.

@ervteng ervteng merged commit 348d1d6 into release-0.14.0 Feb 11, 2020
@delete-merged-branch delete-merged-branch bot deleted the release-fixmemoryleak branch February 11, 2020 23:26
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants