[MLA-1135] Physics sensors - optional reference body #4276

chriselion · 2020-07-28T00:36:24Z

Proposed change(s)

Add optional "virtual root" GameObject to the RigidBodyPoseExtractor. If set, this acts as the reference position, and the root RB is parented to this.
Update the Crawler scene to use a RigidBodySensorComponent, and remove related code from CollectObservations.
Enable toggling (in code) specific bodies in the PoseExtractor. The root body is disabled by default, since that observations for that are always the identity. Need to enable setting these for other bodies in a custom inspector (e.g. disabling the eyeballs in the worm).
Changed pose access to be an IEnumerable, to make skipping disabled poses transparent.

Followup:

retrain UR3 model, because the layout of the observations changed (maybe not a problem if not using the velocities)
Add VirtualRoot to ArticulationBodyPoseExtractor (not needed by anything at the moment).

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

https://jira.unity3d.com/browse/MLA-1135
https://jira.unity3d.com/browse/MLA-1166
https://jira.unity3d.com/browse/MLA-1167

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

…body

chriselion · 2020-07-30T18:36:35Z

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


 [assembly: InternalsVisibleTo("Unity.ML-Agents.Editor.Tests")]
 [assembly: InternalsVisibleTo("Unity.ML-Agents.Editor")]
+[assembly: InternalsVisibleTo("Unity.ML-Agents.Extensions")]


Possibly controversial; I added this to get timers. But it's something we could use in the future, e.g. public actuators in extensions, but keep IActuator internal.

chriselion · 2020-07-30T20:00:52Z

Project/Assets/ML-Agents/Examples/Crawler/Scripts/CrawlerAgent.cs


-        //Get velocities in the context of our orientation cube's space
-        //Note: You can get these velocities in world space as well but it may not train as well.
-        sensor.AddObservation(orientationCube.transform.InverseTransformDirection(bp.rb.velocity));


Corresponds to model space velocity

chriselion · 2020-07-30T20:01:41Z

Project/Assets/ML-Agents/Examples/Crawler/Scripts/CrawlerAgent.cs

-        //Get velocities in the context of our orientation cube's space
-        //Note: You can get these velocities in world space as well but it may not train as well.
-        sensor.AddObservation(orientationCube.transform.InverseTransformDirection(bp.rb.velocity));
-        sensor.AddObservation(orientationCube.transform.InverseTransformDirection(bp.rb.angularVelocity));


No direct correspondence, but removing it didn't seem to hurt much. I also need to convince myself that you can just transform angular velocities like that.

chriselion · 2020-07-30T20:01:57Z

Project/Assets/ML-Agents/Examples/Crawler/Scripts/CrawlerAgent.cs

-        sensor.AddObservation(orientationCube.transform.InverseTransformDirection(bp.rb.angularVelocity));
-
-        //Get position relative to hips in the context of our orientation cube's space
-        sensor.AddObservation(orientationCube.transform.InverseTransformDirection(bp.rb.position - body.position));


Corresponds to model space positions

chriselion · 2020-07-30T20:02:13Z

Project/Assets/ML-Agents/Examples/Crawler/Scripts/CrawlerAgent.cs

-
        if (bp.rb.transform != body)
        {
-            sensor.AddObservation(bp.rb.transform.localRotation);


Corresponds to local space rotations

are these decisions that could have been made with the help of the tool @andrewcoh was talking about for saliency?

In this case, I removed everything from CollectObservations that had an equivalent in the sensor.

In general, we could turn everything on in the sensor, run the tool to see what's important, and then disable unimportant stuff. However:

It could be tedious to attribute a particular index in the tool to a specific option in the sensor.

The tool might be too fine-grained. For example, if it said that the w component of one quaternion wasn't relevant, we couldn't just turn that one off. Or if local space velocities was only relevant for one body, we couldn't keep that one but disable the rest.

com.unity.ml-agents.extensions/Runtime/Sensors/PhysicsSensorSettings.cs

…body

chriselion · 2020-07-30T22:23:34Z

Project/Assets/ML-Agents/Examples/Crawler/Prefabs/DynamicPlatform.prefab

      propertyPath: targetToLookAt
      value: 
      objectReference: {fileID: 7802320107249901494}
+    - target: {fileID: 4622120667686875944, guid: 0456c89e8c9c243d595b039fe7aa0bf9,


Hmm, don't think I meant to override this. Now I need to double check the model settings

chriselion · 2020-07-30T23:02:56Z

Training times and rewards:

master

Job Id	Scene Name	Expected Reward	Mean Reward	Max Reward	Inference Reward	Training Time Hours
crawdy-ppo	crawlerdynamictarget	400	810.8445	813.6515	645.7593	10.79
crawsta-ppo	crawlerstatictarget	2000	2069.4256	2073.8216	1988.6075	11.01

this branch

Job Id	Scene Name	Expected Reward	Mean Reward	Max Reward	Inference Reward	Training Time Hours
crawdy-ppo	crawlerdynamictarget	400	801.0141	821.2339	832.2614	10.79
crawsta-ppo	crawlerstatictarget	2000	1789.9456	1863.3114	1777.4036	13.13

I'm going to retrain the crawlerstatictarget scene again tonight; I got some prefabs mixed up so it was using different settings (added local space velocities too). Hopefully that will bring the training time down and the reward up a bit.

Also note that for crawlerstatictarget, master is above the expected reward and below on this branch. However, the reward on master is the outlier (unusually good) if we look at the last few weeks of data:

com.unity.ml-agents.extensions/Runtime/Sensors/PoseExtractor.cs

com.unity.ml-agents.extensions/Runtime/Sensors/PhysicsSensorSettings.cs

vincentpierre · 2020-07-30T23:27:37Z

com.unity.ml-agents.extensions/Runtime/Sensors/PoseExtractor.cs

+        /// </summary>
+        /// <param name="index"></param>
+        /// <param name="val"></param>
+        public void SetPoseEnabled(int index, bool val)


SetPoseEnabled is only used to make sure the Virtual Root is not being observed right? I did not see being used elsewhere. Making it public might be dangerous, dot sure what would happen if the user called SetPoseEnabled in the middle of training.
Also, nothing to do with Enabled / Disabled GameObjects right?

At the moment, it's only used on the root (virtual or otherwise). But I was to add a custom Editor to expose it per-body. For example, on the Worm scene, the eyes are actually rigid bodies constrained to the head, with locked linear motion and free angular motion:

We would basically be wasting observations on those, so I'd like to be able to filter them out.

For now, I'd prefer to keep things more accessible, and tighten up the visibility if we move to the core package.

Correct, nothing to do Enabled / Disabled GameObjects. I hadn't thought about that - is there a better name that would make this more obvious?

surfnerd · 2020-07-30T23:38:37Z

com.unity.ml-agents.extensions/Runtime/Sensors/PoseExtractor.cs

        /// </summary>
        /// <param name="parentIndices"></param>
-        protected void SetParentIndices(int[] parentIndices)
+        protected void Setup(int[] parentIndices)


should the assumption be checked since this is a protected method on a public class?

surfnerd · 2020-07-30T23:42:29Z

com.unity.ml-agents.extensions/Runtime/Sensors/ArticulationBodyPoseExtractor.cs


        /// <inheritdoc/>
-        protected override Vector3 GetLinearVelocityAt(int index)
+        protected internal override Vector3 GetLinearVelocityAt(int index)


why the internal addition?

Wanted to be able to assert on it

ml-agents/com.unity.ml-agents.extensions/Tests/Editor/Sensors/RigidBodyPoseExtractorTests.cs

Line 67 in 7e75f12

Assert.AreEqual(rb1.velocity, poseExtractor.GetLinearVelocityAt(0));

surfnerd · 2020-07-30T23:50:51Z

com.unity.ml-agents.extensions/Tests/Editor/Sensors/RigidBodyPoseExtractorTests.cs

+            //   * go2
+            //     - rb2
+            //     - joint
+            var virtualRoot = new GameObject("I am vroot");


Chris Elion added 8 commits July 27, 2020 15:47

optional virtual root body

e3adeb6

fix unit tests

7e75f12

add VirtualRoot to sensorComponent

8fd680a

use sensor in prefab, reduce manual observations

95465af

Merge remote-tracking branch 'origin/master' into MLA-1135-reference-…

657aa86

…body

WIP toggle poses

5d7f101

mask for poses, exclude root

0b62125

Merge remote-tracking branch 'origin/master' into MLA-1135-reference-…

fb24874

…body

chriselion marked this pull request as ready for review July 29, 2020 18:52

chriselion marked this pull request as draft July 29, 2020 18:52

Update README.md

60c4ae7

chriselion commented Jul 30, 2020

View reviewed changes

Chris Elion added 2 commits July 30, 2020 12:51

cleanup

03e0e3b

update nn files

5e3829c

chriselion requested review from vincentpierre and surfnerd July 30, 2020 19:59

chriselion marked this pull request as ready for review July 30, 2020 20:00

chriselion requested a review from Hunter-Unity July 30, 2020 20:00

chriselion commented Jul 30, 2020

View reviewed changes

remove commented code

3e27b22

chriselion commented Jul 30, 2020

View reviewed changes

com.unity.ml-agents.extensions/Runtime/Sensors/PhysicsSensorSettings.cs Outdated Show resolved Hide resolved

Chris Elion added 2 commits July 30, 2020 15:16

remove unused imports, fix articulation body test

0243155

Merge remote-tracking branch 'origin/master' into MLA-1135-reference-…

ce254bb

…body

chriselion commented Jul 30, 2020

View reviewed changes

fix prefabs (need to retrain static model)

10755c3

surfnerd reviewed Jul 30, 2020

View reviewed changes

com.unity.ml-agents.extensions/Runtime/Sensors/PoseExtractor.cs Outdated Show resolved Hide resolved

surfnerd reviewed Jul 30, 2020

View reviewed changes

com.unity.ml-agents.extensions/Runtime/Sensors/PhysicsSensorSettings.cs Outdated Show resolved Hide resolved

vincentpierre reviewed Jul 30, 2020

View reviewed changes

rename pose and velocity accessors

6128991

surfnerd reviewed Jul 30, 2020

View reviewed changes

surfnerd approved these changes Jul 30, 2020

View reviewed changes

Chris Elion added 4 commits July 30, 2020 17:10

check valid parent index

cc14a68

wrap check in DEBUG

6e6999c

fix root index

2ab589b

use local velocities for static platform

f6c1a75

chriselion merged commit fe7d147 into master Jul 31, 2020

delete-merged-branch bot deleted the MLA-1135-reference-body branch July 31, 2020 17:29

chriselion changed the title ~~[WIP] [MLA-1135] Physics sensors - optional reference body~~ [MLA-1135] Physics sensors - optional reference body Jul 31, 2020

github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2021

[MLA-1135] Physics sensors - optional reference body #4276

[MLA-1135] Physics sensors - optional reference body #4276

Uh oh!

Conversation

chriselion commented Jul 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chriselion commented Jul 30, 2020

Uh oh!

Uh oh!

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!

Uh oh!

chriselion commented Jul 28, 2020 •

edited

Loading