Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 25 additions & 4 deletions src/Microsoft.ML.Core/Utilities/DoubleParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '.';

Comment thread
mstfbl marked this conversation as resolved.
// 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:
Expand Down Expand Up @@ -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;
Comment thread
mstfbl marked this conversation as resolved.

// The common cases.
Expand All @@ -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 (; ; )
{
Expand All @@ -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)
Expand Down
20 changes: 19 additions & 1 deletion src/Microsoft.ML.Data/DataLoadSave/Text/TextLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
/// </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;
Comment thread
mstfbl marked this conversation as resolved.
Comment thread
mstfbl marked this conversation as resolved.

/// <summary>
/// Specifies the input columns that should be mapped to <see cref="IDataView"/> columns.
/// </summary>
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -1219,6 +1227,9 @@ 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);
_decimalMarker = options.DecimalMarker;
Comment thread
mstfbl marked this conversation as resolved.
_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);
Expand Down Expand Up @@ -1387,6 +1398,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));
Expand Down Expand Up @@ -1414,6 +1426,8 @@ private TextLoader(IHost host, ModelLoadContext ctx)
if (ctx.Header.ModelVerWritten >= 0x0001000D)
{
_escapeChar = ctx.Reader.ReadChar();
_decimalMarker = ctx.Reader.ReadChar();
host.CheckDecode(_decimalMarker == '.' || _decimalMarker == ',');
}
Comment thread
mstfbl marked this conversation as resolved.
else
Comment thread
mstfbl marked this conversation as resolved.
{
Expand Down Expand Up @@ -1463,6 +1477,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);
Expand All @@ -1472,6 +1487,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);
}
Expand Down Expand Up @@ -1612,13 +1628,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);
}
Expand Down
12 changes: 12 additions & 0 deletions test/BaselineOutput/Common/EntryPoints/core_manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading