Creation of components through MLContext and cleanup (text transform) #2394
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,34 +20,32 @@ public static class TextCatalog | |
| /// <param name="catalog">The text-related 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 the column to transform. If set to <see langword="null"/>, the value of the <paramref name="outputColumnName"/> will be used as source.</param> | ||
|
There was a problem hiding this comment.
you removing this example becaue it doesn't use this exact method? #Resolved There was a problem hiding this comment. yeap. removed it from here, and added to the API below (this was one of Senja's comments) In reply to: 254802756 [](ancestors = 254802756) |
||
| public static TextFeaturizingEstimator FeaturizeText(this TransformsCatalog.TextTransforms catalog, | ||
| string outputColumnName, | ||
| string inputColumnName = null) | ||
| => new TextFeaturizingEstimator(Contracts.CheckRef(catalog, nameof(catalog)).GetEnvironment(), | ||
| outputColumnName, inputColumnName); | ||
|
|
||
| /// <summary> | ||
| /// Transform several text columns into featurized float array that represents counts of ngrams and char-grams. | ||
| /// </summary> | ||
| /// <param name="catalog">The text-related transform's catalog.</param> | ||
| /// <param name="outputColumnName">Name of the column resulting from the transformation of <paramref name="inputColumnNames"/>.</param> | ||
| /// <param name="inputColumnNames">Name of the columns to transform. If set to <see langword="null"/>, the value of the <paramref name="outputColumnName"/> will be used as source.</param> | ||
| /// <param name="options">Advanced options to the algorithm.</param> | ||
| /// <example> | ||
| /// <format type="text/markdown"> | ||
| /// <] | ||
| /// ]]> | ||
| /// </format> | ||
| /// </example> | ||
| public static TextFeaturizingEstimator FeaturizeText(this TransformsCatalog.TextTransforms catalog, | ||
|
There was a problem hiding this comment.
link the sample to this one, since it illustrates the usage of Options. #Resolved |
||
| string outputColumnName, | ||
| IEnumerable<string> inputColumnNames, | ||
|
There was a problem hiding this comment.
I know it is not your change, but can we go with params, rather than an IEnumerable? Looking at the sample, having to construct a List is overkill. #Pending |
||
| TextFeaturizingEstimator.Options options) | ||
| => new TextFeaturizingEstimator(Contracts.CheckRef(catalog, nameof(catalog)).GetEnvironment(), | ||
| outputColumnName, inputColumnNames, options); | ||
|
|
||
| /// <summary> | ||
| /// Tokenize incoming text in <paramref name="inputColumnName"/> and output the tokens as <paramref name="outputColumnName"/>. | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -120,26 +120,59 @@ internal sealed class Arguments : TransformInputBase | |
| public TextNormKind VectorNormalizer = TextNormKind.L2; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Advanced options for the <see cref="TextFeaturizingEstimator"/>. | ||
| /// </summary> | ||
| public sealed class Options | ||
|
There was a problem hiding this comment. Can you add a summary tag for each of these settings, and for the Options class? You can take them from the constructor I believe. #Resolved There was a problem hiding this comment. There was a problem hiding this comment.
can you remove this, and use the Arguments above, like for everything else? make the Factories internal, so they are not visible. #Resolved There was a problem hiding this comment. I think we don't want to do that. In reply to: 254358363 [](ancestors = 254358363) There was a problem hiding this comment. perhaps we should do some of this cleanup outside purview of this PR ... I need to understand properly how the factories are used etc. In reply to: 254461580 [](ancestors = 254461580,254358363) |
||
| { | ||
| #pragma warning disable MSML_NoInstanceInitializers // No initializers on instance fields or properties | ||
| /// <summary> | ||
| /// Dataset language. | ||
| /// </summary> | ||
| public Language TextLanguage { get; set; } = DefaultLanguage; | ||
| /// <summary> | ||
| /// Casing used for the text. | ||
| /// </summary> | ||
| public CaseNormalizationMode TextCase { get; set; } = CaseNormalizationMode.Lower; | ||
| /// <summary> | ||
| /// Whether to keep diacritical marks or remove them. | ||
| /// </summary> | ||
| public bool KeepDiacritics { get; set; } = false; | ||
| /// <summary> | ||
| /// Whether to keep punctuation marks or remove them. | ||
| /// </summary> | ||
| public bool KeepPunctuations { get; set; } = true; | ||
| /// <summary> | ||
| /// Whether to keep numbers or remove them. | ||
| /// </summary> | ||
| public bool KeepNumbers { get; set; } = true; | ||
| /// <summary> | ||
| /// Whether to output the transformed text tokens as an additional column. | ||
| /// </summary> | ||
| public bool OutputTokens { get; set; } = false; | ||
| /// <summary> | ||
| /// Vector Normalizer to use. | ||
| /// </summary> | ||
| public TextNormKind VectorNormalizer { get; set; } = TextNormKind.L2; | ||
| /// <summary> | ||
| /// Whether to use stop remover or not. | ||
| /// </summary> | ||
| public bool UseStopRemover { get; set; } = false; | ||
| /// <summary> | ||
| /// Whether to use char extractor or not. | ||
| /// </summary> | ||
| public bool UseCharExtractor { get; set; } = true; | ||
| /// <summary> | ||
| /// Whether to use word extractor or not. | ||
| /// </summary> | ||
| public bool UseWordExtractor { get; set; } = true; | ||
| #pragma warning restore MSML_NoInstanceInitializers // No initializers on instance fields or properties | ||
| } | ||
|
There was a problem hiding this comment. This whole thing requires proper cleaning/documentation. Do you want to do it in this PR, or you prefer leave it for later cleaning? #Pending There was a problem hiding this comment. The current PR is to fix the public API We are doing some cleaning/doc fixes on an opportunistic basis. But we should have a separate issue / PRs for "proper" cleaning and documentation In reply to: 254013725 [](ancestors = 254013725) |
||
|
|
||
| internal readonly string OutputColumn; | ||
| private readonly string[] _inputColumns; | ||
| internal IReadOnlyCollection<string> InputColumns => _inputColumns.AsReadOnly(); | ||
| internal Options OptionalSettings { get; } | ||
|
|
||
| // These parameters are hardcoded for now. | ||
| // REVIEW: expose them once sub-transforms are estimators. | ||
|
|
@@ -232,18 +265,18 @@ public bool NeedInitialSourceColumnConcatTransform | |
| public TransformApplierParams(TextFeaturizingEstimator parent) | ||
| { | ||
| var host = parent._host; | ||
| host.Check(Enum.IsDefined(typeof(Language), parent.OptionalSettings.TextLanguage)); | ||
| host.Check(Enum.IsDefined(typeof(CaseNormalizationMode), parent.OptionalSettings.TextCase)); | ||
| WordExtractorFactory = parent._wordFeatureExtractor?.CreateComponent(host, parent._dictionary); | ||
| CharExtractorFactory = parent._charFeatureExtractor?.CreateComponent(host, parent._dictionary); | ||
| VectorNormalizer = parent.OptionalSettings.VectorNormalizer; | ||
| Language = parent.OptionalSettings.TextLanguage; | ||
| UsePredefinedStopWordRemover = parent.OptionalSettings.UseStopRemover; | ||
| TextCase = parent.OptionalSettings.TextCase; | ||
| KeepDiacritics = parent.OptionalSettings.KeepDiacritics; | ||
| KeepPunctuations = parent.OptionalSettings.KeepPunctuations; | ||
| KeepNumbers = parent.OptionalSettings.KeepNumbers; | ||
| OutputTextTokens = parent.OptionalSettings.OutputTokens; | ||
| Dictionary = parent._dictionary; | ||
| } | ||
| } | ||
|
|
@@ -254,40 +287,42 @@ public TransformApplierParams(TextFeaturizingEstimator parent) | |
| internal const string UserName = "Text Transform"; | ||
| internal const string LoaderSignature = "Text"; | ||
|
|
||
| internal const Language DefaultLanguage = Language.English; | ||
|
|
||
| private const string TransformedTextColFormat = "{0}_TransformedText"; | ||
|
|
||
| internal TextFeaturizingEstimator(IHostEnvironment env, string outputColumnName, string inputColumnName = null) | ||
| : this(env, outputColumnName, new[] { inputColumnName ?? outputColumnName }) | ||
| { | ||
| } | ||
|
|
||
| internal TextFeaturizingEstimator(IHostEnvironment env, string name, IEnumerable<string> source, Options options = null) | ||
| { | ||
| Contracts.CheckValue(env, nameof(env)); | ||
| _host = env.Register(nameof(TextFeaturizingEstimator)); | ||
| _host.CheckValue(source, nameof(source)); | ||
| _host.CheckParam(source.Any(), nameof(source)); | ||
| _host.CheckParam(!source.Any(string.IsNullOrWhiteSpace), nameof(source)); | ||
| _host.CheckNonEmpty(name, nameof(name)); | ||
| _host.CheckValueOrNull(options); | ||
|
|
||
| _inputColumns = source.ToArray(); | ||
| OutputColumn = name; | ||
|
|
||
| OptionalSettings = new Options(); | ||
| if (options != null) | ||
| OptionalSettings = options; | ||
|
|
||
| _dictionary = null; | ||
| if (OptionalSettings.UseWordExtractor) | ||
| _wordFeatureExtractor = new NgramExtractorTransform.NgramExtractorArguments(); | ||
| if (OptionalSettings.UseCharExtractor) | ||
| _charFeatureExtractor = new NgramExtractorTransform.NgramExtractorArguments() { NgramLength = 3, AllLengths = false }; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Trains and returns a <see cref="Transformer"/>. | ||
| /// </summary> | ||
| public ITransformer Fit(IDataView input) | ||
| { | ||
| var h = _host; | ||
|
|
@@ -463,14 +498,18 @@ public ITransformer Fit(IDataView input) | |
| return new Transformer(_host, input, view); | ||
| } | ||
|
|
||
| private static ITransformer Create(IHostEnvironment env, ModelLoadContext ctx) | ||
| => new Transformer(env, ctx); | ||
|
|
||
| private static string GenerateColumnName(Schema schema, string srcName, string xfTag) | ||
| { | ||
| return schema.GetTempColumnName(string.Format("{0}_{1}", srcName, xfTag)); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Returns the <see cref="SchemaShape"/> of the schema which will be produced by the transformer. | ||
| /// Used for schema propagation and verification in a pipeline. | ||
| /// </summary> | ||
| public SchemaShape GetOutputSchema(SchemaShape inputSchema) | ||
| { | ||
| _host.CheckValue(inputSchema, nameof(inputSchema)); | ||
|
|
@@ -485,12 +524,12 @@ public SchemaShape GetOutputSchema(SchemaShape inputSchema) | |
|
|
||
| var metadata = new List<SchemaShape.Column>(2); | ||
| metadata.Add(new SchemaShape.Column(MetadataUtils.Kinds.SlotNames, SchemaShape.Column.VectorKind.Vector, TextType.Instance, false)); | ||
| if (OptionalSettings.VectorNormalizer != TextNormKind.None) | ||
| metadata.Add(new SchemaShape.Column(MetadataUtils.Kinds.IsNormalized, SchemaShape.Column.VectorKind.Scalar, BoolType.Instance, false)); | ||
|
|
||
| result[OutputColumn] = new SchemaShape.Column(OutputColumn, SchemaShape.Column.VectorKind.Vector, NumberType.R4, false, | ||
| new SchemaShape(metadata)); | ||
| if (OptionalSettings.OutputTokens) | ||
| { | ||
| string name = string.Format(TransformedTextColFormat, OutputColumn); | ||
| result[name] = new SchemaShape.Column(name, SchemaShape.Column.VectorKind.VariableVector, TextType.Instance, false); | ||
|
|
@@ -502,18 +541,18 @@ public SchemaShape GetOutputSchema(SchemaShape inputSchema) | |
| // Factory method for SignatureDataTransform. | ||
| internal static IDataTransform Create(IHostEnvironment env, Arguments args, IDataView data) | ||
| { | ||
| var settings = new Options | ||
| { | ||
| TextLanguage = args.Language, | ||
| TextCase = args.TextCase, | ||
| KeepDiacritics = args.KeepDiacritics, | ||
| KeepPunctuations = args.KeepPunctuations, | ||
| KeepNumbers = args.KeepNumbers, | ||
| OutputTokens = args.OutputTokens, | ||
| VectorNormalizer = args.VectorNormalizer, | ||
| UseStopRemover = args.UsePredefinedStopWordRemover, | ||
| UseWordExtractor = args.WordFeatureExtractor != null, | ||
| UseCharExtractor = args.CharFeatureExtractor != null, | ||
| }; | ||
|
|
||
| var estimator = new TextFeaturizingEstimator(env, args.Columns.Name, args.Columns.Source ?? new[] { args.Columns.Name }, settings); | ||
|
|
@@ -530,7 +569,7 @@ private sealed class Transformer : ITransformer, ICanSaveModel | |
| private readonly IHost _host; | ||
| private readonly IDataView _xf; | ||
|
|
||
| internal Transformer(IHostEnvironment env, IDataView input, IDataView view) | ||
| { | ||
| _host = env.Register(nameof(Transformer)); | ||
| _xf = ApplyTransformUtils.ApplyAllTransformsToData(_host, view, new EmptyDataView(_host, input.Schema), input); | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
umm :) #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.
issue #2460 to discuss this
In reply to: 254357706 [](ancestors = 254357706)