Tree-based featurization

[wschin]

Fix#2482. Generating features using tree structure has been a popular technique in data mining. This PR exposes this internal-only feature to the public.

Since I don't have enough time to handle multiple different assignments at the same time, please don't put nit comments and create new issues instead. Thanks a lot.

[eerhardt]

XML Doc on all public classes and APIs. #Resolved

[wschin]

No problem. I am working on them. #Resolved

[eerhardt]

This naming pattern reads a little funny. How about turning it into FeaturizeXXX? Like we have with FeaturizeText. #Resolved

[wschin]

I can do FeaturizeBy.... #Resolved

[eerhardt]

That sounds better (to me at least). #Resolved

[justinormont]

We went away from magic strings in the TextTransform. Previously with tokens=+, we produced a new column named {OutputColName}_TokenizedText.

For the estimators API we have users directly enter the column name for the tokens. We may want to do the same for the Trees/Leaves/Paths of the TreeFeat.

Perhaps:
OutputColumnTreeName, OutputColumnLeavesName, OutputColumnPathsName.

#Resolved

[Ivanidzo4ka]

And don't add that column if they empty or equal to null.
That way you can actually configure which parts of tree structure do you want. #Resolved

[justinormont]

As further background on the PR I was referencing...

Conversation about TextTransform:
via @rogancarr in #2957 PR

When using OutputTokens=true, FeaturizeText creates a new column called ${OutputColumnName}_TransformedText. This isn't really well documented anywhere, and it's odd behavior. I suggest that we make the tokenized text column name explicit in the API.

My suggestion would be the following:

• Change OutputTokens = [bool] to OutputTokensColumn = [string], and a string.NullOrWhitespace(OutputTokensColumn) signifies that this column will not be created. #Resolved

[wschin]

Now we can do optional output columns and custom output column names. Please see tests for examples (or wait for formal API samples). #Resolved

[justinormont]

May want to note in its name that FastTreeTweedie is regression: (the naming of the others list their task types)

Suggested change

	publicstaticFastTreeTweedieFeaturizationEstimatorFeaturizeByFastTreeTweedie(thisTransformsCatalogcatalog,
	publicstaticFastTreeTweedieRegressionFeaturizationEstimatorFeaturizeByFastTreeTweedieRegression(thisTransformsCatalogcatalog,
	``` #ByDesign

[wschin]

Yes if the model name doesn't tell the task. Given that Tweedie somehow implies a regression case, we don't have Regression appended to any of public Tweedie modules. This pattern can be seen in FastTreeTweedieTrainer and FastTreeTweedieModelParameters. #Resolved

[justinormont]

Creating (8) seperate ML.Transforms.FeaturizeBy{ FastTreeBinary, FastForestRegression, FastTreeRegression, FastTreeTweedie, ... } featurizers under ML.Transforms.* seems a bit large. Seems to clutter up the namespace.

In the future, this list should grow as I think we should have LightGBM variants too (and CatBoost if we take it in). The number of independent featurizers will be:
{ FastTree, FastTreeTweedie, FastForest, LightGBM, CatBoost } x { BinaryClassification, Multiclass, Regression, Ranking }`. (with some combinations missing)

Would it be more clean to have one ML.Transforms.TreeFeaturizer(), and put the specific instance type as a parameter? Would it be doable to have one return type? #ByDesign

[wschin]

There are two reasons that I don't like a single TreeFeaturizer.

1. It goes toward an opposite direction of the C# API's design. Most of them are strongly typed to the underlying data structures. You can see we have a lot of FastTree... and LightGbm..., which is intended.
2. It may requires user to specify TreeFeaturizer<TTrainer>(options) and the user manually needs to make sure the type of options is TTrainer.Options, which is easy to make mistakes. #Resolved

[justinormont]

You're right, the current pattern will likely have less mistakes by the user.

For the plethora of FastTree... and LightGBM..., those are namespaced under the task mlContext.Regression.Trainers.FastTree(), hence the duplication isn't visible.

[justinormont]

Can you add a test of ML.Transforms.FeaturizeByFastForestRegression() using FastForest's ShuffleLabels option? I believe this is the only way to use TreeFeat w/ multi-class classification.

[wschin]

Let's make it in another PR. This PR has been too large.. #Resolved

[justinormont]

Seems the UnitTests should be in the main PR. Specifically, I think we should ensure the multi-class case can work. #Resolved

[justinormont]

Thanks for adding.
#Resolved

[justinormont]

@daholste and I were able to map the Key to a float using a CustomMapping().

I'd recommend the multiclass unit test be:

Suggested change

	publicvoidTreeEnsembleFeaturizingPipelineMulticlass()
	[Fact]
	publicvoidTreeEnsembleFeaturizingPipelineMulticlass()
	{
	intdataPointCount=1000;
	vardata=SamplesUtils.DatasetUtils.GenerateRandomMulticlassClassificationExamples(dataPointCount).ToList();
	vardataView=ML.Data.LoadFromEnumerable(data);
	dataView=ML.Data.Cache(dataView);
	vartrainerOptions=newFastForestRegressionTrainer.Options
	{
	NumberOfThreads=1,
	NumberOfTrees=10,
	NumberOfLeaves=4,
	MinimumExampleCountPerLeaf=10,
	FeatureColumnName="Features",
	LabelColumnName="FloatLabel",
	ShuffleLabels=true
	};
	varoptions=newFastForestRegressionFeaturizationEstimator.Options()
	{
	InputColumnName="Features",
	TreesColumnName="Trees",
	LeavesColumnName="Leaves",
	PathsColumnName="Paths",
	TrainerOptions= trainerOptions
	};
	Action<RowWithKey,RowWithFloat>actionConvertKeyToFloat=(RowWithKeyrowWithKey,RowWithFloatrowWithFloat)=>
	{
	rowWithFloat.FloatLabel=rowWithKey.KeyLabel==0?float.NaN:rowWithKey.KeyLabel-1;
	};
	varsplit=ML.Data.TrainTestSplit(dataView,0.5);
	vartrainData=split.TrainSet;
	vartestData=split.TestSet;
	varpipeline=ML.Transforms.Conversion.MapValueToKey("KeyLabel","Label")
	.Append(ML.Transforms.CustomMapping(actionConvertKeyToFloat,"KeyLabel"))
	.Append(ML.Transforms.FeaturizeByFastForestRegression(options))
	.Append(ML.Transforms.Concatenate("CombinedFeatures","Trees","Leaves","Paths"))
	.Append(ML.MulticlassClassification.Trainers.SdcaMaximumEntropy("KeyLabel","CombinedFeatures"));
	varmodel=pipeline.Fit(trainData);
	varprediction=model.Transform(testData);
	varmetrics=ML.MulticlassClassification.Evaluate(prediction,labelColumnName:"KeyLabel");
	Assert.True(metrics.MacroAccuracy>0.6);
	Assert.True(metrics.MicroAccuracy>0.6);
	}
	class RowWithKey
	{
	[KeyType()]
	publicuintKeyLabel{get;set;}
	}
	class RowWithFloat
	{
	publicfloatFloatLabel{get;set;}
	}

Specifically, this is using a CustomMapping() to convert the Key to a float for use in the TreeFeat's FastForest regression. The current method in the unit test requires a user to know/list all of the values in their Label (and their type). The CustomMapping() style is easier for a user to replicate for their dataset.

We also added a TrainTestSplit() so we're not testing on the training set, and we removed the original features from the Concatenate() to ensure the TreeFeat's output features are useful.

[artidoro]

TreeEnsembleModelParameters [](start = 16, length = 27)

Should this be readonly or something similar to make sure it is not altered?

[artidoro]

Since you have already built all this infrastructure why are we not providing the featurizers for LightGbm trainers? I think they still use the same base class for the tree ensemble.
I guess this could come in another PR.

[artidoro]

nit (same in other places if you are doing a revision of the PR):
// The 0-1 encoding of paths the input feature vector follows to reach the leaves.

[artidoro]

TreesColumnName [](start = 26, length = 15)

Suggested renaming:

TreesColumnName -> TreeOutputsColumnName

I think it would be easier to understand, but this is not necessary.

[artidoro]

[abgoswam]

[wschin]

Fix#2482. Generating features using tree structure has been a popular technique in data mining. This PR exposes this internal-only feature to the public.

Since I don't have enough time to handle multiple different assignments at the same time, please don't put nit comments and create new issues instead. Thanks a lot.

[eerhardt]

XML Doc on all public classes and APIs. #Resolved

[wschin]

No problem. I am working on them. #Resolved

[eerhardt]

This naming pattern reads a little funny. How about turning it into FeaturizeXXX? Like we have with FeaturizeText. #Resolved

[wschin]

I can do FeaturizeBy.... #Resolved

[eerhardt]

That sounds better (to me at least). #Resolved

[justinormont]

We went away from magic strings in the TextTransform. Previously with tokens=+, we produced a new column named {OutputColName}_TokenizedText.

For the estimators API we have users directly enter the column name for the tokens. We may want to do the same for the Trees/Leaves/Paths of the TreeFeat.

Perhaps:
OutputColumnTreeName, OutputColumnLeavesName, OutputColumnPathsName.

#Resolved

[Ivanidzo4ka]

And don't add that column if they empty or equal to null.
That way you can actually configure which parts of tree structure do you want. #Resolved

[justinormont]

As further background on the PR I was referencing...

Conversation about TextTransform:
via @rogancarr in #2957 PR

When using OutputTokens=true, FeaturizeText creates a new column called ${OutputColumnName}_TransformedText. This isn't really well documented anywhere, and it's odd behavior. I suggest that we make the tokenized text column name explicit in the API.

My suggestion would be the following:

• Change OutputTokens = [bool] to OutputTokensColumn = [string], and a string.NullOrWhitespace(OutputTokensColumn) signifies that this column will not be created. #Resolved

[wschin]

Now we can do optional output columns and custom output column names. Please see tests for examples (or wait for formal API samples). #Resolved

[justinormont]

May want to note in its name that FastTreeTweedie is regression: (the naming of the others list their task types)

Suggested change

	publicstaticFastTreeTweedieFeaturizationEstimatorFeaturizeByFastTreeTweedie(thisTransformsCatalogcatalog,
	publicstaticFastTreeTweedieRegressionFeaturizationEstimatorFeaturizeByFastTreeTweedieRegression(thisTransformsCatalogcatalog,
	``` #ByDesign

[wschin]

Yes if the model name doesn't tell the task. Given that Tweedie somehow implies a regression case, we don't have Regression appended to any of public Tweedie modules. This pattern can be seen in FastTreeTweedieTrainer and FastTreeTweedieModelParameters. #Resolved

[justinormont]

Creating (8) seperate ML.Transforms.FeaturizeBy{ FastTreeBinary, FastForestRegression, FastTreeRegression, FastTreeTweedie, ... } featurizers under ML.Transforms.* seems a bit large. Seems to clutter up the namespace.

In the future, this list should grow as I think we should have LightGBM variants too (and CatBoost if we take it in). The number of independent featurizers will be:
{ FastTree, FastTreeTweedie, FastForest, LightGBM, CatBoost } x { BinaryClassification, Multiclass, Regression, Ranking }`. (with some combinations missing)

Would it be more clean to have one ML.Transforms.TreeFeaturizer(), and put the specific instance type as a parameter? Would it be doable to have one return type? #ByDesign

[wschin]

There are two reasons that I don't like a single TreeFeaturizer.

1. It goes toward an opposite direction of the C# API's design. Most of them are strongly typed to the underlying data structures. You can see we have a lot of FastTree... and LightGbm..., which is intended.
2. It may requires user to specify TreeFeaturizer<TTrainer>(options) and the user manually needs to make sure the type of options is TTrainer.Options, which is easy to make mistakes. #Resolved

[justinormont]

You're right, the current pattern will likely have less mistakes by the user.

For the plethora of FastTree... and LightGBM..., those are namespaced under the task mlContext.Regression.Trainers.FastTree(), hence the duplication isn't visible.

[justinormont]

Can you add a test of ML.Transforms.FeaturizeByFastForestRegression() using FastForest's ShuffleLabels option? I believe this is the only way to use TreeFeat w/ multi-class classification.

[wschin]

Let's make it in another PR. This PR has been too large.. #Resolved

[justinormont]

Seems the UnitTests should be in the main PR. Specifically, I think we should ensure the multi-class case can work. #Resolved

[justinormont]

Thanks for adding.
#Resolved

[justinormont]

@daholste and I were able to map the Key to a float using a CustomMapping().

I'd recommend the multiclass unit test be:

Suggested change

	publicvoidTreeEnsembleFeaturizingPipelineMulticlass()
	[Fact]
	publicvoidTreeEnsembleFeaturizingPipelineMulticlass()
	{
	intdataPointCount=1000;
	vardata=SamplesUtils.DatasetUtils.GenerateRandomMulticlassClassificationExamples(dataPointCount).ToList();
	vardataView=ML.Data.LoadFromEnumerable(data);
	dataView=ML.Data.Cache(dataView);
	vartrainerOptions=newFastForestRegressionTrainer.Options
	{
	NumberOfThreads=1,
	NumberOfTrees=10,
	NumberOfLeaves=4,
	MinimumExampleCountPerLeaf=10,
	FeatureColumnName="Features",
	LabelColumnName="FloatLabel",
	ShuffleLabels=true
	};
	varoptions=newFastForestRegressionFeaturizationEstimator.Options()
	{
	InputColumnName="Features",
	TreesColumnName="Trees",
	LeavesColumnName="Leaves",
	PathsColumnName="Paths",
	TrainerOptions= trainerOptions
	};
	Action<RowWithKey,RowWithFloat>actionConvertKeyToFloat=(RowWithKeyrowWithKey,RowWithFloatrowWithFloat)=>
	{
	rowWithFloat.FloatLabel=rowWithKey.KeyLabel==0?float.NaN:rowWithKey.KeyLabel-1;
	};
	varsplit=ML.Data.TrainTestSplit(dataView,0.5);
	vartrainData=split.TrainSet;
	vartestData=split.TestSet;
	varpipeline=ML.Transforms.Conversion.MapValueToKey("KeyLabel","Label")
	.Append(ML.Transforms.CustomMapping(actionConvertKeyToFloat,"KeyLabel"))
	.Append(ML.Transforms.FeaturizeByFastForestRegression(options))
	.Append(ML.Transforms.Concatenate("CombinedFeatures","Trees","Leaves","Paths"))
	.Append(ML.MulticlassClassification.Trainers.SdcaMaximumEntropy("KeyLabel","CombinedFeatures"));
	varmodel=pipeline.Fit(trainData);
	varprediction=model.Transform(testData);
	varmetrics=ML.MulticlassClassification.Evaluate(prediction,labelColumnName:"KeyLabel");
	Assert.True(metrics.MacroAccuracy>0.6);
	Assert.True(metrics.MicroAccuracy>0.6);
	}
	class RowWithKey
	{
	[KeyType()]
	publicuintKeyLabel{get;set;}
	}
	class RowWithFloat
	{
	publicfloatFloatLabel{get;set;}
	}

Specifically, this is using a CustomMapping() to convert the Key to a float for use in the TreeFeat's FastForest regression. The current method in the unit test requires a user to know/list all of the values in their Label (and their type). The CustomMapping() style is easier for a user to replicate for their dataset.

We also added a TrainTestSplit() so we're not testing on the training set, and we removed the original features from the Concatenate() to ensure the TreeFeat's output features are useful.

[artidoro]

TreeEnsembleModelParameters [](start = 16, length = 27)

Should this be readonly or something similar to make sure it is not altered?

[artidoro]

Since you have already built all this infrastructure why are we not providing the featurizers for LightGbm trainers? I think they still use the same base class for the tree ensemble.
I guess this could come in another PR.

[artidoro]

nit (same in other places if you are doing a revision of the PR):
// The 0-1 encoding of paths the input feature vector follows to reach the leaves.

[artidoro]

TreesColumnName [](start = 26, length = 15)

Suggested renaming:

TreesColumnName -> TreeOutputsColumnName

I think it would be easier to understand, but this is not necessary.

[artidoro]

[abgoswam]

[wschin]

Fix#2482. Generating features using tree structure has been a popular technique in data mining. This PR exposes this internal-only feature to the public.

Since I don't have enough time to handle multiple different assignments at the same time, please don't put nit comments and create new issues instead. Thanks a lot.

[eerhardt]

XML Doc on all public classes and APIs. #Resolved

[wschin]

No problem. I am working on them. #Resolved

[eerhardt]

This naming pattern reads a little funny. How about turning it into FeaturizeXXX? Like we have with FeaturizeText. #Resolved

[wschin]

I can do FeaturizeBy.... #Resolved

[eerhardt]

That sounds better (to me at least). #Resolved

[justinormont]

We went away from magic strings in the TextTransform. Previously with tokens=+, we produced a new column named {OutputColName}_TokenizedText.

For the estimators API we have users directly enter the column name for the tokens. We may want to do the same for the Trees/Leaves/Paths of the TreeFeat.

Perhaps:
OutputColumnTreeName, OutputColumnLeavesName, OutputColumnPathsName.

#Resolved

[Ivanidzo4ka]

And don't add that column if they empty or equal to null.
That way you can actually configure which parts of tree structure do you want. #Resolved

[justinormont]

As further background on the PR I was referencing...

Conversation about TextTransform:
via @rogancarr in #2957 PR

When using OutputTokens=true, FeaturizeText creates a new column called ${OutputColumnName}_TransformedText. This isn't really well documented anywhere, and it's odd behavior. I suggest that we make the tokenized text column name explicit in the API.

My suggestion would be the following:

• Change OutputTokens = [bool] to OutputTokensColumn = [string], and a string.NullOrWhitespace(OutputTokensColumn) signifies that this column will not be created. #Resolved

[wschin]

Now we can do optional output columns and custom output column names. Please see tests for examples (or wait for formal API samples). #Resolved

[justinormont]

May want to note in its name that FastTreeTweedie is regression: (the naming of the others list their task types)

Suggested change

	publicstaticFastTreeTweedieFeaturizationEstimatorFeaturizeByFastTreeTweedie(thisTransformsCatalogcatalog,
	publicstaticFastTreeTweedieRegressionFeaturizationEstimatorFeaturizeByFastTreeTweedieRegression(thisTransformsCatalogcatalog,
	``` #ByDesign

[wschin]

Yes if the model name doesn't tell the task. Given that Tweedie somehow implies a regression case, we don't have Regression appended to any of public Tweedie modules. This pattern can be seen in FastTreeTweedieTrainer and FastTreeTweedieModelParameters. #Resolved

[justinormont]

Creating (8) seperate ML.Transforms.FeaturizeBy{ FastTreeBinary, FastForestRegression, FastTreeRegression, FastTreeTweedie, ... } featurizers under ML.Transforms.* seems a bit large. Seems to clutter up the namespace.

In the future, this list should grow as I think we should have LightGBM variants too (and CatBoost if we take it in). The number of independent featurizers will be:
{ FastTree, FastTreeTweedie, FastForest, LightGBM, CatBoost } x { BinaryClassification, Multiclass, Regression, Ranking }`. (with some combinations missing)

Would it be more clean to have one ML.Transforms.TreeFeaturizer(), and put the specific instance type as a parameter? Would it be doable to have one return type? #ByDesign

[wschin]

There are two reasons that I don't like a single TreeFeaturizer.

1. It goes toward an opposite direction of the C# API's design. Most of them are strongly typed to the underlying data structures. You can see we have a lot of FastTree... and LightGbm..., which is intended.
2. It may requires user to specify TreeFeaturizer<TTrainer>(options) and the user manually needs to make sure the type of options is TTrainer.Options, which is easy to make mistakes. #Resolved

[justinormont]

You're right, the current pattern will likely have less mistakes by the user.

For the plethora of FastTree... and LightGBM..., those are namespaced under the task mlContext.Regression.Trainers.FastTree(), hence the duplication isn't visible.

[justinormont]

Can you add a test of ML.Transforms.FeaturizeByFastForestRegression() using FastForest's ShuffleLabels option? I believe this is the only way to use TreeFeat w/ multi-class classification.

[wschin]

Let's make it in another PR. This PR has been too large.. #Resolved

[justinormont]

Seems the UnitTests should be in the main PR. Specifically, I think we should ensure the multi-class case can work. #Resolved

[justinormont]

Thanks for adding.
#Resolved

[justinormont]

@daholste and I were able to map the Key to a float using a CustomMapping().

I'd recommend the multiclass unit test be:

Suggested change

	publicvoidTreeEnsembleFeaturizingPipelineMulticlass()
	[Fact]
	publicvoidTreeEnsembleFeaturizingPipelineMulticlass()
	{
	intdataPointCount=1000;
	vardata=SamplesUtils.DatasetUtils.GenerateRandomMulticlassClassificationExamples(dataPointCount).ToList();
	vardataView=ML.Data.LoadFromEnumerable(data);
	dataView=ML.Data.Cache(dataView);
	vartrainerOptions=newFastForestRegressionTrainer.Options
	{
	NumberOfThreads=1,
	NumberOfTrees=10,
	NumberOfLeaves=4,
	MinimumExampleCountPerLeaf=10,
	FeatureColumnName="Features",
	LabelColumnName="FloatLabel",
	ShuffleLabels=true
	};
	varoptions=newFastForestRegressionFeaturizationEstimator.Options()
	{
	InputColumnName="Features",
	TreesColumnName="Trees",
	LeavesColumnName="Leaves",
	PathsColumnName="Paths",
	TrainerOptions= trainerOptions
	};
	Action<RowWithKey,RowWithFloat>actionConvertKeyToFloat=(RowWithKeyrowWithKey,RowWithFloatrowWithFloat)=>
	{
	rowWithFloat.FloatLabel=rowWithKey.KeyLabel==0?float.NaN:rowWithKey.KeyLabel-1;
	};
	varsplit=ML.Data.TrainTestSplit(dataView,0.5);
	vartrainData=split.TrainSet;
	vartestData=split.TestSet;
	varpipeline=ML.Transforms.Conversion.MapValueToKey("KeyLabel","Label")
	.Append(ML.Transforms.CustomMapping(actionConvertKeyToFloat,"KeyLabel"))
	.Append(ML.Transforms.FeaturizeByFastForestRegression(options))
	.Append(ML.Transforms.Concatenate("CombinedFeatures","Trees","Leaves","Paths"))
	.Append(ML.MulticlassClassification.Trainers.SdcaMaximumEntropy("KeyLabel","CombinedFeatures"));
	varmodel=pipeline.Fit(trainData);
	varprediction=model.Transform(testData);
	varmetrics=ML.MulticlassClassification.Evaluate(prediction,labelColumnName:"KeyLabel");
	Assert.True(metrics.MacroAccuracy>0.6);
	Assert.True(metrics.MicroAccuracy>0.6);
	}
	class RowWithKey
	{
	[KeyType()]
	publicuintKeyLabel{get;set;}
	}
	class RowWithFloat
	{
	publicfloatFloatLabel{get;set;}
	}

Specifically, this is using a CustomMapping() to convert the Key to a float for use in the TreeFeat's FastForest regression. The current method in the unit test requires a user to know/list all of the values in their Label (and their type). The CustomMapping() style is easier for a user to replicate for their dataset.

We also added a TrainTestSplit() so we're not testing on the training set, and we removed the original features from the Concatenate() to ensure the TreeFeat's output features are useful.

[artidoro]

TreeEnsembleModelParameters [](start = 16, length = 27)

Should this be readonly or something similar to make sure it is not altered?

[artidoro]

Since you have already built all this infrastructure why are we not providing the featurizers for LightGbm trainers? I think they still use the same base class for the tree ensemble.
I guess this could come in another PR.

[artidoro]

nit (same in other places if you are doing a revision of the PR):
// The 0-1 encoding of paths the input feature vector follows to reach the leaves.

[artidoro]

TreesColumnName [](start = 26, length = 15)

Suggested renaming:

TreesColumnName -> TreeOutputsColumnName

I think it would be easier to understand, but this is not necessary.

[artidoro]

[abgoswam]

[wschin]

Fix#2482. Generating features using tree structure has been a popular technique in data mining. This PR exposes this internal-only feature to the public.

Since I don't have enough time to handle multiple different assignments at the same time, please don't put nit comments and create new issues instead. Thanks a lot.

[eerhardt]

XML Doc on all public classes and APIs. #Resolved

[wschin]

No problem. I am working on them. #Resolved

[eerhardt]

This naming pattern reads a little funny. How about turning it into FeaturizeXXX? Like we have with FeaturizeText. #Resolved

[wschin]

I can do FeaturizeBy.... #Resolved

[eerhardt]

That sounds better (to me at least). #Resolved

[justinormont]

We went away from magic strings in the TextTransform. Previously with tokens=+, we produced a new column named {OutputColName}_TokenizedText.

For the estimators API we have users directly enter the column name for the tokens. We may want to do the same for the Trees/Leaves/Paths of the TreeFeat.

Perhaps:
OutputColumnTreeName, OutputColumnLeavesName, OutputColumnPathsName.

#Resolved

[Ivanidzo4ka]

And don't add that column if they empty or equal to null.
That way you can actually configure which parts of tree structure do you want. #Resolved

[justinormont]

As further background on the PR I was referencing...

Conversation about TextTransform:
via @rogancarr in #2957 PR

When using OutputTokens=true, FeaturizeText creates a new column called ${OutputColumnName}_TransformedText. This isn't really well documented anywhere, and it's odd behavior. I suggest that we make the tokenized text column name explicit in the API.

My suggestion would be the following:

• Change OutputTokens = [bool] to OutputTokensColumn = [string], and a string.NullOrWhitespace(OutputTokensColumn) signifies that this column will not be created. #Resolved

[wschin]

Now we can do optional output columns and custom output column names. Please see tests for examples (or wait for formal API samples). #Resolved

[justinormont]

May want to note in its name that FastTreeTweedie is regression: (the naming of the others list their task types)

Suggested change

	publicstaticFastTreeTweedieFeaturizationEstimatorFeaturizeByFastTreeTweedie(thisTransformsCatalogcatalog,
	publicstaticFastTreeTweedieRegressionFeaturizationEstimatorFeaturizeByFastTreeTweedieRegression(thisTransformsCatalogcatalog,
	``` #ByDesign

[wschin]

Yes if the model name doesn't tell the task. Given that Tweedie somehow implies a regression case, we don't have Regression appended to any of public Tweedie modules. This pattern can be seen in FastTreeTweedieTrainer and FastTreeTweedieModelParameters. #Resolved

[justinormont]

Creating (8) seperate ML.Transforms.FeaturizeBy{ FastTreeBinary, FastForestRegression, FastTreeRegression, FastTreeTweedie, ... } featurizers under ML.Transforms.* seems a bit large. Seems to clutter up the namespace.

In the future, this list should grow as I think we should have LightGBM variants too (and CatBoost if we take it in). The number of independent featurizers will be:
{ FastTree, FastTreeTweedie, FastForest, LightGBM, CatBoost } x { BinaryClassification, Multiclass, Regression, Ranking }`. (with some combinations missing)

Would it be more clean to have one ML.Transforms.TreeFeaturizer(), and put the specific instance type as a parameter? Would it be doable to have one return type? #ByDesign

[wschin]

There are two reasons that I don't like a single TreeFeaturizer.

1. It goes toward an opposite direction of the C# API's design. Most of them are strongly typed to the underlying data structures. You can see we have a lot of FastTree... and LightGbm..., which is intended.
2. It may requires user to specify TreeFeaturizer<TTrainer>(options) and the user manually needs to make sure the type of options is TTrainer.Options, which is easy to make mistakes. #Resolved

[justinormont]

You're right, the current pattern will likely have less mistakes by the user.

For the plethora of FastTree... and LightGBM..., those are namespaced under the task mlContext.Regression.Trainers.FastTree(), hence the duplication isn't visible.

[justinormont]

Can you add a test of ML.Transforms.FeaturizeByFastForestRegression() using FastForest's ShuffleLabels option? I believe this is the only way to use TreeFeat w/ multi-class classification.

[wschin]

Let's make it in another PR. This PR has been too large.. #Resolved

[justinormont]

Seems the UnitTests should be in the main PR. Specifically, I think we should ensure the multi-class case can work. #Resolved

[justinormont]

Thanks for adding.
#Resolved

[justinormont]

@daholste and I were able to map the Key to a float using a CustomMapping().

I'd recommend the multiclass unit test be:

Suggested change

	publicvoidTreeEnsembleFeaturizingPipelineMulticlass()
	[Fact]
	publicvoidTreeEnsembleFeaturizingPipelineMulticlass()
	{
	intdataPointCount=1000;
	vardata=SamplesUtils.DatasetUtils.GenerateRandomMulticlassClassificationExamples(dataPointCount).ToList();
	vardataView=ML.Data.LoadFromEnumerable(data);
	dataView=ML.Data.Cache(dataView);
	vartrainerOptions=newFastForestRegressionTrainer.Options
	{
	NumberOfThreads=1,
	NumberOfTrees=10,
	NumberOfLeaves=4,
	MinimumExampleCountPerLeaf=10,
	FeatureColumnName="Features",
	LabelColumnName="FloatLabel",
	ShuffleLabels=true
	};
	varoptions=newFastForestRegressionFeaturizationEstimator.Options()
	{
	InputColumnName="Features",
	TreesColumnName="Trees",
	LeavesColumnName="Leaves",
	PathsColumnName="Paths",
	TrainerOptions= trainerOptions
	};
	Action<RowWithKey,RowWithFloat>actionConvertKeyToFloat=(RowWithKeyrowWithKey,RowWithFloatrowWithFloat)=>
	{
	rowWithFloat.FloatLabel=rowWithKey.KeyLabel==0?float.NaN:rowWithKey.KeyLabel-1;
	};
	varsplit=ML.Data.TrainTestSplit(dataView,0.5);
	vartrainData=split.TrainSet;
	vartestData=split.TestSet;
	varpipeline=ML.Transforms.Conversion.MapValueToKey("KeyLabel","Label")
	.Append(ML.Transforms.CustomMapping(actionConvertKeyToFloat,"KeyLabel"))
	.Append(ML.Transforms.FeaturizeByFastForestRegression(options))
	.Append(ML.Transforms.Concatenate("CombinedFeatures","Trees","Leaves","Paths"))
	.Append(ML.MulticlassClassification.Trainers.SdcaMaximumEntropy("KeyLabel","CombinedFeatures"));
	varmodel=pipeline.Fit(trainData);
	varprediction=model.Transform(testData);
	varmetrics=ML.MulticlassClassification.Evaluate(prediction,labelColumnName:"KeyLabel");
	Assert.True(metrics.MacroAccuracy>0.6);
	Assert.True(metrics.MicroAccuracy>0.6);
	}
	class RowWithKey
	{
	[KeyType()]
	publicuintKeyLabel{get;set;}
	}
	class RowWithFloat
	{
	publicfloatFloatLabel{get;set;}
	}

Specifically, this is using a CustomMapping() to convert the Key to a float for use in the TreeFeat's FastForest regression. The current method in the unit test requires a user to know/list all of the values in their Label (and their type). The CustomMapping() style is easier for a user to replicate for their dataset.

We also added a TrainTestSplit() so we're not testing on the training set, and we removed the original features from the Concatenate() to ensure the TreeFeat's output features are useful.

[artidoro]

TreeEnsembleModelParameters [](start = 16, length = 27)

Should this be readonly or something similar to make sure it is not altered?

[artidoro]

Since you have already built all this infrastructure why are we not providing the featurizers for LightGbm trainers? I think they still use the same base class for the tree ensemble.
I guess this could come in another PR.

[artidoro]

nit (same in other places if you are doing a revision of the PR):
// The 0-1 encoding of paths the input feature vector follows to reach the leaves.

[artidoro]

TreesColumnName [](start = 26, length = 15)

Suggested renaming:

TreesColumnName -> TreeOutputsColumnName

I think it would be easier to understand, but this is not necessary.

[artidoro]

[abgoswam]

[wschin]

Fix#2482. Generating features using tree structure has been a popular technique in data mining. This PR exposes this internal-only feature to the public.

Since I don't have enough time to handle multiple different assignments at the same time, please don't put nit comments and create new issues instead. Thanks a lot.

[eerhardt]

XML Doc on all public classes and APIs. #Resolved

[wschin]

No problem. I am working on them. #Resolved

[eerhardt]

This naming pattern reads a little funny. How about turning it into FeaturizeXXX? Like we have with FeaturizeText. #Resolved

[wschin]

I can do FeaturizeBy.... #Resolved

[eerhardt]

That sounds better (to me at least). #Resolved

[justinormont]

We went away from magic strings in the TextTransform. Previously with tokens=+, we produced a new column named {OutputColName}_TokenizedText.

For the estimators API we have users directly enter the column name for the tokens. We may want to do the same for the Trees/Leaves/Paths of the TreeFeat.

Perhaps:
OutputColumnTreeName, OutputColumnLeavesName, OutputColumnPathsName.

#Resolved

[Ivanidzo4ka]

And don't add that column if they empty or equal to null.
That way you can actually configure which parts of tree structure do you want. #Resolved

[justinormont]

As further background on the PR I was referencing...

Conversation about TextTransform:
via @rogancarr in #2957 PR

When using OutputTokens=true, FeaturizeText creates a new column called ${OutputColumnName}_TransformedText. This isn't really well documented anywhere, and it's odd behavior. I suggest that we make the tokenized text column name explicit in the API.

My suggestion would be the following:

• Change OutputTokens = [bool] to OutputTokensColumn = [string], and a string.NullOrWhitespace(OutputTokensColumn) signifies that this column will not be created. #Resolved

[wschin]

Now we can do optional output columns and custom output column names. Please see tests for examples (or wait for formal API samples). #Resolved

[justinormont]

May want to note in its name that FastTreeTweedie is regression: (the naming of the others list their task types)

Suggested change

	publicstaticFastTreeTweedieFeaturizationEstimatorFeaturizeByFastTreeTweedie(thisTransformsCatalogcatalog,
	publicstaticFastTreeTweedieRegressionFeaturizationEstimatorFeaturizeByFastTreeTweedieRegression(thisTransformsCatalogcatalog,
	``` #ByDesign

[wschin]

Yes if the model name doesn't tell the task. Given that Tweedie somehow implies a regression case, we don't have Regression appended to any of public Tweedie modules. This pattern can be seen in FastTreeTweedieTrainer and FastTreeTweedieModelParameters. #Resolved

[justinormont]

Creating (8) seperate ML.Transforms.FeaturizeBy{ FastTreeBinary, FastForestRegression, FastTreeRegression, FastTreeTweedie, ... } featurizers under ML.Transforms.* seems a bit large. Seems to clutter up the namespace.

In the future, this list should grow as I think we should have LightGBM variants too (and CatBoost if we take it in). The number of independent featurizers will be:
{ FastTree, FastTreeTweedie, FastForest, LightGBM, CatBoost } x { BinaryClassification, Multiclass, Regression, Ranking }`. (with some combinations missing)

Would it be more clean to have one ML.Transforms.TreeFeaturizer(), and put the specific instance type as a parameter? Would it be doable to have one return type? #ByDesign

[wschin]

There are two reasons that I don't like a single TreeFeaturizer.

1. It goes toward an opposite direction of the C# API's design. Most of them are strongly typed to the underlying data structures. You can see we have a lot of FastTree... and LightGbm..., which is intended.
2. It may requires user to specify TreeFeaturizer<TTrainer>(options) and the user manually needs to make sure the type of options is TTrainer.Options, which is easy to make mistakes. #Resolved

[justinormont]

You're right, the current pattern will likely have less mistakes by the user.

For the plethora of FastTree... and LightGBM..., those are namespaced under the task mlContext.Regression.Trainers.FastTree(), hence the duplication isn't visible.

[justinormont]

Can you add a test of ML.Transforms.FeaturizeByFastForestRegression() using FastForest's ShuffleLabels option? I believe this is the only way to use TreeFeat w/ multi-class classification.

[wschin]

Let's make it in another PR. This PR has been too large.. #Resolved

[justinormont]

Seems the UnitTests should be in the main PR. Specifically, I think we should ensure the multi-class case can work. #Resolved

[justinormont]

Thanks for adding.
#Resolved

[justinormont]

@daholste and I were able to map the Key to a float using a CustomMapping().

I'd recommend the multiclass unit test be:

Suggested change

	publicvoidTreeEnsembleFeaturizingPipelineMulticlass()
	[Fact]
	publicvoidTreeEnsembleFeaturizingPipelineMulticlass()
	{
	intdataPointCount=1000;
	vardata=SamplesUtils.DatasetUtils.GenerateRandomMulticlassClassificationExamples(dataPointCount).ToList();
	vardataView=ML.Data.LoadFromEnumerable(data);
	dataView=ML.Data.Cache(dataView);
	vartrainerOptions=newFastForestRegressionTrainer.Options
	{
	NumberOfThreads=1,
	NumberOfTrees=10,
	NumberOfLeaves=4,
	MinimumExampleCountPerLeaf=10,
	FeatureColumnName="Features",
	LabelColumnName="FloatLabel",
	ShuffleLabels=true
	};
	varoptions=newFastForestRegressionFeaturizationEstimator.Options()
	{
	InputColumnName="Features",
	TreesColumnName="Trees",
	LeavesColumnName="Leaves",
	PathsColumnName="Paths",
	TrainerOptions= trainerOptions
	};
	Action<RowWithKey,RowWithFloat>actionConvertKeyToFloat=(RowWithKeyrowWithKey,RowWithFloatrowWithFloat)=>
	{
	rowWithFloat.FloatLabel=rowWithKey.KeyLabel==0?float.NaN:rowWithKey.KeyLabel-1;
	};
	varsplit=ML.Data.TrainTestSplit(dataView,0.5);
	vartrainData=split.TrainSet;
	vartestData=split.TestSet;
	varpipeline=ML.Transforms.Conversion.MapValueToKey("KeyLabel","Label")
	.Append(ML.Transforms.CustomMapping(actionConvertKeyToFloat,"KeyLabel"))
	.Append(ML.Transforms.FeaturizeByFastForestRegression(options))
	.Append(ML.Transforms.Concatenate("CombinedFeatures","Trees","Leaves","Paths"))
	.Append(ML.MulticlassClassification.Trainers.SdcaMaximumEntropy("KeyLabel","CombinedFeatures"));
	varmodel=pipeline.Fit(trainData);
	varprediction=model.Transform(testData);
	varmetrics=ML.MulticlassClassification.Evaluate(prediction,labelColumnName:"KeyLabel");
	Assert.True(metrics.MacroAccuracy>0.6);
	Assert.True(metrics.MicroAccuracy>0.6);
	}
	class RowWithKey
	{
	[KeyType()]
	publicuintKeyLabel{get;set;}
	}
	class RowWithFloat
	{
	publicfloatFloatLabel{get;set;}
	}

Specifically, this is using a CustomMapping() to convert the Key to a float for use in the TreeFeat's FastForest regression. The current method in the unit test requires a user to know/list all of the values in their Label (and their type). The CustomMapping() style is easier for a user to replicate for their dataset.

We also added a TrainTestSplit() so we're not testing on the training set, and we removed the original features from the Concatenate() to ensure the TreeFeat's output features are useful.

[artidoro]

TreeEnsembleModelParameters [](start = 16, length = 27)

Should this be readonly or something similar to make sure it is not altered?

[artidoro]

Since you have already built all this infrastructure why are we not providing the featurizers for LightGbm trainers? I think they still use the same base class for the tree ensemble.
I guess this could come in another PR.

[artidoro]

nit (same in other places if you are doing a revision of the PR):
// The 0-1 encoding of paths the input feature vector follows to reach the leaves.

[artidoro]

TreesColumnName [](start = 26, length = 15)

Suggested renaming:

TreesColumnName -> TreeOutputsColumnName

I think it would be easier to understand, but this is not necessary.

[artidoro]

[abgoswam]

[wschin]

Fix#2482. Generating features using tree structure has been a popular technique in data mining. This PR exposes this internal-only feature to the public.

Since I don't have enough time to handle multiple different assignments at the same time, please don't put nit comments and create new issues instead. Thanks a lot.

[eerhardt]

XML Doc on all public classes and APIs. #Resolved

[wschin]

No problem. I am working on them. #Resolved

[eerhardt]

This naming pattern reads a little funny. How about turning it into FeaturizeXXX? Like we have with FeaturizeText. #Resolved

[wschin]

I can do FeaturizeBy.... #Resolved

[eerhardt]

That sounds better (to me at least). #Resolved

[justinormont]

We went away from magic strings in the TextTransform. Previously with tokens=+, we produced a new column named {OutputColName}_TokenizedText.

For the estimators API we have users directly enter the column name for the tokens. We may want to do the same for the Trees/Leaves/Paths of the TreeFeat.

Perhaps:
OutputColumnTreeName, OutputColumnLeavesName, OutputColumnPathsName.

#Resolved

[Ivanidzo4ka]

And don't add that column if they empty or equal to null.
That way you can actually configure which parts of tree structure do you want. #Resolved

[justinormont]

As further background on the PR I was referencing...

Conversation about TextTransform:
via @rogancarr in #2957 PR

When using OutputTokens=true, FeaturizeText creates a new column called ${OutputColumnName}_TransformedText. This isn't really well documented anywhere, and it's odd behavior. I suggest that we make the tokenized text column name explicit in the API.

My suggestion would be the following:

• Change OutputTokens = [bool] to OutputTokensColumn = [string], and a string.NullOrWhitespace(OutputTokensColumn) signifies that this column will not be created. #Resolved

[wschin]

Now we can do optional output columns and custom output column names. Please see tests for examples (or wait for formal API samples). #Resolved

[justinormont]

May want to note in its name that FastTreeTweedie is regression: (the naming of the others list their task types)

Suggested change

	publicstaticFastTreeTweedieFeaturizationEstimatorFeaturizeByFastTreeTweedie(thisTransformsCatalogcatalog,
	publicstaticFastTreeTweedieRegressionFeaturizationEstimatorFeaturizeByFastTreeTweedieRegression(thisTransformsCatalogcatalog,
	``` #ByDesign

[wschin]

Yes if the model name doesn't tell the task. Given that Tweedie somehow implies a regression case, we don't have Regression appended to any of public Tweedie modules. This pattern can be seen in FastTreeTweedieTrainer and FastTreeTweedieModelParameters. #Resolved

[justinormont]

Creating (8) seperate ML.Transforms.FeaturizeBy{ FastTreeBinary, FastForestRegression, FastTreeRegression, FastTreeTweedie, ... } featurizers under ML.Transforms.* seems a bit large. Seems to clutter up the namespace.

In the future, this list should grow as I think we should have LightGBM variants too (and CatBoost if we take it in). The number of independent featurizers will be:
{ FastTree, FastTreeTweedie, FastForest, LightGBM, CatBoost } x { BinaryClassification, Multiclass, Regression, Ranking }`. (with some combinations missing)

Would it be more clean to have one ML.Transforms.TreeFeaturizer(), and put the specific instance type as a parameter? Would it be doable to have one return type? #ByDesign

[wschin]

There are two reasons that I don't like a single TreeFeaturizer.

1. It goes toward an opposite direction of the C# API's design. Most of them are strongly typed to the underlying data structures. You can see we have a lot of FastTree... and LightGbm..., which is intended.
2. It may requires user to specify TreeFeaturizer<TTrainer>(options) and the user manually needs to make sure the type of options is TTrainer.Options, which is easy to make mistakes. #Resolved

[justinormont]

You're right, the current pattern will likely have less mistakes by the user.

For the plethora of FastTree... and LightGBM..., those are namespaced under the task mlContext.Regression.Trainers.FastTree(), hence the duplication isn't visible.

[justinormont]

Can you add a test of ML.Transforms.FeaturizeByFastForestRegression() using FastForest's ShuffleLabels option? I believe this is the only way to use TreeFeat w/ multi-class classification.

[wschin]

Let's make it in another PR. This PR has been too large.. #Resolved

[justinormont]

Seems the UnitTests should be in the main PR. Specifically, I think we should ensure the multi-class case can work. #Resolved

[justinormont]

Thanks for adding.
#Resolved

[justinormont]

@daholste and I were able to map the Key to a float using a CustomMapping().

I'd recommend the multiclass unit test be:

Suggested change

	publicvoidTreeEnsembleFeaturizingPipelineMulticlass()
	[Fact]
	publicvoidTreeEnsembleFeaturizingPipelineMulticlass()
	{
	intdataPointCount=1000;
	vardata=SamplesUtils.DatasetUtils.GenerateRandomMulticlassClassificationExamples(dataPointCount).ToList();
	vardataView=ML.Data.LoadFromEnumerable(data);
	dataView=ML.Data.Cache(dataView);
	vartrainerOptions=newFastForestRegressionTrainer.Options
	{
	NumberOfThreads=1,
	NumberOfTrees=10,
	NumberOfLeaves=4,
	MinimumExampleCountPerLeaf=10,
	FeatureColumnName="Features",
	LabelColumnName="FloatLabel",
	ShuffleLabels=true
	};
	varoptions=newFastForestRegressionFeaturizationEstimator.Options()
	{
	InputColumnName="Features",
	TreesColumnName="Trees",
	LeavesColumnName="Leaves",
	PathsColumnName="Paths",
	TrainerOptions= trainerOptions
	};
	Action<RowWithKey,RowWithFloat>actionConvertKeyToFloat=(RowWithKeyrowWithKey,RowWithFloatrowWithFloat)=>
	{
	rowWithFloat.FloatLabel=rowWithKey.KeyLabel==0?float.NaN:rowWithKey.KeyLabel-1;
	};
	varsplit=ML.Data.TrainTestSplit(dataView,0.5);
	vartrainData=split.TrainSet;
	vartestData=split.TestSet;
	varpipeline=ML.Transforms.Conversion.MapValueToKey("KeyLabel","Label")
	.Append(ML.Transforms.CustomMapping(actionConvertKeyToFloat,"KeyLabel"))
	.Append(ML.Transforms.FeaturizeByFastForestRegression(options))
	.Append(ML.Transforms.Concatenate("CombinedFeatures","Trees","Leaves","Paths"))
	.Append(ML.MulticlassClassification.Trainers.SdcaMaximumEntropy("KeyLabel","CombinedFeatures"));
	varmodel=pipeline.Fit(trainData);
	varprediction=model.Transform(testData);
	varmetrics=ML.MulticlassClassification.Evaluate(prediction,labelColumnName:"KeyLabel");
	Assert.True(metrics.MacroAccuracy>0.6);
	Assert.True(metrics.MicroAccuracy>0.6);
	}
	class RowWithKey
	{
	[KeyType()]
	publicuintKeyLabel{get;set;}
	}
	class RowWithFloat
	{
	publicfloatFloatLabel{get;set;}
	}

Specifically, this is using a CustomMapping() to convert the Key to a float for use in the TreeFeat's FastForest regression. The current method in the unit test requires a user to know/list all of the values in their Label (and their type). The CustomMapping() style is easier for a user to replicate for their dataset.

We also added a TrainTestSplit() so we're not testing on the training set, and we removed the original features from the Concatenate() to ensure the TreeFeat's output features are useful.

[artidoro]

TreeEnsembleModelParameters [](start = 16, length = 27)

Should this be readonly or something similar to make sure it is not altered?

[artidoro]

Since you have already built all this infrastructure why are we not providing the featurizers for LightGbm trainers? I think they still use the same base class for the tree ensemble.
I guess this could come in another PR.

[artidoro]

nit (same in other places if you are doing a revision of the PR):
// The 0-1 encoding of paths the input feature vector follows to reach the leaves.

[artidoro]

TreesColumnName [](start = 26, length = 15)

Suggested renaming:

TreesColumnName -> TreeOutputsColumnName

I think it would be easier to understand, but this is not necessary.

[artidoro]

[abgoswam]

[wschin]

Fix#2482. Generating features using tree structure has been a popular technique in data mining. This PR exposes this internal-only feature to the public.

Since I don't have enough time to handle multiple different assignments at the same time, please don't put nit comments and create new issues instead. Thanks a lot.

[eerhardt]

XML Doc on all public classes and APIs. #Resolved

[wschin]

No problem. I am working on them. #Resolved

[eerhardt]

This naming pattern reads a little funny. How about turning it into FeaturizeXXX? Like we have with FeaturizeText. #Resolved

[wschin]

I can do FeaturizeBy.... #Resolved

[eerhardt]

That sounds better (to me at least). #Resolved

[justinormont]

We went away from magic strings in the TextTransform. Previously with tokens=+, we produced a new column named {OutputColName}_TokenizedText.

For the estimators API we have users directly enter the column name for the tokens. We may want to do the same for the Trees/Leaves/Paths of the TreeFeat.

Perhaps:
OutputColumnTreeName, OutputColumnLeavesName, OutputColumnPathsName.

#Resolved

[Ivanidzo4ka]

And don't add that column if they empty or equal to null.
That way you can actually configure which parts of tree structure do you want. #Resolved

[justinormont]

As further background on the PR I was referencing...

Conversation about TextTransform:
via @rogancarr in #2957 PR

When using OutputTokens=true, FeaturizeText creates a new column called ${OutputColumnName}_TransformedText. This isn't really well documented anywhere, and it's odd behavior. I suggest that we make the tokenized text column name explicit in the API.

My suggestion would be the following:

• Change OutputTokens = [bool] to OutputTokensColumn = [string], and a string.NullOrWhitespace(OutputTokensColumn) signifies that this column will not be created. #Resolved

[wschin]

Now we can do optional output columns and custom output column names. Please see tests for examples (or wait for formal API samples). #Resolved

[justinormont]

May want to note in its name that FastTreeTweedie is regression: (the naming of the others list their task types)

Suggested change

	publicstaticFastTreeTweedieFeaturizationEstimatorFeaturizeByFastTreeTweedie(thisTransformsCatalogcatalog,
	publicstaticFastTreeTweedieRegressionFeaturizationEstimatorFeaturizeByFastTreeTweedieRegression(thisTransformsCatalogcatalog,
	``` #ByDesign

[wschin]

Yes if the model name doesn't tell the task. Given that Tweedie somehow implies a regression case, we don't have Regression appended to any of public Tweedie modules. This pattern can be seen in FastTreeTweedieTrainer and FastTreeTweedieModelParameters. #Resolved

[justinormont]

Creating (8) seperate ML.Transforms.FeaturizeBy{ FastTreeBinary, FastForestRegression, FastTreeRegression, FastTreeTweedie, ... } featurizers under ML.Transforms.* seems a bit large. Seems to clutter up the namespace.

In the future, this list should grow as I think we should have LightGBM variants too (and CatBoost if we take it in). The number of independent featurizers will be:
{ FastTree, FastTreeTweedie, FastForest, LightGBM, CatBoost } x { BinaryClassification, Multiclass, Regression, Ranking }`. (with some combinations missing)

Would it be more clean to have one ML.Transforms.TreeFeaturizer(), and put the specific instance type as a parameter? Would it be doable to have one return type? #ByDesign

[wschin]

There are two reasons that I don't like a single TreeFeaturizer.

1. It goes toward an opposite direction of the C# API's design. Most of them are strongly typed to the underlying data structures. You can see we have a lot of FastTree... and LightGbm..., which is intended.
2. It may requires user to specify TreeFeaturizer<TTrainer>(options) and the user manually needs to make sure the type of options is TTrainer.Options, which is easy to make mistakes. #Resolved

[justinormont]

You're right, the current pattern will likely have less mistakes by the user.

For the plethora of FastTree... and LightGBM..., those are namespaced under the task mlContext.Regression.Trainers.FastTree(), hence the duplication isn't visible.

[justinormont]

Can you add a test of ML.Transforms.FeaturizeByFastForestRegression() using FastForest's ShuffleLabels option? I believe this is the only way to use TreeFeat w/ multi-class classification.

[wschin]

Let's make it in another PR. This PR has been too large.. #Resolved

[justinormont]

Seems the UnitTests should be in the main PR. Specifically, I think we should ensure the multi-class case can work. #Resolved

[justinormont]

Thanks for adding.
#Resolved

[justinormont]

@daholste and I were able to map the Key to a float using a CustomMapping().

I'd recommend the multiclass unit test be:

Suggested change

	publicvoidTreeEnsembleFeaturizingPipelineMulticlass()
	[Fact]
	publicvoidTreeEnsembleFeaturizingPipelineMulticlass()
	{
	intdataPointCount=1000;
	vardata=SamplesUtils.DatasetUtils.GenerateRandomMulticlassClassificationExamples(dataPointCount).ToList();
	vardataView=ML.Data.LoadFromEnumerable(data);
	dataView=ML.Data.Cache(dataView);
	vartrainerOptions=newFastForestRegressionTrainer.Options
	{
	NumberOfThreads=1,
	NumberOfTrees=10,
	NumberOfLeaves=4,
	MinimumExampleCountPerLeaf=10,
	FeatureColumnName="Features",
	LabelColumnName="FloatLabel",
	ShuffleLabels=true
	};
	varoptions=newFastForestRegressionFeaturizationEstimator.Options()
	{
	InputColumnName="Features",
	TreesColumnName="Trees",
	LeavesColumnName="Leaves",
	PathsColumnName="Paths",
	TrainerOptions= trainerOptions
	};
	Action<RowWithKey,RowWithFloat>actionConvertKeyToFloat=(RowWithKeyrowWithKey,RowWithFloatrowWithFloat)=>
	{
	rowWithFloat.FloatLabel=rowWithKey.KeyLabel==0?float.NaN:rowWithKey.KeyLabel-1;
	};
	varsplit=ML.Data.TrainTestSplit(dataView,0.5);
	vartrainData=split.TrainSet;
	vartestData=split.TestSet;
	varpipeline=ML.Transforms.Conversion.MapValueToKey("KeyLabel","Label")
	.Append(ML.Transforms.CustomMapping(actionConvertKeyToFloat,"KeyLabel"))
	.Append(ML.Transforms.FeaturizeByFastForestRegression(options))
	.Append(ML.Transforms.Concatenate("CombinedFeatures","Trees","Leaves","Paths"))
	.Append(ML.MulticlassClassification.Trainers.SdcaMaximumEntropy("KeyLabel","CombinedFeatures"));
	varmodel=pipeline.Fit(trainData);
	varprediction=model.Transform(testData);
	varmetrics=ML.MulticlassClassification.Evaluate(prediction,labelColumnName:"KeyLabel");
	Assert.True(metrics.MacroAccuracy>0.6);
	Assert.True(metrics.MicroAccuracy>0.6);
	}
	class RowWithKey
	{
	[KeyType()]
	publicuintKeyLabel{get;set;}
	}
	class RowWithFloat
	{
	publicfloatFloatLabel{get;set;}
	}

Specifically, this is using a CustomMapping() to convert the Key to a float for use in the TreeFeat's FastForest regression. The current method in the unit test requires a user to know/list all of the values in their Label (and their type). The CustomMapping() style is easier for a user to replicate for their dataset.

We also added a TrainTestSplit() so we're not testing on the training set, and we removed the original features from the Concatenate() to ensure the TreeFeat's output features are useful.

[artidoro]

TreeEnsembleModelParameters [](start = 16, length = 27)

Should this be readonly or something similar to make sure it is not altered?

[artidoro]

Since you have already built all this infrastructure why are we not providing the featurizers for LightGbm trainers? I think they still use the same base class for the tree ensemble.
I guess this could come in another PR.

[artidoro]

nit (same in other places if you are doing a revision of the PR):
// The 0-1 encoding of paths the input feature vector follows to reach the leaves.

[artidoro]

TreesColumnName [](start = 26, length = 15)

Suggested renaming:

TreesColumnName -> TreeOutputsColumnName

I think it would be easier to understand, but this is not necessary.

[artidoro]

[abgoswam]

[wschin]

Fix#2482. Generating features using tree structure has been a popular technique in data mining. This PR exposes this internal-only feature to the public.

Since I don't have enough time to handle multiple different assignments at the same time, please don't put nit comments and create new issues instead. Thanks a lot.

[eerhardt]

XML Doc on all public classes and APIs. #Resolved

[wschin]

No problem. I am working on them. #Resolved

[eerhardt]

This naming pattern reads a little funny. How about turning it into FeaturizeXXX? Like we have with FeaturizeText. #Resolved

[wschin]

I can do FeaturizeBy.... #Resolved

[eerhardt]

That sounds better (to me at least). #Resolved

[justinormont]

We went away from magic strings in the TextTransform. Previously with tokens=+, we produced a new column named {OutputColName}_TokenizedText.

For the estimators API we have users directly enter the column name for the tokens. We may want to do the same for the Trees/Leaves/Paths of the TreeFeat.

Perhaps:
OutputColumnTreeName, OutputColumnLeavesName, OutputColumnPathsName.

#Resolved

[Ivanidzo4ka]

And don't add that column if they empty or equal to null.
That way you can actually configure which parts of tree structure do you want. #Resolved

[justinormont]

As further background on the PR I was referencing...

Conversation about TextTransform:
via @rogancarr in #2957 PR

When using OutputTokens=true, FeaturizeText creates a new column called ${OutputColumnName}_TransformedText. This isn't really well documented anywhere, and it's odd behavior. I suggest that we make the tokenized text column name explicit in the API.

My suggestion would be the following:

• Change OutputTokens = [bool] to OutputTokensColumn = [string], and a string.NullOrWhitespace(OutputTokensColumn) signifies that this column will not be created. #Resolved

[wschin]

Now we can do optional output columns and custom output column names. Please see tests for examples (or wait for formal API samples). #Resolved

[justinormont]

May want to note in its name that FastTreeTweedie is regression: (the naming of the others list their task types)

Suggested change

	publicstaticFastTreeTweedieFeaturizationEstimatorFeaturizeByFastTreeTweedie(thisTransformsCatalogcatalog,
	publicstaticFastTreeTweedieRegressionFeaturizationEstimatorFeaturizeByFastTreeTweedieRegression(thisTransformsCatalogcatalog,
	``` #ByDesign

[wschin]

Yes if the model name doesn't tell the task. Given that Tweedie somehow implies a regression case, we don't have Regression appended to any of public Tweedie modules. This pattern can be seen in FastTreeTweedieTrainer and FastTreeTweedieModelParameters. #Resolved

[justinormont]

Creating (8) seperate ML.Transforms.FeaturizeBy{ FastTreeBinary, FastForestRegression, FastTreeRegression, FastTreeTweedie, ... } featurizers under ML.Transforms.* seems a bit large. Seems to clutter up the namespace.

In the future, this list should grow as I think we should have LightGBM variants too (and CatBoost if we take it in). The number of independent featurizers will be:
{ FastTree, FastTreeTweedie, FastForest, LightGBM, CatBoost } x { BinaryClassification, Multiclass, Regression, Ranking }`. (with some combinations missing)

Would it be more clean to have one ML.Transforms.TreeFeaturizer(), and put the specific instance type as a parameter? Would it be doable to have one return type? #ByDesign

[wschin]

There are two reasons that I don't like a single TreeFeaturizer.

1. It goes toward an opposite direction of the C# API's design. Most of them are strongly typed to the underlying data structures. You can see we have a lot of FastTree... and LightGbm..., which is intended.
2. It may requires user to specify TreeFeaturizer<TTrainer>(options) and the user manually needs to make sure the type of options is TTrainer.Options, which is easy to make mistakes. #Resolved

[justinormont]

You're right, the current pattern will likely have less mistakes by the user.

For the plethora of FastTree... and LightGBM..., those are namespaced under the task mlContext.Regression.Trainers.FastTree(), hence the duplication isn't visible.

[justinormont]

Can you add a test of ML.Transforms.FeaturizeByFastForestRegression() using FastForest's ShuffleLabels option? I believe this is the only way to use TreeFeat w/ multi-class classification.

[wschin]

Let's make it in another PR. This PR has been too large.. #Resolved

[justinormont]

Seems the UnitTests should be in the main PR. Specifically, I think we should ensure the multi-class case can work. #Resolved

[justinormont]

Thanks for adding.
#Resolved

[justinormont]

@daholste and I were able to map the Key to a float using a CustomMapping().

I'd recommend the multiclass unit test be:

Suggested change

	publicvoidTreeEnsembleFeaturizingPipelineMulticlass()
	[Fact]
	publicvoidTreeEnsembleFeaturizingPipelineMulticlass()
	{
	intdataPointCount=1000;
	vardata=SamplesUtils.DatasetUtils.GenerateRandomMulticlassClassificationExamples(dataPointCount).ToList();
	vardataView=ML.Data.LoadFromEnumerable(data);
	dataView=ML.Data.Cache(dataView);
	vartrainerOptions=newFastForestRegressionTrainer.Options
	{
	NumberOfThreads=1,
	NumberOfTrees=10,
	NumberOfLeaves=4,
	MinimumExampleCountPerLeaf=10,
	FeatureColumnName="Features",
	LabelColumnName="FloatLabel",
	ShuffleLabels=true
	};
	varoptions=newFastForestRegressionFeaturizationEstimator.Options()
	{
	InputColumnName="Features",
	TreesColumnName="Trees",
	LeavesColumnName="Leaves",
	PathsColumnName="Paths",
	TrainerOptions= trainerOptions
	};
	Action<RowWithKey,RowWithFloat>actionConvertKeyToFloat=(RowWithKeyrowWithKey,RowWithFloatrowWithFloat)=>
	{
	rowWithFloat.FloatLabel=rowWithKey.KeyLabel==0?float.NaN:rowWithKey.KeyLabel-1;
	};
	varsplit=ML.Data.TrainTestSplit(dataView,0.5);
	vartrainData=split.TrainSet;
	vartestData=split.TestSet;
	varpipeline=ML.Transforms.Conversion.MapValueToKey("KeyLabel","Label")
	.Append(ML.Transforms.CustomMapping(actionConvertKeyToFloat,"KeyLabel"))
	.Append(ML.Transforms.FeaturizeByFastForestRegression(options))
	.Append(ML.Transforms.Concatenate("CombinedFeatures","Trees","Leaves","Paths"))
	.Append(ML.MulticlassClassification.Trainers.SdcaMaximumEntropy("KeyLabel","CombinedFeatures"));
	varmodel=pipeline.Fit(trainData);
	varprediction=model.Transform(testData);
	varmetrics=ML.MulticlassClassification.Evaluate(prediction,labelColumnName:"KeyLabel");
	Assert.True(metrics.MacroAccuracy>0.6);
	Assert.True(metrics.MicroAccuracy>0.6);
	}
	class RowWithKey
	{
	[KeyType()]
	publicuintKeyLabel{get;set;}
	}
	class RowWithFloat
	{
	publicfloatFloatLabel{get;set;}
	}

Specifically, this is using a CustomMapping() to convert the Key to a float for use in the TreeFeat's FastForest regression. The current method in the unit test requires a user to know/list all of the values in their Label (and their type). The CustomMapping() style is easier for a user to replicate for their dataset.

We also added a TrainTestSplit() so we're not testing on the training set, and we removed the original features from the Concatenate() to ensure the TreeFeat's output features are useful.

[artidoro]

TreeEnsembleModelParameters [](start = 16, length = 27)

Should this be readonly or something similar to make sure it is not altered?

[artidoro]

Since you have already built all this infrastructure why are we not providing the featurizers for LightGbm trainers? I think they still use the same base class for the tree ensemble.
I guess this could come in another PR.

[artidoro]

nit (same in other places if you are doing a revision of the PR):
// The 0-1 encoding of paths the input feature vector follows to reach the leaves.

[artidoro]

TreesColumnName [](start = 26, length = 15)

Suggested renaming:

TreesColumnName -> TreeOutputsColumnName

I think it would be easier to understand, but this is not necessary.

[artidoro]

[abgoswam]