Converted potentially large variables to type long by mstfbl · Pull Request #5041 · dotnet/machinelearning

mstfbl · 2020-04-20T22:15:39Z

As explained in Issue #3228, very large datasets more than 2.14 billion rows of data can cause overflow when, say, the sum of these labels are obtained, and if these are stored as ints. This PR converts arrays and matrices for storing labels and features in their respective histograms from type int to type long. In addition, this PR updates the version of NaiveBayesMulticlassTrainer's Loader to preserve backwards compatibility.

mstfbl · 2020-04-20T22:17:56Z

...crosoft.ML.StandardTrainers/Standard/MulticlassClassification/MulticlassNaiveBayesTrainer.cs

            // *** Binary format ***
-            // int: _labelCount
-            // int[_labelCount]: _labelHistogram
+            // int: _labelCount (read during reading of _labelHistogram in ReadLongArray())


I added this comment as we are not explicitly reading _labelCount here, but in ReadLongArray() as shown below:

machinelearning/src/Microsoft.ML.Core/Utilities/Stream.cs

Lines 681 to 688 in 214926f

public static long[] ReadLongArray(this BinaryReader reader)

{

Contracts.AssertValue(reader);

int size = reader.ReadInt32();

Contracts.CheckDecode(size >= 0);

return ReadLongArray(reader, size);

}

#Resolved

mstfbl · 2020-04-20T22:21:12Z

...crosoft.ML.StandardTrainers/Standard/MulticlassClassification/MulticlassNaiveBayesTrainer.cs

            // int[_labelCount]: _absentFeaturesLogProb
-            ctx.Writer.WriteIntArray(_labelHistogram.AsSpan(0, _labelCount));
+            ctx.Writer.Write(_labelCount);
+            ctx.Writer.WriteLongStream(_labelHistogram);


.AsSpan() is not implemented for converting from 'System.Span' to 'System.Collections.Generic.IEnumerable. The only other way around this is using Array.Copy(), but that introduces needless array copying. I also don't see a case where not all of _labelHistogram is not serialized. As such, _labelHistogram is always serialized whole here. #Resolved

mstfbl · 2020-04-20T22:21:30Z

...crosoft.ML.StandardTrainers/Standard/MulticlassClassification/MulticlassNaiveBayesTrainer.cs

            {
                if (_labelHistogram[i] > 0)
-                    ctx.Writer.WriteIntsNoCount(_featureHistogram[i].AsSpan(0, _featureCount));
+                    ctx.Writer.WriteLongStream(_featureHistogram[i]);


Sames as above.

.AsSpan() is not implemented for converting from 'System.Span' to 'System.Collections.Generic.IEnumerable. The only other way around this is using Array.Copy(), but that introduces needless array copying. I also don't see a case where not all of _labelHistogram is not serialized. As such, _labelHistogram is always serialized whole here. #Resolved

harishsk · 2020-04-21T04:17:05Z

...crosoft.ML.StandardTrainers/Standard/MulticlassClassification/MulticlassNaiveBayesTrainer.cs

+            else
+            {
+                _labelHistogram = Array.ConvertAll(ctx.Reader.ReadIntArray() ?? new int[0], x => (long)x);
+            }


If ReadIntArray returns null, it likely means the file is bad. Should you be throwing an error in this case? The old behavior seems wrong. #Resolved

Hey Harish, the array being read from ctx.Reader.ReadIntArray(int size) can return null if the size of the array being loaded is 0. Here's the source code:

machinelearning/src/Microsoft.ML.Core/Utilities/Stream.cs

Lines 605 to 641 in 7628d6c

public static int[] ReadIntArray(this BinaryReader reader)

{

Contracts.AssertValue(reader);

int size = reader.ReadInt32();

Contracts.CheckDecode(size >= 0);

return ReadIntArray(reader, size);

}

public static int[] ReadIntArray(this BinaryReader reader, int size)

{

Contracts.AssertValue(reader);

Contracts.Assert(size >= 0);

if (size == 0)

return null;

var values = new int[size];

long bufferSizeInBytes = (long)size * sizeof(int);

if (bufferSizeInBytes < _bulkReadThresholdInBytes)

{

for (int i = 0; i < size; i++)

values[i] = reader.ReadInt32();

}

else

{

unsafe

{

fixed (void* dst = values)

{

ReadBytes(reader, dst, bufferSizeInBytes, bufferSizeInBytes);

}

}

}

return values;

}

#Resolved

harishsk · 2020-04-21T04:24:00Z

...crosoft.ML.StandardTrainers/Standard/MulticlassClassification/MulticlassNaiveBayesTrainer.cs

+                        _featureHistogram[iLabel] = ctx.Reader.ReadLongArray(_featureCount);
+                    else
+                        _featureHistogram[iLabel] = Array.ConvertAll(ctx.Reader.ReadIntArray(_featureCount) ?? new int[0], x => (long)x);
                    for (int iFeature = 0; iFeature < _featureCount; iFeature += 1)


Same comment as above #Resolved

harishsk

Converted potentially large values to type long

6d9d879

mstfbl changed the title ~~Converted potentially large values to type long~~ Converted potentially large variables to type long Apr 20, 2020

mstfbl commented Apr 20, 2020

View reviewed changes

mstfbl added 2 commits April 20, 2020 16:23

Fix to avoid API breaking change

180d81c

Updated NaiveBayesMulticlassTrainer's Loader version

9dd071f

mstfbl marked this pull request as ready for review April 21, 2020 01:39

mstfbl requested a review from a team as a code owner April 21, 2020 01:39

harishsk reviewed Apr 21, 2020

View reviewed changes

harishsk approved these changes Apr 21, 2020

View reviewed changes

Added note on code path for backwards compatability upon request

2ea3326

mstfbl merged commit 0a980fc into dotnet:master Apr 21, 2020

ghost locked as resolved and limited conversation to collaborators Mar 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Converted potentially large variables to type long#5041

Converted potentially large variables to type long#5041
mstfbl merged 4 commits intodotnet:masterfrom
mstfbl:Issue-3228

mstfbl commented Apr 20, 2020 •

edited

Loading

Uh oh!

mstfbl Apr 20, 2020 •

edited

Loading

Uh oh!

mstfbl Apr 20, 2020 •

edited

Loading

Uh oh!

mstfbl Apr 20, 2020 •

edited

Loading

Uh oh!

harishsk Apr 21, 2020 •

edited by mstfbl

Loading

Uh oh!

mstfbl Apr 21, 2020 •

edited

Loading

Uh oh!

harishsk Apr 21, 2020 •

edited by mstfbl

Loading

Uh oh!

harishsk left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	public static long[] ReadLongArray(this BinaryReader reader)
	{
	Contracts.AssertValue(reader);

	int size = reader.ReadInt32();
	Contracts.CheckDecode(size >= 0);
	return ReadLongArray(reader, size);
	}

	public static int[] ReadIntArray(this BinaryReader reader)
	{
	Contracts.AssertValue(reader);

	int size = reader.ReadInt32();
	Contracts.CheckDecode(size >= 0);
	return ReadIntArray(reader, size);
	}

	public static int[] ReadIntArray(this BinaryReader reader, int size)
	{
	Contracts.AssertValue(reader);
	Contracts.Assert(size >= 0);

	if (size == 0)
	return null;
	var values = new int[size];

	long bufferSizeInBytes = (long)size * sizeof(int);
	if (bufferSizeInBytes < _bulkReadThresholdInBytes)
	{
	for (int i = 0; i < size; i++)
	values[i] = reader.ReadInt32();
	}
	else
	{
	unsafe
	{
	fixed (void* dst = values)
	{
	ReadBytes(reader, dst, bufferSizeInBytes, bufferSizeInBytes);
	}
	}
	}

	return values;
	}

Conversation

mstfbl commented Apr 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mstfbl Apr 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mstfbl Apr 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mstfbl Apr 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harishsk Apr 21, 2020 • edited by mstfbl Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mstfbl Apr 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harishsk Apr 21, 2020 • edited by mstfbl Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harishsk left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mstfbl commented Apr 20, 2020 •

edited

Loading

mstfbl Apr 20, 2020 •

edited

Loading

mstfbl Apr 20, 2020 •

edited

Loading

mstfbl Apr 20, 2020 •

edited

Loading

harishsk Apr 21, 2020 •

edited by mstfbl

Loading

mstfbl Apr 21, 2020 •

edited

Loading

harishsk Apr 21, 2020 •

edited by mstfbl

Loading