-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Image analytics documentation, samples, internalization #2372
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
958d05a
to
6fe57a6
Compare
var imagesFolder = Path.GetDirectoryName(imagesDataFile); | ||
// Image loading pipeline. | ||
var pipeline = mlContext.Transforms.LoadImages(imagesFolder, ("ImageReal", "ImagePath")) | ||
.Append(mlContext.Transforms.Resize()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mlContext.Transforms.Resize() [](start = 32, length = 29)
First, I'm offended what I'm not even invited to review this PR! -_-
How Resize()
work without specifying column name? And without new image dimensions? #Closed
// Image loading pipeline. | ||
var pipeline = mlContext.Transforms.LoadImages(imagesFolder, ("ImageObject", "ImagePath")) | ||
.Append(mlContext.Transforms.Resize("ImageObject",imageWidth: 100 , imageHeight: 100 )) | ||
.Append(mlContext.Transforms.ExtractPixels("Pixels", "ImageObject")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Transforms [](start = 44, length = 10)
Considering specificity of domain, can we have Image subcatalog, like we have ml.Transforms.Text,
can we have ml.Transforms.Images
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i might misremember, but Pete disliked the idea; i believe becuase of too many catalogs makes things hard to find; but @artidoro and @rogancarr have expressed the same thing: issue #2361
If i get another thumbs up from @TomFinley, i am up for adding the catalog and moving the extensions there.
I am on the fence, personally. If we have more than 5 xtensions, i'd make a catalog for them. Less than that feels unecessary.
In reply to: 253166949 [](ancestors = 253166949)
{ | ||
public class ConvertToGrayScaleExample | ||
{ | ||
public static void ConvertToGrayScale() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GrayScale [](start = 36, length = 9)
why GrayS
cale? is'nt grayscale one word? #Closed
// Preview of the content of the images.tsv file | ||
// | ||
// imagePath imageType | ||
// tomato.bmp tomato |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[](start = 25, length = 1)
is this a tab?
don't you build file manually? #Closed
Loads the images from a given folder into memory as an <see cref="ImageType"/> | ||
</summary> | ||
<remarks> | ||
The images to load are present in the folder specfied as imageFolder. The get loaded in memory as a <see cref="System.Drawing.Bitmap" /> type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The get [](start = 78, length = 7)
They get? #Closed
/// <summary> | ||
/// Converts the images to grayscale. | ||
/// </summary> | ||
/// <include file='doc.xml' path='doc/members/member[@name="ImageLoadingEstimator"]/*' /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ImageLoadingEstimator [](start = 68, length = 21)
ImageGrayscalingEstimator? #Closed
/// <param name="outputColumnName"> Name of the column resulting from the transformation of <paramref name="inputColumnName"/>.</param> | ||
/// <param name="inputColumnName"> Name of column to transform. If set to <see langword="null"/>, the value of the <paramref name="outputColumnName"/> will be used as source.</param> | ||
/// <param name="colors">The color schema as defined in <see cref="ImagePixelExtractingEstimator.ColorBits"/>.</param> | ||
/// <param name="interleave">Wheather to interleave the pixels, meaning keep them in the `ARGB ARGB` order, or leave them separated in the plannar form.</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
plannar [](start = 147, length = 7)
planar.
I still find this description confusing. Best I can come up with is : if set to false we will produce array where first we output values for first channel for all pixel then for second channel and so on, if set to true we will output values for pixels in channel tuples
. But it's still probably confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// <param name="catalog"> The transform's catalog.</param> | ||
/// <param name="outputColumnName"> Name of the column resulting from the transformation of <paramref name="inputColumnName"/>.</param> | ||
/// <param name="inputColumnName"> Name of column to transform. If set to <see langword="null"/>, the value of the <paramref name="outputColumnName"/> will be used as source.</param> | ||
/// <param name="colors">The color schema as defined in <see cref="ImagePixelExtractingEstimator.ColorBits"/>.</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The color schema as defined in . [](start = 33, length = 85)
Specifies which colors to extract from image. Order of colors is always Alpha, Red, Green Blue. #Closed
// Image loading pipeline. | ||
var pipeline = mlContext.Transforms.LoadImages(imagesFolder, ("ImageObject", "ImagePath")) | ||
.Append(mlContext.Transforms.Resize("ImageObject",imageWidth: 100 , imageHeight: 100 )) | ||
.Append(mlContext.Transforms.ExtractPixels("Pixels", "ImageObject")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Anipik: We could make a good benchmark for the image processing pipeline.
I'd recommend using the Dog Breeds vs. Fruits dataset which we used in NimbusML for its image examples. We currently host this dataset in our CDN for NimbusML.
In Python, the dataset / image loader looks like:
# Load image summary data from github
url = "https://express-tlcresources.azureedge.net/datasets/DogBreedsVsFruits/DogFruitWiki.SHUF.117KB.735-rows.tsv"
df_train = pd.read_csv(url, sep = "\t", nrows = 100)
df_train['ImagePath_full'] = "https://express-tlcresources.azureedge.net/datasets/DogBreedsVsFruits/" + \
df_train['ImagePath']
... load images
Purpose of the dataset is for example code & includes ~775 images of dogs & fruit:
The Dog Breeds vs. Fruits would also be nice for our samples repo.
/cc @CESARDELATORRE #Resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jormont, maybe log an issue about it. It might get forgotten here as a comment.
In reply to: 253212682 [](ancestors = 253212682)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sfilipi -- good idea:
/// Converts the images to grayscale. | ||
/// </summary> | ||
/// <include file='doc.xml' path='doc/members/member[@name="ImageGrayscalingEstimator"]/*' /> | ||
[BestFriend] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BestFriend [](start = 5, length = 10)
Why BestFriend, it's still public #Resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -433,7 +267,7 @@ public override void Save(ModelSaveContext ctx) | |||
|
|||
base.SaveColumns(ctx); | |||
|
|||
foreach (ColumnInfo info in _columns) | |||
foreach (ImagePixelExtractingEstimator.ColumnInfo info in _columns) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ImagePixelExtractingEstimator.ColumnInfo [](start = 21, length = 40)
var? #Resolved
public readonly bool AsFloat; | ||
|
||
internal readonly byte Planes; | ||
public bool Alpha => (Colors & ColorBits.Alpha) != 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public [](start = 12, length = 6)
all other channels are internal #Resolved
Interleave = ctx.Reader.ReadBoolByte(); | ||
} | ||
|
||
public void Save(ModelSaveContext ctx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public [](start = 12, length = 6)
should be internal #Pending
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't change it atm, as it is public in the interface. Artidoro opened an issue about it.
In reply to: 253243474 [](ancestors = 253243474)
[assembly: LoadableClass(typeof(IRowMapper), typeof(ImageResizerTransformer), null, typeof(SignatureLoadRowMapper), | ||
ImageResizerTransformer.UserName, ImageResizerTransformer.LoaderSignature)] | ||
[assembly: LoadableClass(typeof(IRowMapper), typeof(ImageResizingTransformer), null, typeof(SignatureLoadRowMapper), | ||
ImageResizingTransformer.UserName, ImageResizingTransformer.LoaderSignature)] | ||
|
||
namespace Microsoft.ML.ImageAnalytics | ||
{ | ||
// REVIEW: Rewrite as LambdaTransform to simplify. | ||
/// <summary> | ||
/// Transform which takes one or many columns of <see cref="ImageType"/> and resize them to provided height and width. | ||
/// </summary> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// ? #Resolved
@@ -456,8 +401,97 @@ protected override Delegate MakeGetter(Row input, int iinfo, Func<int, bool> act | |||
/// <summary> | |||
/// Estimator which resizes images. | |||
/// </summary> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// ? #Resolved
I think @artidoro has tendency to add
|
@@ -6,5 +6,6 @@ | |||
using Microsoft.ML; | |||
|
|||
[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.StaticPipe" + PublicKey.Value)] | |||
[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.Tests" + PublicKey.TestValue)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#2306
Maybe we should convert all direct calls to estimators/transformers to calls to mlContext extensions?
feel free to ignore. #Pending
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we doing that now, or post V1? i agree, but might make more sense to spend the time in other things and do this later.
In reply to: 253244221 [](ancestors = 253244221)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Loading the images in memory is the first step of almost every pipeline that does image processing, and further analysis on images. | ||
The images to load need to be in the formats supported by <see cref="System.Drawing.Bitmap" />. | ||
|
||
For end-to-end image processing pipelines, and scenarios in your applications, see the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove comment - For end-to-end image processing pipelines and scenarios in your application, see the...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
<member name="ImagePixelExtractingEstimator"> | ||
<summary> | ||
Extracts the pixels from the input images and, converts them into a vector of numbers, than can be further used as feature by the algorithms added to the pipeline. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove comment - Extracts the pixels from the input images and converts them into a vector of numbers.
Also rather "than can be further", I would say "This can be further used as a ..." #Resolved
Extracts the pixels from the input images and, converts them into a vector of numbers, than can be further used as feature by the algorithms added to the pipeline. | ||
</summary> | ||
<remarks> | ||
The ImagePixelExtractingEstimator extracts the pixels from the input images and, converts them into a vector of numbers, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment here about comments and "This can be further..." #Resolved
/// <example> | ||
/// <format type="text/markdown"> | ||
/// <] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the file is called ConvertToGrayScale.cs - does casing matter? Should ConvertToGrayScale.cs be renamed? #Resolved
/// <param name="catalog"> The transform's catalog.</param> | ||
/// <param name="outputColumnName"> Name of the column resulting from the transformation of <paramref name="inputColumnName"/>.</param> | ||
/// <param name="inputColumnName"> Name of column to transform. If set to <see langword="null"/>, the value of the <paramref name="outputColumnName"/> will be used as source.</param> | ||
/// <param name="colors"> Specifies which colors to extract from the image. The order of colors is: Alpha, Red, Green Blue.</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
colors [](start = 25, length = 6)
This looks like an enum. Is there an enum cref? #Resolved
/// <param name="outputColumnName"> Name of the column resulting from the transformation of <paramref name="inputColumnName"/>.</param> | ||
/// <param name="inputColumnName"> Name of column to transform. If set to <see langword="null"/>, the value of the <paramref name="outputColumnName"/> will be used as source.</param> | ||
/// <param name="colors"> Specifies which colors to extract from the image. The order of colors is: Alpha, Red, Green Blue.</param> | ||
/// <param name="interleave">Wheather to interleave the pixels, meaning keep them in the `ARGB ARGB` order, or leave them separated in the planar form.</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
planar [](start = 147, length = 6)
What is planar form? Is there some context that should already be known by the reader? If not, is it worth talking about this in the remarks section? Or maybe having just some more verbage in this description? #Resolved
//Preview of the transformedData | ||
var transformedDataPreview = transformedData.Preview(); | ||
|
||
// Preview of the content of the images.tsv file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preview of the content of the images.tsv file [](start = 15, length = 45)
The output shown below is not the contents of images.tsv file instead its transformedData. #Resolved
string outputColumnName, | ||
string inputColumnName = null, | ||
ImagePixelExtractingEstimator.ColorBits colors = ImagePixelExtractingEstimator.ColorBits.Rgb, | ||
bool interleave = false, float scale = ImagePixelExtractingTransformer.Defaults.Scale, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, I would put scale on a separate line #Resolved
var transformedDataPreview = transformedData.Preview(); | ||
|
||
// Preview of the content of the images.tsv file | ||
// The actual images, in the ImageReal column are of type System.Drawing.Bitmap. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ImageReal [](start = 41, length = 9)
typo #Resolved
var transformedDataPreview = transformedData.Preview(); | ||
|
||
// Preview of the content of the images.tsv file | ||
// The actual images, in the ImageReal column are of type System.Drawing.Bitmap. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ImageReal [](start = 41, length = 9)
typo #Resolved
/// Those pre-trained models have a defined width and height for their input images, so often, after getting loaded, the images will need to get resized before | ||
/// further processing. | ||
/// The new width and height, as well as other properties of resizing, like type of scaling (uniform, or non-uniform), and whether to pad the image, | ||
/// or just crop it can be specidied separately for each column loaded, through the <see cref="ImageResizingEstimator.ColumnInfo"/>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
specidied [](start = 35, length = 9)
specified #Resolved
/// <example> | ||
/// <format type="text/markdown"> | ||
/// <] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resize? #Resolved
/// </summary> | ||
public sealed class ImagePixelExtractorTransformer : OneToOneTransformerBase | ||
/// <remarks> | ||
/// During the transformation, the columns of <see cref="ImageType"/> are converted them into a vector representign the image pixels |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
representign [](start = 108, length = 12)
representing #Resolved
/// <param name="name">Name of the column resulting from the transformation of <paramref name="inputColumnName"/>.</param> | ||
/// <param name="inputColumnName">Name of column to transform. If set to <see langword="null"/>, the value of the <paramref name="name"/> will be used as source.</param> | ||
/// <param name="colors">What colors to extract.</param> | ||
/// <param name="interleave">Wheather to interleave the pixels, meaning keep them in the `RGB RGB` order, or leave them in the plannar form: of all red pixels, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whether #Resolved
/// <param name="inputColumnName">Name of column to transform. If set to <see langword="null"/>, the value of the <paramref name="name"/> will be used as source.</param> | ||
/// <param name="colors">What colors to extract.</param> | ||
/// <param name="interleave">Wheather to interleave the pixels, meaning keep them in the `RGB RGB` order, or leave them in the plannar form: of all red pixels, | ||
/// than all green, than all blue.</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then all green, then all blue #Resolved
///<summary> | ||
/// Extract pixels values from image and produce array of values. | ||
///</summary> | ||
/// <param name="env">The host environment.</param> | ||
/// <param name="outputColumnName">Name of the column resulting from the transformation of <paramref name="inputColumnName"/>. Null means <paramref name="inputColumnName"/> is replaced.</param> | ||
/// <param name="inputColumnName">Name of the input column.</param> | ||
/// <param name="colors">What colors to extract.</param> | ||
/// <param name="interleave"></param> | ||
/// <param name="interleave">Wheather to interleave the pixels, meaning keep them in the `RGB RGB` order, or leave them in the plannar form: of all red pixels, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wheather [](start = 37, length = 8)
Whether
then all green, then all blue #Resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a technique known as <a href="http://www.stat.harvard.edu/Faculty_Content/meng/JCGS01.pdf"> data augmentation</a>. | ||
|
||
For end-to-end image processing pipelines, and scenarios in your applications, see the | ||
<a href="https://github.com/dotnet/machinelearning-samples/tree/master/samples/csharp/getting-started">examples in the machinelearning-samples github repository.</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<a href="https://github.com/dotnet/ [](start = 10, length = 35)
this is xml style. you're in a markdown block. would this work? #Pending
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, the tag it's html. I mostly added this here to have an example in the solution of doing markdown in a document.
In reply to: 253699067 [](ancestors = 253699067)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's in markdown, would a normal markdown link work?
<a href="https://github.com/dotnet/machinelearning-samples/tree/master/samples/csharp/getting-started">examples in the machinelearning-samples github repository.</a> | |
[examples in the machinelearning-samples github repository](https://github.com/dotnet/machinelearning-samples/tree/master/samples/csharp/getting-started). |
Also, the period could be outside the href:
<a href="https://github.com/dotnet/machinelearning-samples/tree/master/samples/csharp/getting-started">examples in the machinelearning-samples github repository.</a> | |
<a href="https://github.com/dotnet/machinelearning-samples/tree/master/samples/csharp/getting-started">examples in the machinelearning-samples github repository</a>. | |
``` #Resolved |
<a href="https://github.com/dotnet/machinelearning-samples/tree/master/samples/csharp/getting-started">examples in the machinelearning-samples github repository.</a> | ||
]]></format> | ||
<seealso cref="ImageEstimatorsCatalog" /> | ||
<seealso cref="ImageLoadingEstimator"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should discuss these. please see my comments in Teams.
The images might be converted to grayscale to reduce the complexity of the model. | ||
The grayed out images contain less information to process than the colored images. | ||
Another use case for converting to grayscale is to generate new images out of the existing ones, so you can have a larger dataset, | ||
a technique known as <a href="http://www.stat.harvard.edu/Faculty_Content/meng/JCGS01.pdf"> data augmentation</a>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a technique known as <a href="http://www.stat.harvard.edu/Faculty_Content/meng/JCGS01.pdf"> data augmentation</a>. | |
a technique known as <a href="http://www.stat.harvard.edu/Faculty_Content/meng/JCGS01.pdf">data augmentation</a>. | |
``` #Resolved |
var imagesFolder = Path.GetDirectoryName(imagesDataFile); | ||
// Image loading pipeline. | ||
var pipeline = mlContext.Transforms.LoadImages(imagesFolder, ("ImageObject", "ImagePath")) | ||
// .Append(mlContext.Transforms.Resize("ImageObject",imageWidth: 100 , imageHeight: 100 )) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra commented out line. #Resolved
Codecov Report
@@ Coverage Diff @@
## master #2372 +/- ##
==========================================
+ Coverage 71.25% 71.25% +<.01%
==========================================
Files 783 785 +2
Lines 140781 140885 +104
Branches 16088 16103 +15
==========================================
+ Hits 100311 100388 +77
- Misses 36016 36032 +16
- Partials 4454 4465 +11
|
Addressing both:
#1209 in the first commit by adding samples, and documenting the classes and the extensions better.
#1798 in the second commit by:
1- Internalizing constructors of estimators and transformers
2- Keep ColumnInfo and move it to to the estimators #1760
3- Rename Arguments -> Options