Skip to content

Conversation

chriselion
Copy link
Contributor

@chriselion chriselion commented Feb 18, 2020

To be merged into #3448

@@ -487,10 +487,8 @@ public void TestCumulativeReward()
agent1.LazyInitialize();
agent2.SetPolicy(new TestPolicy());

int expectedAgent1Resets= 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These were unused, and switch to var instead of int

int expectedResets= 0;
int expectedAgentAction = 0;
int expectedAgentActionSinceReset = 0;
var expectedAgentStepCount = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added some extra checks on the step count and collectobservations calls.

expectedAgentStepCount += 1;

// If the next step will put the agent at maxSteps, we expect it to reset
if (agent1.GetStepCount() == maxStep - 1 || (i == 0)){
Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMHO this is a little clearer (since it's a value on the actual Agent, not just a test variable)

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.

LGTM

@chriselion chriselion merged commit 3b7f190 into develop-modify-stepping-logic Feb 18, 2020
@delete-merged-branch delete-merged-branch bot deleted the develop-modify-stepping-logic-test branch February 18, 2020 22:55
vincentpierre added a commit that referenced this pull request Feb 19, 2020
* Moving the max step logic
 - Created a new Academy Event called AgentIncrementStep to be called before SetStatus
 - Implemented the AgentSteping logic

* second commit : Moving the step counting at the begining. I had to edit the tests but I think they are now closer to what we want

* addressing comments

* Update com.unity.ml-agents/Runtime/Agent.cs

Co-Authored-By: Chris Goy <[email protected]>

* Update com.unity.ml-agents/Runtime/Agent.cs

Co-Authored-By: Chris Goy <[email protected]>

* Made the tests not be broken

* Update com.unity.ml-agents/Runtime/Agent.cs

Co-Authored-By: Chris Elion <[email protected]>

* step logic changes: unit test (#3467)

* Added a line in the changelog

Co-authored-by: Chris Goy <[email protected]>
Co-authored-by: Chris Elion <[email protected]>
@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.

3 participants