Merge pull request #195 from Unity-Technologies/ordered-tag-queries

Fixed issues with determinism in Unity Simulation

com.unity.perception/CHANGELOG.md

 
 ### Added
 
+Added Register() and Unregister() methods to the RandomizerTag API so users can implement RandomizerTag compatible GameObject caching
+
+
+Switched accessibility of scenario MonoBehaviour lifecycle functions (Awake, Start, Update) from private to protected to enable users to define their own overrides when deriving the Scenario class.
+
+The GameObjectOneWayCache has been made public for users to cache GameObjects within their own custom Randomizers.
 
 ### Deprecated
 
 
-Fixed the math offsetting the iteration index of each Unity Simulation instance directly after they deserialize their app-params
+Fixed the math offsetting the iteration index of each Unity Simulation instance directly after they deserialize their app-params.
+
+The RandomizerTagManager now uses a LinkedHashSet data structure to register tags to preserve insertion order determinism in Unity Simulation.
+
+GameObjectOneWayCache now correctly registers and unregisters RandomizerTags on cached GameObjects.
 
 ## [0.7.0-preview.1] - 2021-02-01
 

com.unity.perception/Runtime/Randomization/Randomizers/RandomizerExamples/Utilities/GameObjectOneWayCache.cs

     /// Facilitates object pooling for a pre-specified collection of prefabs with the caveat that objects can be fetched
     /// from the cache but not returned. Every frame, the cache needs to be reset, which will return all objects to the pool
     /// </summary>
-    class GameObjectOneWayCache
+    public class GameObjectOneWayCache
-        // Objects will reset to this origin when not being used
-        List<GameObject>[] m_InstantiatedObjects;
+        List<CachedObjectData>[] m_InstantiatedObjects;
+
+        /// <summary>
+        /// The number of active cache objects in the scene
+        /// </summary>
+        /// <summary>
+        /// Creates a new GameObjectOneWayCache
+        /// </summary>
+        /// <param name="parent">The parent object all cached instances will be parented under</param>
+        /// <param name="prefabs">The prefabs to cache</param>
-            m_InstantiatedObjects = new List<GameObject>[prefabs.Length];
+            m_InstantiatedObjects = new List<CachedObjectData>[prefabs.Length];
             m_NumObjectsActive = new int[prefabs.Length];
 
             var index = 0;
                 m_InstanceIdToIndex.Add(instanceId, index);
-                m_InstantiatedObjects[index] = new List<GameObject>();
+                m_InstantiatedObjects[index] = new List<CachedObjectData>();
+        /// <summary>
+        /// Retrieves an existing instance of the given prefab from the cache if available.
+        /// Otherwise, instantiate a new instance of the given prefab.
+        /// </summary>
+        /// <param name="prefab"></param>
+        /// <returns></returns>
+        /// <exception cref="ArgumentException"></exception>
-            {
-            }
 
             ++NumObjectsActive;
             if (m_NumObjectsActive[index] < m_InstantiatedObjects[index].Count)
-                return nextInCache;
-            }
-            else
-            {
-                ++NumObjectsInCache;
-                var newObject = Object.Instantiate(prefab, m_CacheParent);
-                ++m_NumObjectsActive[index];
-                m_InstantiatedObjects[index].Add(newObject);
-                return newObject;
+                foreach (var tag in nextInCache.randomizerTags)
+                    tag.Register();
+                return nextInCache.instance;
+
+            ++NumObjectsInCache;
+            var newObject = Object.Instantiate(prefab, m_CacheParent);
+            ++m_NumObjectsActive[index];
+            m_InstantiatedObjects[index].Add(new CachedObjectData(newObject));
+            return newObject;
+        /// <summary>
+        /// Return all active cache objects back to an inactive state
+        /// </summary>
         public void ResetAllObjects()
         {
             using (s_ResetAllObjectsMarker.Auto())
                 {
                     m_NumObjectsActive[i] = 0;
-                    foreach (var obj in m_InstantiatedObjects[i])
+                    foreach (var cachedObjectData in m_InstantiatedObjects[i])
-                        obj.transform.localPosition = new Vector3(10000, 0, 0);
+                        cachedObjectData.instance.transform.localPosition = new Vector3(10000, 0, 0);
+                        foreach (var tag in cachedObjectData.randomizerTags)
+                            tag.Unregister();
+            }
+        }
+
+        struct CachedObjectData
+        {
+            public GameObject instance;
+            public RandomizerTag[] randomizerTags;
+
+            public CachedObjectData(GameObject instance)
+            {
+                this.instance = instance;
+                randomizerTags = instance.GetComponents<RandomizerTag>();
             }
         }
     }

com.unity.perception/Runtime/Randomization/Randomizers/RandomizerTag.cs

     {
         RandomizerTagManager tagManager => RandomizerTagManager.singleton;
 
-        void Awake()
+        /// <summary>
+        /// Awake is called when this RandomizerTag is created or instantiated
+        /// </summary>
+        protected virtual void Awake()
+        {
+            Register();
+        }
+
+        /// <summary>
+        /// OnDestroy is called when this RandomizerTag is destroyed
+        /// </summary>
+        protected virtual void OnDestroy()
+        {
+            Unregister();
+        }
+
+        /// <summary>
+        /// Registers this tag with the tagManager
+        /// </summary>
+        public void Register()
-        void OnDestroy()
+        /// <summary>
+        /// Unregisters this tag with the tagManager
+        /// </summary>
+        public void Unregister()
         {
             tagManager.RemoveTag(this);
         }

com.unity.perception/Runtime/Randomization/Randomizers/RandomizerTagManager.cs

         public static RandomizerTagManager singleton { get; } = new RandomizerTagManager();
 
         Dictionary<Type, HashSet<Type>> m_TypeTree = new Dictionary<Type, HashSet<Type>>();
-        Dictionary<Type, HashSet<RandomizerTag>> m_TagMap = new Dictionary<Type, HashSet<RandomizerTag>>();
+        Dictionary<Type, LinkedHashSet<RandomizerTag>> m_TagMap = new Dictionary<Type, LinkedHashSet<RandomizerTag>>();
 
         /// <summary>
         /// Enumerates over all RandomizerTags of the given type present in the scene
             if (m_TypeTree.ContainsKey(tagType))
                 return;
 
-            m_TagMap.Add(tagType, new HashSet<RandomizerTag>());
+            m_TagMap.Add(tagType, new LinkedHashSet<RandomizerTag>());
             m_TypeTree.Add(tagType, new HashSet<Type>());
 
             var baseType = tagType.BaseType;
                 {
-                    m_TagMap.Add(baseType, new HashSet<RandomizerTag>());
+                    m_TagMap.Add(baseType, new LinkedHashSet<RandomizerTag>());
                     m_TypeTree[baseType] = new HashSet<Type> { tagType };
                 }
                 else
 
         internal void RemoveTag<T>(T tag) where T : RandomizerTag
         {
-            var tagType = typeof(T);
+            var tagType = tag.GetType();
             if (m_TagMap.ContainsKey(tagType) && m_TagMap[tagType].Contains(tag))
                 m_TagMap[tagType].Remove(tag);
         }

com.unity.perception/Runtime/Randomization/Scenarios/ScenarioBase.cs

 using System.Collections.Generic;
 using System.IO;
 using Unity.Simulation;
+using UnityEditor;
+using UnityEngine.Perception.GroundTruth;
-using UnityEngine.Perception.GroundTruth;
 
 namespace UnityEngine.Perception.Randomization.Scenarios
 {
     [DefaultExecutionOrder(-1)]
     public abstract class ScenarioBase : MonoBehaviour
     {
+        const string k_ScenarioIterationMetricDefinitionId = "DB1B258E-D1D0-41B6-8751-16F601A2E230";
+        bool m_FirstScenarioFrame = true;
+        MetricDefinition m_IterationMetricDefinition;
-        const string k_ScenarioIterationMetricDefinitionId = "DB1B258E-D1D0-41B6-8751-16F601A2E230";
+        // ReSharper disable once InconsistentNaming
+        [SerializeReference] internal List<Randomizer> m_Randomizers = new List<Randomizer>();
-        bool m_FirstScenarioFrame = true;
-        MetricDefinition m_IterationMetricDefinition;
+
+        /// <summary>
+        /// If true, this scenario will quit the Unity application when it's finished executing
+        /// </summary>
+        [HideInInspector]
+        public bool quitOnComplete = true;
 
         IEnumerable<Randomizer> activeRandomizers
         {
                         yield return randomizer;
             }
         }
-
-        // ReSharper disable once InconsistentNaming
-        [SerializeReference] internal List<Randomizer> m_Randomizers = new List<Randomizer>();
 
         /// <summary>
         /// Return the list of randomizers attached to this scenario
         /// <summary>
-        /// If true, this scenario will quit the Unity application when it's finished executing
-        /// </summary>
-        [HideInInspector] public bool quitOnComplete = true;
-
-        /// <summary>
         /// The name of the Json file this scenario's constants are serialized to/from.
         /// </summary>
         public virtual string configFileName => "scenario_configuration";
             get
             {
 #if UNITY_EDITOR
+
                 // This compiler define is required to allow samplers to
                 // iterate the scenario's random state in edit-mode
                 if (s_ActiveScenario == null)
         {
             Directory.CreateDirectory(Application.dataPath + "/StreamingAssets/");
             using (var writer = new StreamWriter(defaultConfigFilePath, false))
+            {
+            }
         }
 
         /// <summary>
         }
 
         /// <summary>
-        /// This method executed directly after this scenario has been registered and initialized
+        /// Awake is called when this scenario MonoBehaviour is created or instantiated
-        protected virtual void OnAwake() { }
-
-        void Awake()
+        protected virtual void Awake()
-            OnAwake();
             foreach (var randomizer in m_Randomizers)
                 randomizer.Create();
             ValidateParameters();
                 Guid.Parse(k_ScenarioIterationMetricDefinitionId));
         }
 
-        void OnEnable()
+        /// <summary>
+        /// OnEnable is called when this scenario is enabled
+        /// </summary>
+        protected virtual void OnEnable()
-        void OnDisable()
+        /// <summary>
+        /// OnEnable is called when this scenario is disabled
+        /// </summary>
+        protected virtual void OnDisable()
-        void Start()
+        /// <summary>
+        /// Start is called after Awake but before the first Update method call
+        /// </summary>
+        protected virtual void Start()
         {
             var randomSeedMetricDefinition = DatasetCapture.RegisterMetricDefinition(
                 "random-seed",
 #endif
         }
 
-        struct IterationMetricData
-        {
-            public int iteration;
-        }
-
-        void Update()
+        /// <summary>
+        /// Update is called once per frame
+        /// </summary>
+        protected virtual void Update()
         {
             // TODO: remove this check when the perception camera can capture the first frame of output
             if (m_SkipFrame)
                 if (!Manager.FinalUploadsDone)
                     return;
 #if UNITY_EDITOR
-                UnityEditor.EditorApplication.ExitPlaymode();
+                EditorApplication.ExitPlaymode();
 #else
                 Application.Quit();
 #endif
                 ResetRandomStateOnIteration();
                 DatasetCapture.ReportMetric(m_IterationMetricDefinition, new[]
                 {
-                    new IterationMetricData()
-                    {
-                        iteration = currentIteration
-                    }
+                    new IterationMetricData { iteration = currentIteration }
                 });
                 foreach (var randomizer in activeRandomizers)
                     randomizer.IterationStart();
                     $"Cannot remove non-randomizer type {randomizerType.Name} from randomizer list");
             var removed = false;
             for (var i = 0; i < m_Randomizers.Count; i++)
-            {
                 if (m_Randomizers[i].GetType() == randomizerType)
                 {
                     m_Randomizers.RemoveAt(i);
-            }
+
             if (!removed)
                 throw new ScenarioException(
                     $"No active Randomizer of type {randomizerType.Name} could be removed");
                 if (randomizer is T)
                     return i;
             }
+
             throw new ScenarioException($"A Randomizer of type {typeof(T).Name} was not added to this scenario");
         }
 
         {
             foreach (var randomizer in m_Randomizers)
             foreach (var parameter in randomizer.parameters)
-            {
                 try
                 {
                     parameter.Validate();
                     Debug.LogException(exception, this);
                 }
-            }
+        }
+
+        struct IterationMetricData
+        {
+            // ReSharper disable once NotAccessedField.Local
+            public int iteration;
         }
     }
 }

com.unity.perception/Runtime/Randomization/Scenarios/UnitySimulationScenario.cs

 using System;
-using System.IO;
-using UnityEngine.Perception.Randomization.Samplers;
 
 namespace UnityEngine.Perception.Randomization.Scenarios
 {
         }
 
         /// <inheritdoc/>
-        public sealed override void DeserializeFromFile(string configFilePath)
+        protected override void Start()
-            base.DeserializeFromFile(configFilePath);
+            base.Start();
             currentIteration = constants.instanceIndex;
         }
     }

com.unity.perception/Tests/Runtime/Randomization/RandomizerTests/RandomizerTagTests.cs

-using System.Linq;
+using System.Collections.Generic;
+using System.Linq;
-using UnityEngine.Perception.Randomization.Scenarios;
 using Assert = Unity.Assertions.Assert;
 
 namespace RandomizationTests.RandomizerTests
     {
-        public class ParentTag : RandomizerTag { }
-
-        public class ChildTag : ParentTag { }
-
-        GameObject m_TestObject;
-        FixedLengthScenario m_Scenario;
+        public class BaseTag : RandomizerTag { }
-        [SetUp]
-        public void Setup()
-        {
-            m_TestObject = new GameObject();
-            m_Scenario = m_TestObject.AddComponent<FixedLengthScenario>();
-            m_Scenario.quitOnComplete = false;
-        }
+        public class DerivedTag : BaseTag { }
-        public void TearDown()
+        public void Teardown()
-            Object.DestroyImmediate(m_TestObject);
+            var tags = Object.FindObjectsOfType<BaseTag>();
+            foreach (var tag in tags)
+            {
+                if (tag != null && tag.gameObject != null)
+                    Object.DestroyImmediate(tag.gameObject);
+            }
-        public void TagQueryFindsCorrectNumberOfGameObjects()
+        public void TagInheritanceWorksInTagQueries()
-            gameObject.AddComponent<ParentTag>();
+            gameObject.AddComponent<BaseTag>();
-            gameObject2.AddComponent<ChildTag>();
+            gameObject2.AddComponent<DerivedTag>();
-            var queriedObjects = tagManager.Query<ParentTag>().ToArray();
-            Assert.AreEqual(queriedObjects.Length, copyCount);
+            var queriedBaseTags = tagManager.Query<BaseTag>().ToArray();
+            Assert.AreEqual(queriedBaseTags.Length, copyCount);
-            queriedObjects = tagManager.Query<ChildTag>().ToArray();
-            Assert.AreEqual(queriedObjects.Length, copyCount);
+            var queriedDerivedTags = tagManager.Query<DerivedTag>().ToArray();
+            Assert.AreEqual(queriedDerivedTags.Length, copyCount);
+
+            queriedBaseTags = tagManager.Query<BaseTag>(true).ToArray();
+            Assert.AreEqual(queriedBaseTags.Length, copyCount * 2);
+        }
+
+        [Test]
+        public void TagQueriesPreserveInsertionOrder()
+        {
+            const int copyCount = 5;
+            const int destroyCount = 3;
+
+            var testObj = new GameObject();
+            testObj.AddComponent<BaseTag>();
+
+            var testObjects = new List<GameObject> { testObj };
+
+            for (var i = 0; i < copyCount - 1; i++)
+                testObjects.Add(Object.Instantiate(testObj));
+
+            for (var i = 0; i < destroyCount; i++)
+            {
+                Object.DestroyImmediate(testObjects[1]);
+                testObjects.RemoveAt(1);
+            }
+
+            for (var i = 0; i < copyCount + destroyCount; i++)
+                testObjects.Add(Object.Instantiate(testObj));
+
+            var tagManager = RandomizerTagManager.singleton;
+            var tags = tagManager.Query<BaseTag>();
+            var tagsArray = tags.ToArray();
-            queriedObjects = tagManager.Query<ParentTag>(true).ToArray();
-            Assert.AreEqual(queriedObjects.Length, copyCount * 2);
+            var index = 0;
+            foreach (var tag in tagsArray)
+                Assert.AreEqual(tag, testObjects[index++].GetComponent<BaseTag>());
         }
     }
 }

com.unity.perception/Runtime/Randomization/Randomizers/LinkedHashSet.cs

+using System.Collections;
+using System.Collections.Generic;
+
+namespace UnityEngine.Perception.Randomization.Randomizers
+{
+    /// <summary>
+    /// This collection has the properties of a HashSet that also preserves insertion order. As such, this data
+    /// structure demonstrates the following time complexities:
+    /// O(1) lookup, O(1) insertion, O(1) removal, and O(n) traversal
+    /// </summary>
+    /// <typeparam name="T">The item type to store in this collection</typeparam>
+    class LinkedHashSet<T> : ICollection<T>
+    {
+        readonly IDictionary<T, LinkedListNode<T>> m_Dictionary;
+        readonly LinkedList<T> m_LinkedList;
+
+        public LinkedHashSet() : this(EqualityComparer<T>.Default) { }
+
+        public LinkedHashSet(IEqualityComparer<T> comparer)
+        {
+            m_Dictionary = new Dictionary<T, LinkedListNode<T>>(comparer);
+            m_LinkedList = new LinkedList<T>();
+        }
+
+        public int Count => m_Dictionary.Count;
+
+        public bool IsReadOnly => m_Dictionary.IsReadOnly;
+
+        void ICollection<T>.Add(T item)
+        {
+            Add(item);
+        }
+
+        public bool Add(T item)
+        {
+            if (m_Dictionary.ContainsKey(item)) return false;
+            var node = m_LinkedList.AddLast(item);
+            m_Dictionary.Add(item, node);
+            return true;
+        }
+
+        public void Clear()
+        {
+            m_LinkedList.Clear();
+            m_Dictionary.Clear();
+        }
+
+        public bool Remove(T item)
+        {
+            var found = m_Dictionary.TryGetValue(item, out var node);
+            if (!found) return false;
+            m_Dictionary.Remove(item);
+            m_LinkedList.Remove(node);
+            return true;
+        }
+
+        public IEnumerator<T> GetEnumerator()
+        {
+            return m_LinkedList.GetEnumerator();
+        }
+
+        IEnumerator IEnumerable.GetEnumerator()
+        {
+            return GetEnumerator();
+        }
+
+        public bool Contains(T item)
+        {
+            return m_Dictionary.ContainsKey(item);
+        }
+
+        public void CopyTo(T[] array, int arrayIndex)
+        {
+            m_LinkedList.CopyTo(array, arrayIndex);
+        }
+    }
+}

com.unity.perception/Runtime/Randomization/Randomizers/LinkedHashSet.cs.meta

+fileFormatVersion: 2
+guid: 2b6c83991e2c4c88af9b2b43c90121c7
+timeCreated: 1612730504