[MLA-1879] culture-invariant sorting for sensors and actuators

com.unity.ml-agents/CHANGELOG.md

@@ -44,6 +44,9 @@ depend on the previous behavior, you can explicitly set the Agent's `InferenceDe
 
 ### Bug Fixes
 #### com.unity.ml-agents / com.unity.ml-agents.extensions (C#)
+- Fixed a bug where sensors and actuators could get sorted inconsistently on different systems to different Culture
+settings. Unfortunately, this may require retraining models if it changes the resulting order of the sensors
+or actuators on your system. (#5194)
 #### ml-agents / ml-agents-envs / gym-unity (Python)
 
 

com.unity.ml-agents/Runtime/Actuators/ActuatorManager.cs

@@ -12,7 +12,7 @@ namespace Unity.MLAgents.Actuators
     internal class ActuatorManager : IList<IActuator>
     {
         // IActuators managed by this object.
-        IList<IActuator> m_Actuators;
+        List<IActuator> m_Actuators;
 
         // An implementation of IDiscreteActionMask that allows for writing to it based on an offset.
         ActuatorDiscreteActionMask m_DiscreteActionMask;
@@ -95,7 +95,7 @@ internal void ReadyActuatorsForExecution(IList<IActuator> actuators, int numCont
 #endif
 
             // Sort the Actuators by name to ensure determinism
-            SortActuators();
+            SortActuators(m_Actuators);
             var continuousActions = numContinuousActions == 0 ? ActionSegment<float>.Empty :
                 new ActionSegment<float>(new float[numContinuousActions]);
             var discreteActions = numDiscreteBranches == 0 ? ActionSegment<int>.Empty : new ActionSegment<int>(new int[numDiscreteBranches]);
@@ -317,11 +317,9 @@ public void ResetData()
         /// <summary>
         /// Sorts the <see cref="IActuator"/>s according to their <see cref="IActuator.Name"/> value.
         /// </summary>
-        void SortActuators()
+        internal static void SortActuators(List<IActuator> actuators)
         {
-            ((List<IActuator>)m_Actuators).Sort((x,
-                y) => x.Name
-                .CompareTo(y.Name));
+            actuators.Sort((x, y) => string.Compare(x.Name, y.Name, StringComparison.InvariantCulture));
         }
 
         /// <summary>

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

@@ -989,7 +989,7 @@ internal void InitializeSensors()
             }
 
             // Sort the Sensors by name to ensure determinism
-            sensors.Sort((x, y) => x.GetName().CompareTo(y.GetName()));
+            SensorUtils.SortSensors(sensors);
 
 #if DEBUG
             // Make sure the names are actually unique

com.unity.ml-agents/Runtime/Inference/BarracudaModelExtensions.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using Unity.Barracuda;
@@ -34,7 +35,7 @@ public static string[] GetInputNames(this Model model)
                 names.Add(mem.input);
             }
 
-            names.Sort();
+            names.Sort(StringComparer.InvariantCulture);
 
             return names.ToArray();
         }
@@ -87,7 +88,7 @@ public static IReadOnlyList<TensorProxy> GetInputTensors(this Model model)
                 });
             }
 
-            tensors.Sort((el1, el2) => el1.name.CompareTo(el2.name));
+            tensors.Sort((el1, el2) => string.Compare(el1.name, el2.name, StringComparison.InvariantCulture));
 
             return tensors;
         }
@@ -150,7 +151,7 @@ public static string[] GetOutputNames(this Model model)
                 }
             }
 
-            names.Sort();
+            names.Sort(StringComparer.InvariantCulture);
 
             return names.ToArray();
         }

com.unity.ml-agents/Runtime/Sensors/ISensor.cs

@@ -1,3 +1,7 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+
 namespace Unity.MLAgents.Sensors
 {
     /// <summary>
@@ -124,4 +128,13 @@ public static int ObservationSize(this ISensor sensor)
             return count;
         }
     }
+
+    internal static class SensorUtils
+    {
+        internal static void SortSensors(List<ISensor> sensors)
+        {
+            // Use InvariantCulture to ensure consistent sorting between different culture settings.
+            sensors.Sort((x, y) => string.Compare(x.GetName(), y.GetName(), StringComparison.InvariantCulture));
+        }
+    }
 }

com.unity.ml-agents/Tests/Editor/Actuators/ActuatorManagerTests.cs

@@ -1,4 +1,6 @@
 using System;
+using System.Collections.Generic;
+using System.Globalization;
 using System.Linq;
 using NUnit.Framework;
 using Unity.MLAgents.Actuators;
@@ -321,5 +323,31 @@ public void TestHeuristic()
             Assert.IsTrue(va2.m_HeuristicCalled);
             Assert.AreEqual(va2.m_DiscreteBufferSize, 4);
         }
+
+
+        /// <summary>
+        /// Test that sensors sort by name consistently across culture settings.
+        /// Example strings and cultures taken from
+        /// https://docs.microsoft.com/en-us/globalization/locale/sorting-and-string-comparison
+        /// </summary>
+        /// <param name="culture"></param>
+        [TestCase("da-DK")]
+        [TestCase("en-US")]
+        public void TestSortActuators(string culture)
+        {
+            List<IActuator> actuators = new List<IActuator>();
+            var actuator0 = new TestActuator(ActionSpec.MakeContinuous(2), "Apple");
+            var actuator1 = new TestActuator(ActionSpec.MakeContinuous(2), "Æble");
+            actuators.Add(actuator0);
+            actuators.Add(actuator1);
+
+            var originalCulture = CultureInfo.CurrentCulture;
+            CultureInfo.CurrentCulture = new CultureInfo(culture);
+            ActuatorManager.SortActuators(actuators);
+            CultureInfo.CurrentCulture = originalCulture;
+
+            Assert.AreEqual(actuator1, actuators[0]);
+            Assert.AreEqual(actuator0, actuators[1]);
+        }
     }
 }

com.unity.ml-agents/Tests/Runtime/Sensor/SensorUtilTests.cs

@@ -0,0 +1,57 @@
+using System;
+using System.Collections.Generic;
+using System.Globalization;
+using NUnit.Framework;
+using UnityEngine;
+using Unity.MLAgents.Sensors;
+using Unity.MLAgents.Utils.Tests;
+
+namespace Unity.MLAgents.Tests
+{
+
+    [TestFixture]
+    public class SensorUtilTests
+    {
+        internal class TempCulture : IDisposable
+        {
+            private CultureInfo m_OriginalCulture;
+
+            internal TempCulture(CultureInfo newCulture)
+            {
+                m_OriginalCulture = CultureInfo.CurrentCulture;
+                CultureInfo.CurrentCulture = newCulture;
+            }
+
+            public void Dispose()
+            {
+                CultureInfo.CurrentCulture = m_OriginalCulture;
+            }
+        }
+
+        /// <summary>
+        /// Test that sensors sort by name consistently across culture settings.
+        /// Example strings and cultures taken from
+        /// https://docs.microsoft.com/en-us/globalization/locale/sorting-and-string-comparison
+        /// </summary>
+        /// <param name="culture"></param>
+        [TestCase("da-DK")]
+        [TestCase("en-US")]
+        public void TestSortCulture(string culture)
+        {
+            List<ISensor> sensors = new List<ISensor>();
+            var sensor0 = new TestSensor("Apple");
+            var sensor1 = new TestSensor("Æble");
+            sensors.Add(sensor0);
+            sensors.Add(sensor1);
+
+            var originalCulture = CultureInfo.CurrentCulture;
+            CultureInfo.CurrentCulture = new CultureInfo(culture);
+            SensorUtils.SortSensors(sensors);
+            CultureInfo.CurrentCulture = originalCulture;
+
+            Assert.AreEqual(sensor1, sensors[0]);
+            Assert.AreEqual(sensor0, sensors[1]);
+        }
+
+    }
+}