Added buffer re-use while reading the image in netstandard 2.0. Addressed Eric's comments. Changed ImageLoadingTransformer to take a bool type instead of a DataViewType to make it user friendly. (type = true means we are using VBuffer<byte> , type = false means we are using ImageDataViewType)

harshithapv · harshithapv · commit 3ad26b4a1a06 · 2019-10-01T15:59:06.000-07:00
diff --git a/docs/samples/Microsoft.ML.Samples/Dynamic/ImageClassification/ResnetV2101TransferLearningTrainTestSplit.cs b/docs/samples/Microsoft.ML.Samples/Dynamic/ImageClassification/ResnetV2101TransferLearningTrainTestSplit.cs
@@ -56,11 +56,12 @@ public static void Example()
 
                 IDataView trainDataset = trainTestData.TrainSet;
                 IDataView testDataset = trainTestData.TestSet;
-                var validationSet = mlContext.Transforms.LoadImages("Image", fullImagesetFolderPath, new VectorDataViewType(NumberDataViewType.Byte), "ImagePath")
+                
+                var validationSet = mlContext.Transforms.LoadImages("Image", fullImagesetFolderPath, true, "ImagePath") // true indicates we want the image as a VBuffer<byte>
                     .Fit(testDataset)
                     .Transform(testDataset);
 
-                var pipeline = mlContext.Transforms.LoadImages("Image", fullImagesetFolderPath, new VectorDataViewType(NumberDataViewType.Byte), "ImagePath")
+                var pipeline = mlContext.Transforms.LoadImages("Image", fullImagesetFolderPath, true, "ImagePath") // true indicates we want the image as a VBuffer<byte>
                     .Append(mlContext.Model.ImageClassification(
                         "Image", "Label",
                         // Just by changing/selecting InceptionV3 here instead of 
diff --git a/pkg/Microsoft.ML.ImageAnalytics/Microsoft.ML.ImageAnalytics.nupkgproj b/pkg/Microsoft.ML.ImageAnalytics/Microsoft.ML.ImageAnalytics.nupkgproj
@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk" DefaultTargets="Pack">
 
   <PropertyGroup>
-    <TargetFramework>netstandard2.0</TargetFramework>
+    <TargetFrameworks>netstandard2.0;netstandard2.1;netcoreapp2.1</TargetFrameworks>
     <PackageDescription>ML.NET component for Image support</PackageDescription>
   </PropertyGroup>
 
diff --git a/src/Microsoft.ML.Core/Data/IEstimator.cs b/src/Microsoft.ML.Core/Data/IEstimator.cs
@@ -64,7 +64,7 @@ internal Column(string name, VectorKind vecKind, DataViewType itemType, bool isK
                 Contracts.CheckNonEmpty(name, nameof(name));
                 Contracts.CheckValueOrNull(annotations);
                 Contracts.CheckParam(!(itemType is KeyDataViewType), nameof(itemType), "Item type cannot be a key");
-                //Contracts.CheckParam(!(itemType is VectorDataViewType), nameof(itemType), "Item type cannot be a vector");
+                Contracts.CheckParam(!(itemType is VectorDataViewType), nameof(itemType), "Item type cannot be a vector");
                 Contracts.CheckParam(!isKey || KeyDataViewType.IsValidDataType(itemType.RawType), nameof(itemType), "The item type must be valid for a key");
 
                 Name = name;
diff --git a/src/Microsoft.ML.Dnn/ImageClassificationTransform.cs b/src/Microsoft.ML.Dnn/ImageClassificationTransform.cs
@@ -1208,7 +1208,7 @@ public SchemaShape GetOutputSchema(SchemaShape inputSchema)
                 if (!inputSchema.TryFindColumn(input, out var col))
                     throw _host.ExceptSchemaMismatch(nameof(inputSchema), "input", input);
                 var expectedType = _inputTypes[i];
-                if (!col.ItemType.Equals(expectedType))
+                if (!col.ItemType.Equals(expectedType.GetItemType()))
                     throw _host.ExceptSchemaMismatch(nameof(inputSchema), "input", input, expectedType.ToString(), col.ItemType.ToString());
             }
             for (var i = 0; i < _options.OutputColumns.Length; i++)
diff --git a/src/Microsoft.ML.ImageAnalytics/ExtensionsCatalog.cs b/src/Microsoft.ML.ImageAnalytics/ExtensionsCatalog.cs
@@ -107,14 +107,14 @@ internal static ImageLoadingEstimator LoadImages(this TransformsCatalog catalog,
         /// <param name="inputColumnName">Name of the column with paths to the images to load.
         /// This estimator operates over text data.</param>
         /// <param name="imageFolder">Folder where to look for images.</param>
-        /// <param name="type">Image type - VectorDataView type or ImageDataViewType. Defaults to ImageDataViewType if not specified or null.</param>
+        /// <param name="type">Image type flag - If true loads image as a VectorDataView type else loads image as ImageDataViewType. Defaults to ImageDataViewType if not specified or false.</param>
         /// <example>
         /// <format type="text/markdown">
         /// <![CDATA[
         ///  [!code-csharp[LoadImages](~/../docs/samples/docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/ImageAnalytics/LoadImages.cs)]
         /// ]]></format>
         /// </example>
-        public static ImageLoadingEstimator LoadImages(this TransformsCatalog catalog, string outputColumnName, string imageFolder, DataViewType type, string inputColumnName = null)
+        public static ImageLoadingEstimator LoadImages(this TransformsCatalog catalog, string outputColumnName, string imageFolder, bool type, string inputColumnName = null)
            => new ImageLoadingEstimator(CatalogUtils.GetEnvironment(catalog), imageFolder, type, new[] { (outputColumnName, inputColumnName ?? outputColumnName) });
 
         /// <summary>
@@ -129,7 +129,7 @@ public static ImageLoadingEstimator LoadImages(this TransformsCatalog catalog, s
         /// </remarks>
         /// <param name="catalog">The transform's catalog.</param>
         /// <param name="imageFolder">Folder where to look for images.</param>
-        /// <param name="type">Image type - VectorDataView type or ImageDataViewType. Defaults to ImageDataViewType if not specified or null.</param>
+        /// <param name="type">Image type flag - If true loads image as a VectorDataView type else loads image as ImageDataViewType. Defaults to ImageDataViewType if not specified or false.</param>
         /// <param name="columns">Specifies the names of the input columns for the transformation, and their respective output column names.</param>
         /// <example>
         /// <format type="text/markdown">
@@ -138,7 +138,7 @@ public static ImageLoadingEstimator LoadImages(this TransformsCatalog catalog, s
         /// ]]></format>
         /// </example>
         [BestFriend]
-        internal static ImageLoadingEstimator LoadImages(this TransformsCatalog catalog, string imageFolder, DataViewType type, params InputOutputColumnPair[] columns)
+        internal static ImageLoadingEstimator LoadImages(this TransformsCatalog catalog, string imageFolder, bool type, params InputOutputColumnPair[] columns)
         {
             var env = CatalogUtils.GetEnvironment(catalog);
             env.CheckValue(columns, nameof(columns));
diff --git a/src/Microsoft.ML.ImageAnalytics/ImageLoader.cs b/src/Microsoft.ML.ImageAnalytics/ImageLoader.cs
@@ -3,11 +3,12 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
-using System.Buffers;
 using System.Collections.Generic;
 using System.Drawing;
 using System.IO;
 using System.Linq;
+using System.Runtime.InteropServices;
+using System.Security.Cryptography;
 using System.Text;
 using Microsoft.ML;
 using Microsoft.ML.CommandLine;
@@ -71,10 +72,10 @@ internal sealed class Options : TransformInputBase
         /// </summary>
         public readonly string ImageFolder;
         /// <summary>
-        /// The DataViewType for the image. It can be a VectorDataView of bytes or ImageDataView type.
-        /// If no options are specified, it defaults to ImageDataView type.
+        /// The flag for DataViewType for the image. If Type true, it is a VectorDataView of bytes else it is an ImageDataView type.
+        /// If no options are specified, it defaults to false for ImageDataView type.
         /// </summary>
-        public readonly DataViewType Type;
+        public readonly bool Type;
 
         /// <summary>
         /// The columns passed to this <see cref="ITransformer"/>.
@@ -91,7 +92,7 @@ internal ImageLoadingTransformer(IHostEnvironment env, string imageFolder = null
             : base(Contracts.CheckRef(env, nameof(env)).Register(nameof(ImageLoadingTransformer)), columns)
         {
             ImageFolder = imageFolder;
-            Type = new ImageDataViewType();
+            Type = false;
         }
 
         /// <summary>
@@ -101,14 +102,11 @@ internal ImageLoadingTransformer(IHostEnvironment env, string imageFolder = null
         /// <param name="imageFolder">Folder where to look for images.</param>
         /// <param name="type">Image type - ImageDataViewType or VectorDataViewType. Defaults to ImageDataViewType if not specified.</param>
         /// <param name="columns">Names of input and output columns.</param>
-        internal ImageLoadingTransformer(IHostEnvironment env, string imageFolder = null, DataViewType type = null, params (string outputColumnName, string inputColumnName)[] columns)
+        internal ImageLoadingTransformer(IHostEnvironment env, string imageFolder = null, bool type = false, params (string outputColumnName, string inputColumnName)[] columns)
             : base(Contracts.CheckRef(env, nameof(env)).Register(nameof(ImageLoadingTransformer)), columns)
         {
             ImageFolder = imageFolder;
-            if (type == null)
-                Type = new ImageDataViewType();
-            else
-                Type = type;
+            Type = type;
         }
 
         // Factory method for SignatureDataTransform.
@@ -125,7 +123,7 @@ private static ImageLoadingTransformer Create(IHostEnvironment env, ModelLoadCon
             env.CheckValue(ctx, nameof(ctx));
 
             ctx.CheckAtModel(GetVersionInfo());
-            return new ImageLoadingTransformer(env.Register(nameof(ImageLoadingTransformer)), ctx );
+            return new ImageLoadingTransformer(env.Register(nameof(ImageLoadingTransformer)), ctx);
         }
 
         private ImageLoadingTransformer(IHost host, ModelLoadContext ctx)
@@ -137,13 +135,13 @@ private ImageLoadingTransformer(IHost host, ModelLoadContext ctx)
 
             ImageFolder = ctx.LoadStringOrNull();
 
-            if(new VectorDataViewType(NumberDataViewType.Byte).ToString().Equals(ctx.LoadStringOrNull()))
+            if (ctx.LoadStringOrNull().Equals("True"))
             {
-                Type = new VectorDataViewType(NumberDataViewType.Byte);
+                Type = true; // It is a VBuffer<byte> type
             }
             else
             {
-                Type = new ImageDataViewType();
+                Type = false; // It is a ImageDataViewType
             }
 
         }
@@ -195,9 +193,9 @@ private static VersionInfo GetVersionInfo()
         private sealed class Mapper : OneToOneMapperBase
         {
             private readonly ImageLoadingTransformer _parent;
-            private readonly DataViewType _type;
+            private readonly bool _type;
 
-            public Mapper(ImageLoadingTransformer parent, DataViewSchema inputSchema, DataViewType type)
+            public Mapper(ImageLoadingTransformer parent, DataViewSchema inputSchema, bool type)
                 : base(parent.Host.Register(nameof(Mapper)), parent, inputSchema)
             {
                 _type = type;
@@ -207,18 +205,18 @@ public Mapper(ImageLoadingTransformer parent, DataViewSchema inputSchema, DataVi
             protected override Delegate MakeGetter(DataViewRow input, int iinfo, Func<int, bool> activeOutput, out Action disposer)
             {
                 disposer = null;
-                // Check for the type of Image, VBuffer<bytes> or ImageDataViewType and call the appropriate MakeGetter function
-                if (new VectorDataViewType(NumberDataViewType.Byte).Equals(_type))
+                // Check for the type of Image, if true load images as VBuffer<bytes> else load images as ImageDataViewType
+                if (_type)
                 {
-                    return MakeGetterType(input, iinfo, activeOutput, (VectorDataViewType)_type, out disposer);
+                    return MakeGetterVectorDataViewByteType(input, iinfo, activeOutput, out disposer);
                 }
                 else
                 {
-                    return MakeGetterType(input, iinfo, activeOutput, (ImageDataViewType)_type, out disposer);
+                    return MakeGetterImageDataViewType(input, iinfo, activeOutput, out disposer);
                 }
             }
 
-            private Delegate MakeGetterType(DataViewRow input, int iinfo, Func<int, bool> activeOutput, ImageDataViewType type, out Action disposer)
+            private Delegate MakeGetterImageDataViewType(DataViewRow input, int iinfo, Func<int, bool> activeOutput, out Action disposer)
             {
                 Contracts.AssertValue(input);
                 Contracts.Assert(0 <= iinfo && iinfo < _parent.ColumnPairs.Length);
@@ -253,7 +251,7 @@ private Delegate MakeGetterType(DataViewRow input, int iinfo, Func<int, bool> ac
                 return del;
             }
 
-            private Delegate MakeGetterType(DataViewRow input, int iinfo, Func<int, bool> activeOutput, VectorDataViewType type, out Action disposer)
+            private Delegate MakeGetterVectorDataViewByteType(DataViewRow input, int iinfo, Func<int, bool> activeOutput, out Action disposer)
             {
                 Contracts.AssertValue(input);
                 Contracts.Assert(0 <= iinfo && iinfo < _parent.ColumnPairs.Length);
@@ -283,6 +281,7 @@ private Delegate MakeGetterType(DataViewRow input, int iinfo, Func<int, bool> ac
             public static int LoadDataIntoBuffer(string path, ref VBuffer<byte> imgData)
             {
                 int count = -1;
+                int bytesread = -1;
                 // bufferSize == 1 used to avoid unnecessary buffer in FileStream
                 using (FileStream fs = new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.Read, bufferSize: 1))
                 {
@@ -291,34 +290,68 @@ public static int LoadDataIntoBuffer(string path, ref VBuffer<byte> imgData)
                         throw new IOException($"File {path} too big to open.");
                     else if (fileLength == 0)
                     {
+                        Console.WriteLine("File length is zero even though the image is not empty");
                         byte[] imageBuffer;
                         // Some file systems (e.g. procfs on Linux) return 0 for length even when there's content.
                         // Thus we need to assume 0 doesn't mean empty.
                         imageBuffer = File.ReadAllBytes(path);
                         count = imageBuffer.Length;
-                        Console.WriteLine("File length is zero");
+                        imgData = new VBuffer<byte>(count,imageBuffer);
+                        return count;
                     }
 
                     count = (int)fileLength;
+                    var editor = VBufferEditor.Create(ref imgData, count);
 
 #if NETSTANDARD2_0
-                    byte[] buffer = null;
-                    buffer = File.ReadAllBytes(path);
-                    imgData = new VBuffer<byte>(buffer.Length, buffer);
+                    bytesread = ReadToEnd(fs, editor.Values);
+                    Contracts.Assert(count == bytesread);
 
 #else
-                    var editor = VBufferEditor.Create(ref imgData, count);
                     fs.Read(editor.Values);
-                    imgData = editor.Commit();
+                    bytesread = editor.Values.Length;
+                    Contracts.Assert(count == bytesread);
 #endif
+                    imgData = editor.Commit();
                     return count;
 
                 }
 
             }
 
+            public static int ReadToEnd(System.IO.Stream stream, Span<byte> bufferspan)
+            {
+
+                int chunksize = 4096; // Most optimal size for buffer, friendly to CPU's L1 cache
+                byte[] readBuffer = new byte[chunksize];
+                int totalBytesRead = 0;
+                int bytesRead;
+                unsafe
+                {
+                    fixed (byte* readBufferPtr = readBuffer)
+                    fixed (byte* bufferSpanPtr = &MemoryMarshal.GetReference(bufferspan))
+                    {
+                        while ((bytesRead = stream.Read(readBuffer, 0, readBuffer.Length)) > 0)
+                        {
+                            Buffer.MemoryCopy(readBufferPtr, bufferSpanPtr + totalBytesRead, bufferspan.Length - totalBytesRead, bytesRead);
+                            totalBytesRead += bytesRead;
+                        }
+                    }
+                }
+                return totalBytesRead;
+
+            }
+
+            public DataViewType GetDataViewType()
+            {
+                if (_type)
+                    return new VectorDataViewType(NumberDataViewType.Byte);
+                else
+                    return  new ImageDataViewType();
+            }
+
             protected override DataViewSchema.DetachedColumn[] GetOutputColumnsCore()
-                => _parent.ColumnPairs.Select(x => new DataViewSchema.DetachedColumn(x.outputColumnName, _type, null)).ToArray();
+                => _parent.ColumnPairs.Select(x => new DataViewSchema.DetachedColumn(x.outputColumnName, GetDataViewType(), null)).ToArray();
         }
     }
 
@@ -371,18 +404,18 @@ internal ImageLoadingEstimator(IHostEnvironment env, string imageFolder, params
         /// <param name="imageFolder">Folder where to look for images.</param>
         /// <param name="type">Image type - VectorDataView type or ImageDataViewType. Defaults to ImageDataViewType if not specified or null.</param>
         /// <param name="columns">Names of input and output columns.</param>
-        internal ImageLoadingEstimator(IHostEnvironment env, string imageFolder, DataViewType type = null, params (string outputColumnName, string inputColumnName)[] columns)
+        internal ImageLoadingEstimator(IHostEnvironment env, string imageFolder, bool type = false, params (string outputColumnName, string inputColumnName)[] columns)
             : this(env, new ImageLoadingTransformer(env, imageFolder, type, columns), type)
         {
         }
 
-        internal ImageLoadingEstimator(IHostEnvironment env, ImageLoadingTransformer transformer, DataViewType type = null)
+        internal ImageLoadingEstimator(IHostEnvironment env, ImageLoadingTransformer transformer, bool type = false)
             : base(Contracts.CheckRef(env, nameof(env)).Register(nameof(ImageLoadingEstimator)), transformer)
         {
-            if (type == null)
+            if (!type)
                 _imageType = new ImageDataViewType();
             else
-                _imageType = type;
+                _imageType = new VectorDataViewType(NumberDataViewType.Byte);
         }
 
         /// <summary>
@@ -400,7 +433,10 @@ public override SchemaShape GetOutputSchema(SchemaShape inputSchema)
                 if (!(col.ItemType is TextDataViewType) || col.Kind != SchemaShape.Column.VectorKind.Scalar)
                     throw Host.ExceptSchemaMismatch(nameof(inputSchema), "input", inputColumnName, TextDataViewType.Instance.ToString(), col.GetTypeString());
 
-                result[outputColumnName] = new SchemaShape.Column(outputColumnName, SchemaShape.Column.VectorKind.Scalar, _imageType, false);
+                if (_imageType is ImageDataViewType)
+                    result[outputColumnName] = new SchemaShape.Column(outputColumnName, SchemaShape.Column.VectorKind.Scalar, _imageType, false);
+                else
+                    result[outputColumnName] = new SchemaShape.Column(outputColumnName, SchemaShape.Column.VectorKind.Vector, NumberDataViewType.Byte, false);
             }
 
             return new SchemaShape(result.Values);
diff --git a/src/Microsoft.ML.ImageAnalytics/Microsoft.ML.ImageAnalytics.csproj b/src/Microsoft.ML.ImageAnalytics/Microsoft.ML.ImageAnalytics.csproj
@@ -3,6 +3,7 @@
   <PropertyGroup>
     <TargetFrameworks>netstandard2.0;netstandard2.1;netcoreapp2.1</TargetFrameworks>
     <IncludeInPackage>Microsoft.ML.ImageAnalytics</IncludeInPackage>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
   </PropertyGroup>
 
   <ItemGroup>
@@ -13,11 +14,5 @@
     <ProjectReference Include="..\Microsoft.ML.Core\Microsoft.ML.Core.csproj" />
     <ProjectReference Include="..\Microsoft.ML.Data\Microsoft.ML.Data.csproj" />
   </ItemGroup>
-
-  <ItemGroup>
-    <Reference Include="System.Buffers">
-      <HintPath>..\..\..\..\..\..\Program Files\dotnet\packs\Microsoft.NETCore.App.Ref\3.0.0-preview8-28405-07\ref\netcoreapp3.0\System.Buffers.dll</HintPath>
-    </Reference>
-  </ItemGroup>
   
 </Project>
diff --git a/test/Microsoft.ML.Tests/ScenariosWithDirectInstantiation/TensorflowTests.cs b/test/Microsoft.ML.Tests/ScenariosWithDirectInstantiation/TensorflowTests.cs

Original file line number	Diff line number	Diff line change
`@@ -1208,7 +1208,7 @@ public SchemaShape GetOutputSchema(SchemaShape inputSchema)`
`1208`	`1208`	`if (!inputSchema.TryFindColumn(input, out var col))`
`1209`	`1209`	`throw _host.ExceptSchemaMismatch(nameof(inputSchema), "input", input);`
`1210`	`1210`	`var expectedType = _inputTypes[i];`
`1211`		`- if (!col.ItemType.Equals(expectedType))`
	`1211`	`+ if (!col.ItemType.Equals(expectedType.GetItemType()))`
`1212`	`1212`	`throw _host.ExceptSchemaMismatch(nameof(inputSchema), "input", input, expectedType.ToString(), col.ItemType.ToString());`
`1213`	`1213`	`}`
`1214`	`1214`	`for (var i = 0; i < _options.OutputColumns.Length; i++)`