Calling EndEpisode inside CollectObservations results in a recursive call to CollectObservations

[Phong13]

Calling EndEpisode inside CollectObservations results in a recursive/nested CollectObservations call. The stack trace looks like this:

16 CollectObservations called 
UnityEngine.Debug:Log(Object)
Ball3DAgent:CollectObservations(VectorSensor) (at Assets/ML-Agents/Examples/3DBall/Scripts/Ball3DAgent.cs:52)
Unity.MLAgents.Agent:NotifyAgentDone(DoneReason) (at D:/Workspace/Unity/MLAgentsColossus_UpgradeToRelease_6/CleanRelease7/com.unity.ml-agents/Runtime/Agent.cs:515)
Unity.MLAgents.Agent:EndEpisodeAndReset(DoneReason) (at D:/Workspace/Unity/MLAgentsColossus_UpgradeToRelease_6/CleanRelease7/com.unity.ml-agents/Runtime/Agent.cs:731)
Unity.MLAgents.Agent:EndEpisode() (at D:/Workspace/Unity/MLAgentsColossus_UpgradeToRelease_6/CleanRelease7/com.unity.ml-agents/Runtime/Agent.cs:706)
Ball3DAgent:CollectObservations(VectorSensor) (at Assets/ML-Agents/Examples/3DBall/Scripts/Ball3DAgent.cs:57)
Unity.MLAgents.Agent:SendInfoToBrain() (at D:/Workspace/Unity/MLAgentsColossus_UpgradeToRelease_6/CleanRelease7/com.unity.ml-agents/Runtime/Agent.cs:1013)
Unity.MLAgents.Agent:SendInfo() (at D:/Workspace/Unity/MLAgentsColossus_UpgradeToRelease_6/CleanRelease7/com.unity.ml-agents/Runtime/Agent.cs:1262)
Unity.MLAgents.Academy:EnvironmentStep() (at D:/Workspace/Unity/MLAgentsColossus_UpgradeToRelease_6/CleanRelease7/com.unity.ml-agents/Runtime/Academy.cs:578)
Unity.MLAgents.AcademyFixedUpdateStepper:FixedUpdate() (at D:/Workspace/Unity/MLAgentsColossus_UpgradeToRelease_6/CleanRelease7/com.unity.ml-agents/Runtime/Academy.cs:43)

Notice that CollectObservations is called from within a previous CollectObservations call. I am aware that it is probably not a great idea to end an episode while CollectObservations it being run, but since the API allows it, the API should handle this.

There is a similar issue when EndEpisode is called from within EnvironmentReset. This should not be valid. There is no episode to end. This should generate an error. Instead it fires "CollectObservations" and "EpisodeBegin" before the environment has finished resetting.

There is a similar issue when EndEpisode is called from within PreAcademyStep. What step are we in? The previous step or the next step? FixedUpdate has stepped, but the Academy has not yet incremented the step counter. A call to "CollectObservations" happens before the AcademyStep has been incremented. What step does do these observations belong to? Also OnBeginEpisode gets fired. How many steps were in that episode? Does the step we are about to execute belong to this episode or the next?

To Reproduce
Use the 3DBall example
Add a EndEpisode inside CollectObservations.
Add a Debug.Log to CollectObservations that prints out the academy step:

    public override void CollectObservations(VectorSensor sensor)
    {
...
        Debug.Log(Academy.Instance.StepCount + " CollectObservations called ");
        if (Academy.Instance.StepCount % 16 == 0)
        {
            EndEpisode();
            Debug.Break();
        }
...
}

Run the scene in training mode and observe the nested CollectObservations call in the stack trace.

Try calling EndEpisode inside AcademyReset and PreAcademyStep and observe the similar calls.

Expected Result:

There should be some enforcement inside the Academy to ensure that EndEpisode is being called in a valid context.

• EndEpisode should only be callable in the context of a valid episode step (not in PreAcademyStep or EnvironmentReset)
• Calling EndEpisode outside the context of a valid episode step should do nothing and print an error to the console
• Calling EndEpisode in PreAcademyStep (after the previous step has finished but before the next step has started) should do nothing and print an error to the console. Which step do these observations belong to?
• Calling EndEpisode in CollectObservations should generated an error and do nothing.
• Calling EndEpisode should not generate an embedded call to CollectObservations. This extra call to CollectObservation will not return the correct observations most of the time.

The extra CollectObservations call generated by EndEpisode is probably not generating valid observations:

If it is called from within PreAcademyStep, then the step may have not initialized completely. For example the StepCount has not been incremented. It is possible that StepCount is being used in observations (eg. even steps are my turn, odd steps are my opponents turn).

If it is called from within CollectObservations then there is a recursive call to CollectObservtions. It is likely that these calls could set setting state on the agent and these state changes would interfere with each other.

If it is called in OnActionReceived then the agent has probably applied its actions to the environment. The new observations would not generate the actions generated this frame.

Console logs / stack traces

Ball3DAgent:CollectObservations(VectorSensor) (at Assets/ML-Agents/Examples/3DBall/Scripts/Ball3DAgent.cs:52)
Unity.MLAgents.Agent:NotifyAgentDone(DoneReason) (at D:/Workspace/Unity/MLAgentsColossus_UpgradeToRelease_6/CleanRelease7/com.unity.ml-agents/Runtime/Agent.cs:515)
Unity.MLAgents.Agent:EndEpisodeAndReset(DoneReason) (at D:/Workspace/Unity/MLAgentsColossus_UpgradeToRelease_6/CleanRelease7/com.unity.ml-agents/Runtime/Agent.cs:731)
Unity.MLAgents.Agent:EndEpisode() (at D:/Workspace/Unity/MLAgentsColossus_UpgradeToRelease_6/CleanRelease7/com.unity.ml-agents/Runtime/Agent.cs:706)
Ball3DAgent:CollectObservations(VectorSensor) (at Assets/ML-Agents/Examples/3DBall/Scripts/Ball3DAgent.cs:57)
Unity.MLAgents.Agent:SendInfoToBrain() (at D:/Workspace/Unity/MLAgentsColossus_UpgradeToRelease_6/CleanRelease7/com.unity.ml-agents/Runtime/Agent.cs:1013)
Unity.MLAgents.Agent:SendInfo() (at D:/Workspace/Unity/MLAgentsColossus_UpgradeToRelease_6/CleanRelease7/com.unity.ml-agents/Runtime/Agent.cs:1262)
Unity.MLAgents.Academy:EnvironmentStep() (at D:/Workspace/Unity/MLAgentsColossus_UpgradeToRelease_6/CleanRelease7/com.unity.ml-agents/Runtime/Academy.cs:578)
Unity.MLAgents.AcademyFixedUpdateStepper:FixedUpdate() (at D:/Workspace/Unity/MLAgentsColossus_UpgradeToRelease_6/CleanRelease7/com.unity.ml-agents/Runtime/Academy.cs:43)

Screenshots

Environment (please complete the following information):

• Unity 2019.3
• Windows 10
• Version 7
• 2.3.1
• 3DBall environment but any environment will work

[andrewcoh]

Hi @Phong13

I'm passing this on to the team to decide the best way to address this issue since we've seen similar things like this i.e. this older issue #4220 and fix #4227. Thank you for pointing this out.

[Phong13]

Thanks, I put a bit more thought into this overnight regarding the desired behavior.

For PPO and SAC to work correctly, an episode should consist of a an ordered collection of steps. Each step should have:

• Observations
• Actions
• Reward

==================
The Academy should have some integrity checking to ensure that EndEpisode is being called in a valid context:

• The agent must be inside an episode
• There must be at least one step
• The last step must be complete (has Observations, Actions, Reward)

If EndEpisode is called and this is not the case then errors should be generated.

[chriselion]

This was fixed in a previous version.

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.