Brains as Scriptable Objects #1250

vincentpierre · 2018-09-21T02:45:36Z

Abstract

Make the Brain class inherit from ScriptableObject rather than MonoBehaviour. Make development of Brains easier for the developer.

Background

The current way to create, maintain and reuse Brains is laborious. A lot of this friction comes from the fact that they inherit from MonoBehaviour and must therefore be attached to a GameObject in the scene to be used. Scriptable objects are assets meaning that they exist outside of the scene. This means that they could be shared across scenes, be linked to Agent’s prefabs.

Goals

Remove the need to have a Brain GameObject in the Environment Scene
Maintain all current functionalities through simple UI
Design a clear API for Agents and Academy to interact with Brains

Non-Goals

Create new functionalities
Create complex UI

Plan

Ported most functionalities, still need to : - Documentation - Add Comments - Custom drawer for BrainParameters - Fix the UnitTests - Review Functionalities

…n the Protobuf objects

…c into LearningBrain and Training Hub

awjuliani · 2018-09-24T22:01:25Z

UnitySDK/Assets/ML-Agents/Editor/BrainParametersDrawer.cs

+    [CustomPropertyDrawer(typeof(BrainParameters))]
+    public class BrainParametersDrawer : PropertyDrawer
+    {
+//        private BrainParameters _bp;


Remove dead line

awjuliani · 2018-09-24T22:01:54Z

UnitySDK/Assets/ML-Agents/Editor/BrainParametersDrawer.cs

+        public override void OnGUI(Rect position, SerializedProperty property, GUIContent label)
+        {
+
+//            CheckInitialize(property, label);


Remove these dead lines as well.

awjuliani · 2018-09-24T22:03:21Z

UnitySDK/Assets/ML-Agents/Editor/TrainingHubDrawer.cs

+
+        public override void OnGUI(Rect position, SerializedProperty property, GUIContent label)
+        {
+


Remove extra spaces.

awjuliani · 2018-09-24T22:09:08Z

Just finished a look through the code. Only suggestions are minor cosmetic. Also tested the functionality by creating two brains for 3DBall, and using one to train. Biggest feedback is around usability. As mentioned offline, having a way to distinguish the type of a brain from the editor would be nice. Perhaps we can even use custom icons for different brain types? The other usability issue is that we have to re-input the brain parameters for each brain we create for a given agent. I know we want to eventually do this automatically by creating the brain from the agent, but I wonder if there is some intermediate solution, such as creating a new brain based on an existing brain?

…Technologies/ml-agents into develop-scriptable-brains

mmattar · 2018-09-28T03:59:12Z

UnitySDK/Assets/ML-Agents/Editor/BrainEditor.cs

+        /// </summary>
+        /// <param name="source">The BrainParameters that will be copied</param>
+        /// <param name="target">The BrainParameters that will be overwritten</param>
+        private static void DeepCopyBrainParametersFrom(


Remove From

mmattar · 2018-09-28T03:59:29Z

UnitySDK/Assets/ML-Agents/Editor/BrainEditor.cs

+        /// <param name="source">The BrainParameters that will be copied</param>
+        /// <param name="target">The BrainParameters that will be overwritten</param>
+        private static void DeepCopyBrainParametersFrom(
+            BrainParameters source, ref BrainParameters target)


source and destination

mmattar · 2018-09-28T04:02:14Z

UnitySDK/Assets/ML-Agents/Editor/BrainEditor.cs

        }

        public override void OnInspectorGUI()
        {
            var brain = (Brain) target;
-            CopyBrainParametersFrom(brain);
+            var brainToCopy = EditorGUILayout.ObjectField("Copy Brain Parameters from : ", null,


var brainToCopy = EditorGUILayout.ObjectField( "Copy Brain Parameters from : ", null, typeof(Brain), false) as Brain;

mmattar · 2018-09-28T04:02:42Z

UnitySDK/Assets/ML-Agents/Editor/BrainEditor.cs

            var serializedBrain = serializedObject;
            serializedBrain.Update(); 
            EditorGUILayout.PropertyField(serializedBrain.FindProperty("brainParameters"), true);
            serializedBrain.ApplyModifiedProperties();

+            // Draws an horizontal thick line


UnitySDK/Assets/ML-Agents/Editor/BroadcastHubDrawer.cs

mmattar · 2018-09-28T04:22:25Z

UnitySDK/Assets/ML-Agents/Editor/ResetParameterDrawer.cs

+            {
+                return;
+            }
+            if (GUI.Button(removeRect, new GUIContent("Remove Last",


indentation

mmattar · 2018-09-28T04:22:39Z

UnitySDK/Assets/ML-Agents/Editor/ResetParameterDrawer.cs

+            }
+
+            // This is the Remove button
+            if (_dict.Count == 0)


comment - if there are no parameter, hide the remove button

mmattar · 2018-09-28T04:28:09Z

UnitySDK/Assets/ML-Agents/Editor/ResetParameterDrawer.cs

+        /// </summary>
+        /// <param name="property">The SerializedProperty of the ResetParameters
+        /// to make the custom GUI for.</param>
+        /// <param name="label">The label of this property.</param>
        private void CheckInitialize(SerializedProperty property, GUIContent label)


InitializeParameter

mmattar · 2018-09-28T04:28:22Z

UnitySDK/Assets/ML-Agents/Editor/ResetParameterDrawer.cs

-
+        /// <summary>
+        /// Removes the last ResetParameter from the ResetParameters
+        /// </summary>
        private void RemoveLastItem()


REmoveLastParameter

mmattar · 2018-09-28T04:28:32Z

UnitySDK/Assets/ML-Agents/Editor/ResetParameterDrawer.cs

            }
        }

+        /// <summary>
+        /// Adds a new ResetParameter to the ResetParameters with a default name.
+        /// </summary>
        private void AddNewItem()


AddParameter

UnitySDK/Assets/ML-Agents/Scripts/Brain.cs

… TF# is disabled

* Modified first docs * modified the markdown, not the images * Missing doc * Replaced Internal with Learned Brain * updated the images * Addressed some comments * Renamed Training Hub to Broadcast Hub * Forgot one file * Added new images * addressing comment * Fixed some typos

mmattar

I find the term Learning Brain odd for Inference mode. Are there another options?
Some comments from earlier review are still pending.

mmattar · 2018-10-04T20:40:06Z

UnitySDK/Assets/ML-Agents/Editor/BrainEditor.cs

+                vectorObservationSize = src.vectorObservationSize,
+                numStackedVectorObservations = src.numStackedVectorObservations,
+                vectorActionSize = (int[]) src.vectorActionSize.Clone(),
+                cameraResolutions = (resolution[])src.cameraResolutions.Clone(),


add space before src.cameraResolutions.Clone(),

mmattar · 2018-10-04T20:40:24Z

UnitySDK/Assets/ML-Agents/Editor/BrainEditor.cs

+                numStackedVectorObservations = src.numStackedVectorObservations,
+                vectorActionSize = (int[]) src.vectorActionSize.Clone(),
+                cameraResolutions = (resolution[])src.cameraResolutions.Clone(),
+                vectorActionDescriptions = (string[])src.vectorActionDescriptions.Clone(),


add space before src.vectorActionDescriptions.Clone(),

mmattar · 2018-10-04T20:41:42Z

UnitySDK/Assets/ML-Agents/Editor/BroadcastHubDrawer.cs

+        private BroadcastHub _hub;
+        // The height of a line in the Unity Inspectors
+        private const float LineHeight = 17f;
+        private const float ExtraSpaceBellow = 10f;


Refactor due to typo: ExtraSpaceBellow --> ExtraSpaceBelow

mmattar · 2018-10-04T20:42:16Z

UnitySDK/Assets/ML-Agents/Editor/BroadcastHubDrawer.cs

+        private BroadcastHub _hub;
+        // The height of a line in the Unity Inspectors
+        private const float LineHeight = 17f;
+        private const float ExtraSpaceBellow = 10f;


What is this field?

UnitySDK/Assets/ML-Agents/Editor/BroadcastHubDrawer.cs

mmattar · 2018-10-04T20:53:11Z

docs/Python-API.md

    must provide dictionaries from Brain names to arrays for `action`, `memory`
-    and `value`. For example: If you have two external Brains named `brain1` and
+    and `value`. For example: If you have two training Brains named `brain1` and


training Brain?

mmattar · 2018-10-04T20:53:28Z

docs/Migrating.md

@@ -92,7 +109,7 @@ in order to ensure a smooth transition.
  with ML-Agents. For more information on using `learn.py`, see
  [here](Training-ML-Agents.md#training-with-mlagents-learn).
 * Hyperparameters for training Brains are now stored in the
-  `trainer_config.yaml` file. For more information on using this file, see
+  `Controler_config.yaml` file. For more information on using this file, see


typo: Controler_config

mmattar · 2018-10-04T20:54:06Z

docs/Migrating.md

+     ML-Agents`
+  *  Edit their `Brain Paramters` to be the same as the parameters used 
+     in the `Brain` GameObjects
+  *  Set the `Brain` property of the agents to the appropriate `Brain`


Sentence somewhat confusing ...

mmattar · 2018-10-04T20:55:26Z

docs/Learning-Environment-Design.md

 [Brains](Learning-Environment-Design-Brains.md) for details on using the
-different types of Brains. You can extend the CoreBrain class to create
-different Brain types if the four built-in types don't do what you need.
+different types of Brains. You can create new kinds of brains if the three


What other brains can we build?

I do not know, you could imagine a brain that would record all the actions perform and store them to disk for example.

mmattar · 2018-10-04T20:56:16Z

docs/Learning-Environment-Design-Brains.md

+There are 3 kinds of brains you can use:
+
+* [Learning](Learning-Environment-Learning-Brains.md) – Use a
+  **LearningBrain**  to make use of a trained model or train a new model.


double-space after LearningBrain

mmattar

Few more things

mmattar · 2018-10-05T02:28:29Z

UnitySDK/Assets/ML-Agents/Editor/PlayerBrainEditor.cs

+    public class PlayerBrainEditor : BrainEditor
+    {
+
+        public override void OnInspectorGUI()


@vincentpierre - this method is still huge, please break-up.

mmattar · 2018-10-05T02:29:42Z

UnitySDK/Assets/ML-Agents/Scripts/Academy.cs

-                            brain.gameObject.name,
-                            (CommunicatorObjects.BrainTypeProto)
-                            brain.brainType));
+                            bp.ToProto(brain.name, broadcastHub.IsControlled(brain))


indentation is off on this line and next.

mmattar · 2018-10-05T02:31:38Z

UnitySDK/Assets/ML-Agents/Scripts/BroadcastHub.cs

+
+namespace MLAgents
+{
+


remove empty line

mmattar · 2018-10-05T02:31:46Z

UnitySDK/Assets/ML-Agents/Scripts/BroadcastHub.cs

+{
+
+    [System.Serializable]
+    public class BroadcastHub


class comments

mmattar · 2018-10-05T02:32:02Z

UnitySDK/Assets/ML-Agents/Scripts/BroadcastHub.cs

+        [SerializeField]
+        private List<Brain> _brainsToControl = new List<Brain>();
+
+        public int Count


method comment

mmattar · 2018-10-05T02:32:19Z

UnitySDK/Assets/ML-Agents/Scripts/BroadcastHub.cs

+            get { return broadcastingBrains.Count; }
+        }        
+
+        public bool IsControlled(Brain b)


method comment

change b to brain

mmattar · 2018-10-05T02:32:32Z

UnitySDK/Assets/ML-Agents/Scripts/BroadcastHub.cs

+            return _brainsToControl.Contains(b);
+        }
+
+        public void SetTraining(Brain b, bool train)


method comment

change b to brain

mmattar · 2018-10-05T02:32:45Z

UnitySDK/Assets/ML-Agents/Scripts/BroadcastHub.cs

+            }
+        }
+
+        public void Clear()


method comment

mmattar · 2018-10-05T02:33:18Z

UnitySDK/Assets/ML-Agents/Scripts/HeuristicBrain.cs

+                decision.brainParameters = brainParameters;
+            }
+        }
+


remove empty line

mmattar · 2018-10-05T02:33:31Z

UnitySDK/Assets/ML-Agents/Scripts/HeuristicBrain.cs

+                    agentInfos[agent].reward,
+                    agentInfos[agent].done,
+                    agentInfos[agent].memories));
+


remove empty line

…ameters file, made a CumSum method

vincentpierre · 2018-10-05T20:25:56Z

@mmattar Comments addressed

mmattar

Couple of minor comments, otherwise, 🚢

mmattar · 2018-10-05T21:10:37Z

UnitySDK/Assets/ML-Agents/Editor/PlayerBrainEditor.cs

+        /// </summary>
+        /// <param name="serializedBrain"> The SerializedObject corresponding to the brain. </param>
+        /// <param name="brain"> The Brain of which properties are displayed. </param>
+        private static void DrawContinuousKeyMapping(SerializedObject serializedBrain,


Either:

private static void DrawContinuousKeyMapping(SerializedObject serializedBrain, PlayerBrain brain)

OR

private static void DrawContinuousKeyMapping( SerializedObject serializedBrain, PlayerBrain brain)

mmattar · 2018-10-05T21:11:39Z

UnitySDK/Assets/ML-Agents/Scripts/Utilities.cs

+        /// If the input is [a, b, c], the result will be [0, a, a+b, a+b+c]
+        /// </summary>
+        /// <returns> The cumulative sum of the input array.</returns>
+        public static int[] CumSum(int [] array)


vincentpierre added 13 commits September 13, 2018 10:58

Initial Commit

c5500b9

Ported most functionalities, still need to : - Documentation - Add Comments - Custom drawer for BrainParameters - Fix the UnitTests - Review Functionalities

Added Custom Drawer for the Brain Parameters

a1ac493

Improvements to the HubDrawer

7390d47

Modified the Brain Editors

d02780d

Minor bug fixes and UI changes

189428d

Modified the Help Boxes of the Drawers

2ec51ab

Modified Brain class, renamed Initialize and made DecideAction virtual

34cba7a

Fix the UnityTests

733e7e7

Simpler Brain creation menu

798e2e7

Renamed Internal Brain to Learning Brain

da66b24

modified the parameters to remove reference to External or Internal i…

c6c54bd

…n the Protobuf objects

Updated the protobuf generated files

04c76b5

Fix the Pytests

43972d2

vincentpierre requested a review from awjuliani September 21, 2018 02:45

vincentpierre and others added 7 commits September 20, 2018 19:50

Removed the graph scope from the Learning Brain

0ec70f9

Resolved the merge conflicts

deab466

cleaner logic than try catch

976ee17

Removed the isExternal field of the brain and put the isTraining logi…

cfd39b3

…c into LearningBrain and Training Hub

Modified how the Brain finds the Academy

7585cc9

Removed refences to CoreBrain

aa773c4

Fix import bug

ed1d03c

awjuliani reviewed Sep 24, 2018

View reviewed changes

vincentpierre added 5 commits September 24, 2018 16:03

Addressed some comments

6a3b023

Merge branch 'develop-scriptable-brains' of https://github.com/Unity-…

2e60124

…Technologies/ml-agents into develop-scriptable-brains

Remove useless imports

48337df

Added nice icons for the brains

5523654

Added a feature to deepcopy brain parameters between brains

972a15f

xiaomaogy mentioned this pull request Sep 27, 2018

Editor crashes every time I try to play using an Internal brain type #918

Closed

vincentpierre added 6 commits September 27, 2018 14:33

Refactored the BrainParameters drawer

9e14dd0

Addressed offline comments

25edfe4

Comments on the ResetParametersDrawer

cca89d3

refactired the BroadcastHubDrawer

192cf23

Adding comments

af6cdd3

Add comment on the horizontal bar

51e09a9

mmattar suggested changes Sep 28, 2018

View reviewed changes

vincentpierre added 3 commits September 28, 2018 09:31

Added new comments

a8bbf58

Refactor of the Editors

c124350

minor changes

b032767

vincentpierre commented Sep 28, 2018

View reviewed changes

UnitySDK/Assets/ML-Agents/Scripts/Brain.cs Show resolved Hide resolved

vincentpierre added 2 commits October 1, 2018 10:38

Added documentation, Fixed a bug in the tenporary internal brain when…

2ee17aa

… TF# is disabled

mmattar suggested changes Oct 4, 2018

View reviewed changes

vincentpierre added 6 commits October 4, 2018 15:02

address comments on the code

eb01b12

addressed comments on documents

f1d7ca1

Merge branch 'develop' into develop-scriptable-brains

81d225d

Resolving conflicts with develop

1d3a0d3

Resolving conflicts on the Learning Brain

645be61

Minor tweaks

5509187

mmattar suggested changes Oct 5, 2018

View reviewed changes

vincentpierre added 2 commits October 5, 2018 09:53

Addressed Comments

62595eb

Created a Clone method in the BrainParameters, created a new BrainPar…

5fd9844

…ameters file, made a CumSum method

mmattar approved these changes Oct 5, 2018

View reviewed changes

Added Unit Test

383158b

vincentpierre merged commit 253b3bb into develop Oct 5, 2018

vincentpierre deleted the develop-scriptable-brains branch October 5, 2018 21:40

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


		public override void OnGUI(Rect position, SerializedProperty property, GUIContent label)
		{


		namespace MLAgents
		{

Brains as Scriptable Objects #1250

Brains as Scriptable Objects #1250

Conversation

vincentpierre commented Sep 21, 2018 • edited

Abstract

Background

Goals

Non-Goals

Plan

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awjuliani commented Sep 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmattar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmattar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vincentpierre commented Oct 5, 2018

mmattar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vincentpierre commented Sep 21, 2018 •

edited