Skip to content

step logic changes: unit test #3467

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

Merged

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)

Chris Elion and others added 2 commits February 18, 2020 13:23
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 <goyenator@gmail.com>

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

Co-Authored-By: Chris Goy <goyenator@gmail.com>

* Made the tests not be broken

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

Co-Authored-By: Chris Elion <chris.elion@unity3d.com>

* step logic changes: unit test (#3467)

* Added a line in the changelog

Co-authored-by: Chris Goy <christopherg@unity3d.com>
Co-authored-by: Chris Elion <celion@gmail.com>
@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