[change] Make Agent non-abstract, update Basic scene. #3528

chriselion · 2020-02-28T05:20:14Z

Proposed change(s)

Make Agent non-abstract, and remove exception from default implementation of Heuristic()
Change Basic scene to not inherit from Agent
- Calls Agent.GetAction() and uses a SensorComponent instead of CollectObservations
- I kept the name BasicAgent - not sure what else to call it?
Move SensorBase out of Sensor namespace and into Examples.

Useful links (Github issues, JIRA tickets, ML-Agents forum threads etc.)

Types of change(s)

Checklist

Added tests that prove my fix is effective or that my feature works
Updated the changelog (if applicable)
Updated the documentation (if applicable)
Updated the migration guide (if applicable)

Other comments

chriselion · 2020-02-28T05:21:06Z

Project/Assets/ML-Agents/Examples/Basic/Scripts/BasicAgent.cs

+/// Observations are generated by the attached SensorComponent, and the actions
+/// are retrieved from the Agent.
+/// </summary>
+public class BasicAgent : MonoBehaviour


BasicController? BasicPlayer? BasicAI?

BasicController

chriselion · 2020-02-28T05:21:43Z

Project/Assets/ML-Agents/Examples/Basic/Scripts/BasicAgent.cs

-        largeGoal.transform.position = new Vector3(m_LargeGoalPosition - 10f, 0f, 0f);
-    }
-
-    public override float[] Heuristic()


Removed this since I don't want to inherit from Agent. I don't think it was getting us much.

chriselion · 2020-02-28T05:22:27Z

Project/Assets/ML-Agents/Examples/Basic/Scripts/BasicAgent.cs

@@ -92,16 +85,21 @@ public void FixedUpdate()

    void WaitTimeInference()
    {
-        if (!Academy.Instance.IsCommunicatorOn)
+        if (Academy.Instance.IsCommunicatorOn)


The logic got flipped here accidentally at some point. Now training steps quickly and inference takes its time.

chriselion · 2020-02-28T05:23:05Z

Project/Assets/ML-Agents/Examples/SharedAssets/Scripts/SensorBase.cs

@@ -1,7 +1,9 @@
-namespace MLAgents.Sensors
+using MLAgents.Sensors;


note: moved out of com.unity.ml-agents

chriselion · 2020-02-28T05:24:14Z

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

-            throw new UnityAgentsException(
-                "The Heuristic method was not implemented for the Agent on the " +
-                $"{gameObject.name} GameObject.");
+            return null;


If we want to keep this usable without inheriting, I think we need to remove this exception. Will to be overruled though.

Why do we need to remove it? If Heuristic is called but not implemented, an error should be raised. Or should be make a Heuristic component?
Other question, is it okay for it to be null? Does that break something down the line?

How about a log warning instead of an exception?

is it okay for it to be null?

I think that would depend on the AgentAction implementation.

surfnerd

cool!

chriselion · 2020-02-29T01:09:30Z

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

-                $"{gameObject.name} GameObject.");
+            Debug.LogWarning("Heuristic method called but not implemented. Return placeholder actions.");
+            var param = m_PolicyFactory.brainParameters;
+            var actionSize = param.vectorActionSpaceType == SpaceType.Continuous ?


Feels like this should be a property on brainParameters since we do it in ResetData() too :

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

Lines 443 to 452 in 78a218b

if (param.vectorActionSpaceType == SpaceType.Continuous)

{

m_Action.vectorActions = new float[param.vectorActionSize[0]];

m_Info.storedVectorActions = new float[param.vectorActionSize[0]];

}

else

{

m_Action.vectorActions = new float[param.vectorActionSize.Length];

m_Info.storedVectorActions = new float[param.vectorActionSize.Length];

}

int BrainParameters.ActionAllocationSize()?

IMO, vectorActionSize should be an int and in discrete control only there should be an array of ints called discreteActionBrancheSizes.

That sounds like a bigger change that's out of scope for this PR. Are you OK leaving this code as is, or do you think a utility method/property would be cleaner?

vincentpierre · 2020-03-02T18:23:22Z

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

-                $"{gameObject.name} GameObject.");
+            Debug.LogWarning("Heuristic method called but not implemented. Return placeholder actions.");
+            var param = m_PolicyFactory.brainParameters;
+            var actionSize = param.vectorActionSpaceType == SpaceType.Continuous ?


IMO, vectorActionSize should be an int and in discrete control only there should be an array of ints called discreteActionBrancheSizes.

vincentpierre · 2020-03-02T18:24:00Z

Project/Assets/ML-Agents/Examples/Basic/Scripts/BasicAgent.cs

+/// Observations are generated by the attached SensorComponent, and the actions
+/// are retrieved from the Agent.
+/// </summary>
+public class BasicAgent : MonoBehaviour


BasicController

Chris Elion added 5 commits February 27, 2020 21:04

Agent not abstract, use SensorComponent

62e4618

comments

b7f9d12

move SensorBase to examples

ab64547

undo ProjectVersion

e98fd08

changelog

1d2866c

chriselion commented Feb 28, 2020

View reviewed changes

chriselion requested review from vincentpierre, mmattar and surfnerd February 28, 2020 05:24

surfnerd approved these changes Feb 28, 2020

View reviewed changes

warn and 0 array for default Heuristic

6ecf8ed

chriselion commented Feb 29, 2020

View reviewed changes

vincentpierre approved these changes Mar 2, 2020

View reviewed changes

rename BasicAgent to BasicController

f0d2ddf

chriselion merged commit 1073ee6 into master Mar 2, 2020

delete-merged-branch bot deleted the develop-agent-non-abstract branch March 2, 2020 19:21

chriselion mentioned this pull request Mar 7, 2020

remove duplicate version of SensorBase #3588

Merged

10 tasks

github-actions bot locked as resolved and limited conversation to collaborators May 15, 2021

		@@ -1,7 +1,9 @@
		namespace MLAgents.Sensors
		using MLAgents.Sensors;

	if (param.vectorActionSpaceType == SpaceType.Continuous)
	{
	m_Action.vectorActions = new float[param.vectorActionSize[0]];
	m_Info.storedVectorActions = new float[param.vectorActionSize[0]];
	}
	else
	{
	m_Action.vectorActions = new float[param.vectorActionSize.Length];
	m_Info.storedVectorActions = new float[param.vectorActionSize.Length];
	}

[change] Make Agent non-abstract, update Basic scene. #3528

[change] Make Agent non-abstract, update Basic scene. #3528

Uh oh!

Conversation

chriselion commented Feb 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed change(s)

Useful links (Github issues, JIRA tickets, ML-Agents forum threads etc.)

Types of change(s)

Checklist

Other comments

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

surfnerd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chriselion commented Feb 28, 2020 •

edited

Loading