Stats SideChannel (for custom TensorBoard metrics) #3660

chriselion · 2020-03-19T23:06:56Z

Proposed change(s)

Add a SideChannel that can be used to sent stats from C# to python, and later add those to TensorBoard.
Also adds methods to the Academy to find an instance (or instances) of specific SideChannel subclass (inspired by MonoBehaviour.GetComponent(s))

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

https://jira.unity3d.com/browse/MLA-675
https://forum.unity.com/threads/custom-graphs-in-tensorboard.843865/
#2984
https://github.com/Unity-Technologies/ml-agents/pull/3594/files

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

Sample graph of the number of time the 3DBallAgent drops the ball

chriselion · 2020-03-19T23:08:22Z

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

+        /// </summary>
+        /// <typeparam name="T"></typeparam>
+        /// <returns></returns>
+        public T GetSideChannel<T>() where T: SideChannel


@vincentpierre @surfnerd I think we had talked about this before. It seems like a clean way to get access to sidechannels without adding a custom method to Academy for each one.

I can replace Academy.FloatProperties with this later if we're happy with it.

Let's replace Academy.Instance.FloatProperty then. Do you want to do it in this PR or make another one?

Let's do it in another PR.

https://jira.unity3d.com/browse/MLA-799

com.unity.ml-agents/Runtime/SideChannels/EngineConfigurationChannel.cs

ml-agents/mlagents/trainers/agent_processor.py

vincentpierre

I wonder how you got the graph for the number of times the ball got dropped. Are you keeping trak of the total number in C#?
I think the PR looks good but we need to make a decision regarding GetSideChannel before moving forward. I think it is a good idea but I think having 2 methods (one that returns a list and one that returns an instance) is a bit confusing. There is no real notion of "first" with side channel and the GetSideChannel method introduces one.

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

com.unity.ml-agents/Runtime/SideChannels/EngineConfigurationChannel.cs

mmattar · 2020-03-20T17:20:11Z

com.unity.ml-agents/Runtime/SideChannels/StatsSideChannel.cs

+using System;
+namespace MLAgents.SideChannels
+{
+    public class StatsSideChannel : SideChannel


Would TensorBoardMetricsSideChannel be a more appropriate name?

It won't just be for TB - with this change it'll show up anywhere Python stats are written (e.g. CSV, timers, and perhaps later the Console).

Yeah, I kept the name generic but called out TensorBoard in the comments. I'll also add an example of this in the "Using TensorBoard" docs.

Ok, let's then document the different ways that adding a stat here will bubble up on the Python side. I find the term stats odd since TB refers to metrics, but I also can't see a better alternative (esp that it does align with timer stats)

I moved the comments below up here and clarified a bit.

com.unity.ml-agents/Runtime/SideChannels/StatsSideChannel.cs

mmattar · 2020-03-20T17:21:50Z

com.unity.ml-agents/Runtime/SideChannels/StatsSideChannel.cs

+        /// </summary>
+        /// <param name="key"></param>
+        /// <param name="value"></param>
+        public void AddStat(string key, float value)


If we rename the class based on the method above - this ought to be renamed to AddMetricValue

mmattar · 2020-03-20T17:22:37Z

ml-agents/mlagents/trainers/env_manager.py

@@ -108,4 +109,11 @@ def _process_step_infos(self, step_infos: List[EnvironmentStep]) -> int:
                        name_behavior_id, ActionInfo.empty()
                    ),
                )
+
+                # In order to prevent conflicts between multiple environments,


This would be helpful documentation in the C# class too.

I am rather hesitant on this. Are the stats only usable when training? Is it not possible for users to get the C# stats when using the LL-API? If it is possible to use it with LL-API, then C# should not assume how the stats are consumed (at least not without a warning of type : "When using ml-agents training ™️, the stats will...")

I moved it to a comment on StatAggregationMethod.MostRecent:

To avoid conflicts between multiple environments, the ML Agents environment will only
keep stats from worker index 0.

mmattar · 2020-03-20T17:24:41Z

com.unity.ml-agents/Runtime/Communicator/RpcCommunicator.cs

@@ -537,6 +537,43 @@ public void UnregisterSideChannel(SideChannel sideChannel)
            }
        }

+        /// <summary>


Do we expect the implementation of these two methods to be different for different ICommunicator implementations?

The implementation will depend on how the SideChannels are stored by the ICommunicator implementation. There's no guarantee that another implementation would use a UUID->SideChannel dictionary.

We could make this an internal static method in SideChannel.cs if you think it's likely to be reusable.

ml-agents-envs/mlagents_envs/side_channel/stats_side_channel.py

ml-agents/mlagents/trainers/agent_processor.py

ervteng · 2020-03-20T18:17:48Z

ml-agents/mlagents/trainers/tests/test_subprocess_env_manager.py

@@ -147,7 +147,7 @@ def test_advance(self, mock_create_worker, external_brains_mock, step_mock):
        env_manager.set_agent_manager(brain_name, agent_manager_mock)

        step_info_dict = {brain_name: Mock()}
-        step_info = EnvironmentStep(step_info_dict, 0, action_info_dict)
+        step_info = EnvironmentStep(step_info_dict, 0, action_info_dict, {})


We probably should put something in the dictionary so that the method gets tested, and similarly run set_environment_stats in the test_agent_processor.py tests

com.unity.ml-agents/Runtime/Communicator/RpcCommunicator.cs

chriselion · 2020-03-24T00:48:44Z

I wonder how you got the graph for the number of times the ball got dropped. Are you keeping trak of the total number in C#?

I hacked that as a static for testing :)

chriselion · 2020-03-24T00:49:31Z

Mind taking another look at this? It changed a bit, mostly around the multiple aggregation types.

chriselion · 2020-03-24T00:52:53Z

ml-agents-envs/mlagents_envs/side_channel/stats_side_channel.py

+
+
+# Determines the behavior of how multiple stats within the same summary period are combined.
+class StatsAggregationMethod(Enum):


Is there a better place for this? It needs to live in ml-agents-envs (unless we move the SideChannel?) but it feels strange importing mlagents_envs.side_channel.stats_side_channel in a lot of places just to get the enum. @ervteng thoughts?

Hmm, this actually seems the least obtrusive since the StatsReporter lives in mlagents. The other option would be in the stats.py, but then we'd have to move that to mlagents_envs.

vincentpierre

👍 Should we add stats to one of our environments? For example in FoodCollector, amount of food collected or in crawler, total distance travelled?
I think this would help us make sure it is behaving as expected.

vincentpierre · 2020-03-24T17:33:24Z

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

+        /// </summary>
+        /// <typeparam name="T"></typeparam>
+        /// <returns></returns>
+        public T GetSideChannel<T>() where T: SideChannel


Let's replace Academy.Instance.FloatProperty then. Do you want to do it in this PR or make another one?

chriselion · 2020-03-24T17:55:16Z

Project/Assets/ML-Agents/Examples/FoodCollector/Scripts/FoodCollectorSettings.cs

@@ -44,5 +45,6 @@ void ClearObjects(GameObject[] objects)
    public void Update()
    {
        scoreText.text = $"Score: {totalScore}";
+        Academy.Instance.GetSideChannel<StatsSideChannel>().AddStat("TotalScore", totalScore);


@vincentpierre Added to FoodCollector here. If we want to add to other examples, let's do so in a followup PR.

Should we store the StatsSideChannel in a private property to avoid calling GetSideChannel every update?

Yeah, probably - I'm not to worried about the performance, but we should set a good example...

…csharp

chriselion · 2020-03-25T00:29:46Z

FWIW here's FoodCollector with the contribute flag set to true everywhere:

Stats side channel

1dfa14e

chriselion commented Mar 19, 2020

View reviewed changes

com.unity.ml-agents/Runtime/SideChannels/EngineConfigurationChannel.cs Show resolved Hide resolved

chriselion commented Mar 19, 2020

View reviewed changes

ml-agents/mlagents/trainers/agent_processor.py Outdated Show resolved Hide resolved

chriselion requested review from vincentpierre, ervteng and surfnerd March 19, 2020 23:17

fix python tests

21de227

vincentpierre approved these changes Mar 20, 2020

View reviewed changes

com.unity.ml-agents/Runtime/Academy.cs Show resolved Hide resolved

mmattar reviewed Mar 20, 2020

View reviewed changes

vincentpierre reviewed Mar 20, 2020

View reviewed changes

ml-agents-envs/mlagents_envs/side_channel/stats_side_channel.py Show resolved Hide resolved

vincentpierre reviewed Mar 20, 2020

View reviewed changes

ml-agents/mlagents/trainers/agent_processor.py Outdated Show resolved Hide resolved

vincentpierre reviewed Mar 20, 2020

View reviewed changes

ml-agents/mlagents/trainers/agent_processor.py Outdated Show resolved Hide resolved

ervteng reviewed Mar 20, 2020

View reviewed changes

feedback, chooise aggregation method

8d8a382

surfnerd reviewed Mar 23, 2020

View reviewed changes

com.unity.ml-agents/Runtime/Communicator/RpcCommunicator.cs Show resolved Hide resolved

surfnerd reviewed Mar 23, 2020

View reviewed changes

com.unity.ml-agents/Runtime/Communicator/RpcCommunicator.cs Show resolved Hide resolved

Chris Elion added 2 commits March 23, 2020 17:04

clean docs

c98edf8

changelog and unit tests

80a3e74

chriselion requested review from vincentpierre, ervteng, surfnerd and mmattar March 24, 2020 00:48

chriselion commented Mar 24, 2020

View reviewed changes

vincentpierre approved these changes Mar 24, 2020

View reviewed changes

chriselion commented Mar 24, 2020

View reviewed changes

Chris Elion added 2 commits March 24, 2020 11:49

Merge remote-tracking branch 'origin/master' into MLA-675-stats-from-…

32f08b4

…csharp

add note in docs, clean up FoodCollector usage.

9e93bd4

chriselion merged commit 399ad3c into master Mar 25, 2020

delete-merged-branch bot deleted the MLA-675-stats-from-csharp branch March 25, 2020 00:47

chriselion mentioned this pull request Mar 25, 2020

Custom tensorboard observations #3594

Closed

1 task

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



		# Determines the behavior of how multiple stats within the same summary period are combined.
		class StatsAggregationMethod(Enum):

Stats SideChannel (for custom TensorBoard metrics) #3660

Stats SideChannel (for custom TensorBoard metrics) #3660

Uh oh!

Conversation

chriselion commented Mar 19, 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!

Uh oh!

Uh oh!

vincentpierre left a comment

Choose a reason for hiding this comment

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!

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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

chriselion commented Mar 24, 2020

Uh oh!

chriselion commented Mar 24, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vincentpierre 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!

chriselion commented Mar 19, 2020 •

edited

Loading