Added decimal marker option in TextLoader (#5145)

* Added decimal marker option in TextLoader

* Added decimalChar to more TextLoader constructors

* Removed decimalMarker from TextLoader constructors due to API breaking

* Added unit test for ',' as a decimal marker, and added decimalMarker to TextLoaderCursor and TextLoaderParser

* Added DecimalMarker in DoubleParser

* Added decimal marker check and removed decimalMarker from CreateTextLoader's constructor

* Added TextLoader decimalMarker unit tests, and refined logic in DoubleParser

* Refine tests, logic, csv dataset

* nit fix

* Compressed tests using <T>

Author: mstfbl

src/Microsoft.ML.Core/Utilities/DoubleParser.cs

@@ -17,6 +17,21 @@ internal static class DoubleParser
         private const ulong TopThreeBits = 0xE000000000000000UL;
         private const char InfinitySymbol = '\u221E';
 
+        // Note for future development: DoubleParser is a static class and DecimalMarker is a
+        // static variable, which means only one instance of these can exist at once. As such,
+        // the value of DecimalMarker cannot vary when datasets with differing decimal markers
+        // are loaded together at once, which would result in not being able to accurately read
+        // the dataset with the differing decimal marker. Although this edge case where we attempt
+        // to load in datasets with different decimal markers at once is unlikely to occur, we
+        // should still be aware of this and plan to fix it in the future.
+
+        // The decimal marker that separates the integer part from the fractional part of a number
+        // written in decimal from can vary across different cultures as either '.' or ','. The
+        // default decimal marker in ML .NET is '.', however through this static char variable,
+        // we allow users to specify the decimal marker used in their datasets as ',' as well.
+        [BestFriend]
+        internal static char DecimalMarker = '.';
+
         // REVIEW: casting ulong to Double doesn't always do the right thing, for example
         // with 0x84595161401484A0UL. Hence the gymnastics several places in this code. Note that
         // long to Double does work. The work around is:
@@ -555,6 +570,12 @@ private static bool TryParseCore(ReadOnlySpan<char> span, ref int ich, ref bool
                     break;
 
                 case '.':
+                    if (DecimalMarker != '.') // Decimal marker was not '.', but we encountered a '.', which must be an error.
+                        return false; // Since this was an error, return false, which will later make the caller to set NaN as the out value.
+                    goto LPoint;
+                case ',':
+                    if (DecimalMarker != ',') // Same logic as above.
+                        return false;
                     goto LPoint;
 
                 // The common cases.
@@ -571,7 +592,7 @@ private static bool TryParseCore(ReadOnlySpan<char> span, ref int ich, ref bool
                     break;
             }
 
-            // Get digits before '.'
+            // Get digits before the decimal marker, which may be '.' or ','
             uint d;
             for (; ; )
             {
@@ -593,14 +614,14 @@ private static bool TryParseCore(ReadOnlySpan<char> span, ref int ich, ref bool
             }
             Contracts.Assert(i < span.Length);
 
-            if (span[i] != '.')
+            if (span[i] != DecimalMarker)
                 goto LAfterDigits;
 
             LPoint:
             Contracts.Assert(i < span.Length);
-            Contracts.Assert(span[i] == '.');
+            Contracts.Assert(span[i] == DecimalMarker);
 
-            // Get the digits after '.'
+            // Get the digits after the decimal marker, which may be '.' or ','
             for (; ; )
             {
                 if (++i >= span.Length)

src/Microsoft.ML.Data/DataLoadSave/Text/TextLoader.cs

@@ -474,6 +474,12 @@ public class Options
             [Argument(ArgumentType.AtMostOnce, Name = nameof(Separator), Visibility = ArgumentAttribute.VisibilityType.EntryPointsOnly, HelpText = "Source column separator.", ShortName = "sep")]
             public char[] Separators = new[] { Defaults.Separator };
 
+            /// <summary>
+            /// The character that should be used as the decimal marker. Default value is '.'. Only '.' and ',' are allowed to be decimal markers.
+            /// </summary>
+            [Argument(ArgumentType.AtMostOnce, Name = "Decimal Marker", HelpText = "Character symbol used to separate the integer part from the fractional part of a number written in decimal form.", ShortName = "decimal")]
+            public char DecimalMarker = Defaults.DecimalMarker;
+
             /// <summary>
             /// Specifies the input columns that should be mapped to <see cref="IDataView"/> columns.
             /// </summary>
@@ -541,6 +547,7 @@ internal static class Defaults
             internal const bool AllowQuoting = false;
             internal const bool AllowSparse = false;
             internal const char Separator = '\t';
+            internal const char DecimalMarker = '.';
             internal const bool HasHeader = false;
             internal const bool TrimWhitespace = false;
             internal const bool ReadMultilines = false;
@@ -1071,7 +1078,7 @@ private static VersionInfo GetVersionInfo()
                 //verWrittenCur: 0x0001000A, // Added ForceVector in Range
                 //verWrittenCur: 0x0001000B, // Header now retained if used and present
                 //verWrittenCur: 0x0001000C, // Removed Min and Contiguous from KeyType, and added ReadMultilines flag to OptionFlags
-                verWrittenCur: 0x0001000D, // Added escapeChar option
+                verWrittenCur: 0x0001000D, // Added escapeChar option and decimal marker option to allow for ',' to be a decimal marker
                 verReadableCur: 0x0001000A,
                 verWeCanReadBack: 0x00010009,
                 loaderSignature: LoaderSignature,
@@ -1103,6 +1110,7 @@ private enum OptionFlags : uint
         // Input size is zero for unknown - determined by the data (including sparse rows).
         private readonly int _inputSize;
         private readonly char[] _separators;
+        private readonly char _decimalMarker;
         private readonly Bindings _bindings;
 
         private readonly Parser _parser;
@@ -1219,6 +1227,11 @@ internal TextLoader(IHostEnvironment env, Options options = null, IMultiStreamSo
                 }
             }
 
+            if (options.DecimalMarker != '.' && options.DecimalMarker != ',')
+                throw _host.ExceptUserArg(nameof(Options.DecimalMarker), "Decimal marker cannot be the '{0}' character. It must be '.' or ','.", options.DecimalMarker);
+            if (!options.AllowQuoting && options.DecimalMarker == ',' && _separators.Contains(','))
+                throw _host.ExceptUserArg(nameof(Options.AllowQuoting), "Quoting must be allowed if decimal marker and separator are the ',' character.");
+            _decimalMarker = options.DecimalMarker;
             _escapeChar = options.EscapeChar;
             if(_separators.Contains(_escapeChar))
                 throw _host.ExceptUserArg(nameof(Options.EscapeChar), "EscapeChar '{0}' can't be used both as EscapeChar and separator", _escapeChar);
@@ -1387,6 +1400,7 @@ private TextLoader(IHost host, ModelLoadContext ctx)
             // int: number of separators
             // char[]: separators
             // char: escapeChar
+            // char: decimal marker
             // bindings
             int cbFloat = ctx.Reader.ReadInt32();
             host.CheckDecode(cbFloat == sizeof(float));
@@ -1414,10 +1428,13 @@ private TextLoader(IHost host, ModelLoadContext ctx)
             if (ctx.Header.ModelVerWritten >= 0x0001000D)
             {
                 _escapeChar = ctx.Reader.ReadChar();
+                _decimalMarker = ctx.Reader.ReadChar();
+                host.CheckDecode(_decimalMarker == '.' || _decimalMarker == ',');
             }
             else
             {
                 _escapeChar = Defaults.EscapeChar;
+                _decimalMarker = Defaults.DecimalMarker;
             }
 
             host.CheckDecode(!_separators.Contains(_escapeChar));
@@ -1463,6 +1480,7 @@ void ICanSaveModel.Save(ModelSaveContext ctx)
             // int: number of separators
             // char[]: separators
             // char: escapeChar
+            // char: decimal marker
             // bindings
             ctx.Writer.Write(sizeof(float));
             ctx.Writer.Write(_maxRows);
@@ -1472,6 +1490,7 @@ void ICanSaveModel.Save(ModelSaveContext ctx)
             ctx.Writer.Write(_inputSize);
             ctx.Writer.WriteCharArray(_separators);
             ctx.Writer.Write(_escapeChar);
+            ctx.Writer.Write(_decimalMarker);
 
             _bindings.Save(ctx);
         }
@@ -1612,13 +1631,15 @@ public BoundLoader(TextLoader loader, IMultiStreamSource files)
             public DataViewRowCursor GetRowCursor(IEnumerable<DataViewSchema.Column> columnsNeeded, Random rand = null)
             {
                 _host.CheckValueOrNull(rand);
+                DoubleParser.DecimalMarker = _loader._decimalMarker;
                 var active = Utils.BuildArray(_loader._bindings.OutputSchema.Count, columnsNeeded);
                 return Cursor.Create(_loader, _files, active);
             }
 
             public DataViewRowCursor[] GetRowCursorSet(IEnumerable<DataViewSchema.Column> columnsNeeded, int n, Random rand = null)
             {
                 _host.CheckValueOrNull(rand);
+                DoubleParser.DecimalMarker = _loader._decimalMarker;
                 var active = Utils.BuildArray(_loader._bindings.OutputSchema.Count, columnsNeeded);
                 return Cursor.CreateSet(_loader, _files, active, n);
             }

test/BaselineOutput/Common/EntryPoints/core_manifest.json

@@ -369,6 +369,18 @@
                   "\t"
                 ]
               },
+              {
+                "Name": "Decimal Marker",
+                "Type": "Char",
+                "Desc": "Character symbol used to separate the integer part from the fractional part of a number written in decimal form.",
+                "Aliases": [
+                  "decimal"
+                ],
+                "Required": false,
+                "SortOrder": 150.0,
+                "IsNullable": false,
+                "Default": "."
+              },
               {
                 "Name": "TrimWhitespace",
                 "Type": "Bool",

test/Microsoft.ML.Tests/TextLoaderTests.cs

@@ -840,6 +840,168 @@ public void TestTextLoaderBackCompat_VerWritt_0x0001000C()
             Assert.Equal("Iris-setosa", previewIris.RowView[0].Values[index].Value.ToString());
         }
 
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void TestCommaAsDecimalMarker(bool useCsvVersion)
+        {
+            // When userCsvVersion == false:
+            // Datasets iris.txt and iris-decimal-marker-as-comma.txt are the exact same, except for their
+            // decimal markers. Decimal marker in iris.txt is '.', and ',' in iris-decimal-marker-as-comma.txt.
+
+            // When userCsvVersion == true:
+            // Check to confirm TextLoader can read data from a CSV file where the separator is ',', decimals
+            // are enclosed with quotes, and with the decimal marker being ','.
+
+            // Do these checks with both float and double as types of features being read, to test decimal marker
+            // recognition with both doubles and floats.
+            TestCommaAsDecimalMarkerHelper<float>(useCsvVersion);
+            TestCommaAsDecimalMarkerHelper<double>(useCsvVersion);
+        }
+        
+        private void TestCommaAsDecimalMarkerHelper<T>(bool useCsvVersion)
+        {
+            // Datasets iris.txt and iris-decimal-marker-as-comma.txt are the exact same, except for their
+            // decimal markers. Decimal marker in iris.txt is '.', and ',' in iris-decimal-marker-as-comma.txt.
+            // Datasets iris.txt and iris-decimal-marker-as-comma.csv have the exact same data, however the .csv
+            // version has ',' as decimal marker and separator, and feature values are enclosed with quotes.
+            // T varies as either float or double, so that decimal markers can be tested for both floating
+            // point value types.
+            var mlContext = new MLContext(seed: 1);
+
+            // Read dataset with period as decimal marker.
+            string dataPathDecimalMarkerPeriod = GetDataPath("iris.txt");
+            var readerDecimalMarkerPeriod = new TextLoader(mlContext, new TextLoader.Options()
+            {
+                Columns = new[]
+                        {
+                            new TextLoader.Column("Label", DataKind.UInt32, 0),
+                            new TextLoader.Column("Features", typeof(T) == typeof(double) ? DataKind.Double : DataKind.Single, new [] { new TextLoader.Range(1, 4) }),
+                        },
+                DecimalMarker = '.'
+            });
+            var textDataDecimalMarkerPeriod = readerDecimalMarkerPeriod.Load(GetDataPath(dataPathDecimalMarkerPeriod));
+
+            // Load values from iris.txt
+            DataViewSchema columnsPeriod = textDataDecimalMarkerPeriod.Schema;
+            using DataViewRowCursor cursorPeriod = textDataDecimalMarkerPeriod.GetRowCursor(columnsPeriod);
+            UInt32 labelPeriod = default;
+            ValueGetter<UInt32> labelDelegatePeriod = cursorPeriod.GetGetter<UInt32>(columnsPeriod[0]);
+            VBuffer<T> featuresPeriod = default;
+            ValueGetter<VBuffer<T>> featuresDelegatePeriod = cursorPeriod.GetGetter<VBuffer<T>>(columnsPeriod[1]);
+
+            // Iterate over each row and save labels and features to array for future comparison
+            int count = 0;
+            UInt32[] labels = new uint[150];
+            T[][] features = new T[150][];
+            while (cursorPeriod.MoveNext())
+            {
+                //Get values from respective columns
+                labelDelegatePeriod(ref labelPeriod);
+                featuresDelegatePeriod(ref featuresPeriod);
+                labels[count] = labelPeriod;
+                features[count] = featuresPeriod.GetValues().ToArray();
+                count++;
+            }
+
+            // Read dataset with comma as decimal marker.
+            // Dataset is either the .csv version or the .txt version.
+            string dataPathDecimalMarkerComma;
+            TextLoader.Options options = new TextLoader.Options()
+            {
+                Columns = new[]
+                        {
+                            new TextLoader.Column("Label", DataKind.UInt32, 0),
+                            new TextLoader.Column("Features", typeof(T) == typeof(double) ? DataKind.Double : DataKind.Single, new [] { new TextLoader.Range(1, 4) })
+                        },
+            };
+            // Set TextLoader.Options for the .csv or .txt cases.
+            if (useCsvVersion)
+            {
+                dataPathDecimalMarkerComma = GetDataPath("iris-decimal-marker-as-comma.csv");
+                options.DecimalMarker = ',';
+                options.Separator = ",";
+                options.AllowQuoting = true;
+                options.HasHeader = true;
+            }
+            else
+            {
+                dataPathDecimalMarkerComma = GetDataPath("iris-decimal-marker-as-comma.txt");
+                options.DecimalMarker = ',';
+            }
+            var readerDecimalMarkerComma = new TextLoader(mlContext, options);
+            var textDataDecimalMarkerComma = readerDecimalMarkerComma.Load(GetDataPath(dataPathDecimalMarkerComma));
+
+            // Load values from dataset with comma as decimal marker
+            DataViewSchema columnsComma = textDataDecimalMarkerComma.Schema;
+            using DataViewRowCursor cursorComma = textDataDecimalMarkerComma.GetRowCursor(columnsComma);
+            UInt32 labelComma = default;
+            ValueGetter<UInt32> labelDelegateComma = cursorComma.GetGetter<UInt32>(columnsComma[0]);
+            VBuffer<T> featuresComma = default;
+            ValueGetter<VBuffer<T>> featuresDelegateComma = cursorComma.GetGetter<VBuffer<T>>(columnsComma[1]);
+
+            // Check values from dataset with comma as decimal marker match those in iris.txt (period decimal marker)
+            count = 0;
+            while (cursorComma.MoveNext())
+            {
+                //Get values from respective columns
+                labelDelegateComma(ref labelComma);
+                featuresDelegateComma(ref featuresComma);
+                Assert.Equal(labels[count], labelComma);
+                Assert.Equal(features[count], featuresComma.GetValues().ToArray());
+                count++;
+            }
+        }
+
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void TestWrongDecimalMarkerInputs(bool useCommaAsDecimalMarker)
+        {
+            // When DecimalMarker does not match the actual decimal marker used in the dataset,
+            // we obtain values of NaN. Check that the values are indeed NaN in this case.
+            // Do this check for both cases where decimal markers in the dataset are '.' and ','.
+            var mlContext = new MLContext(seed: 1);
+
+            // Try reading a dataset where '.' is the actual decimal marker, but DecimalMarker = ',',
+            // and vice versa.
+            string dataPath;
+            TextLoader.Options options = new TextLoader.Options()
+            {
+                Columns = new[]
+                        {
+                            new TextLoader.Column("Label", DataKind.UInt32, 0),
+                            new TextLoader.Column("Features", DataKind.Single, new [] { new TextLoader.Range(1, 4) })
+                        },
+            };
+            if (useCommaAsDecimalMarker)
+            {
+                dataPath = GetDataPath("iris.txt"); //  Has '.' as decimal marker inside dataset
+                options.DecimalMarker = ','; // Choose wrong decimal marker on purpose
+            }
+            else
+            {
+                dataPath = GetDataPath("iris-decimal-marker-as-comma.txt"); // Has ',' as decimal marker inside dataset
+                options.DecimalMarker = '.'; // Choose wrong decimal marker on purpose
+            }
+            var reader = new TextLoader(mlContext, options);
+            var textData = reader.Load(GetDataPath(dataPath));
+
+            // Check that the features being loaded are NaN.
+            DataViewSchema columns = textData.Schema;
+            using DataViewRowCursor cursor = textData.GetRowCursor(columns);
+            VBuffer<Single> featuresPeriod = default;
+            ValueGetter<VBuffer<Single>> featuresDelegatePeriod = cursor.GetGetter<VBuffer<Single>>(columns[1]);
+            
+            // Iterate over each row and check that feature values are NaN.
+            while (cursor.MoveNext())
+            {
+                featuresDelegatePeriod.Invoke(ref featuresPeriod);
+                foreach(float feature in featuresPeriod.GetValues().ToArray())
+                    Assert.Equal(feature, Single.NaN);
+            }
+        }
+
         private class IrisNoFields
         {
         }