Adding CodeGen piece for MatrixFactorization trainer #4391

maryamariyan · 2019-10-25T23:21:45Z

This PR adds CodeGen piece for MatrixFactorization trainer (For the recommendation task)

TODO:

Add MatrixFactorization trainer generator
Add tests for the trainer generator
Review other CodeGen tests to find more use cases that can be tested

maryamariyan · 2019-10-25T23:25:56Z

src/Microsoft.ML.CodeGenerator/CodeGenerator/CSharp/TrainerGeneratorBase.cs

@@ -47,7 +48,10 @@ private void Initialize(PipelineNode node)
            {
                node.Properties.Add("LabelColumnName", "Label");
            }
-            node.Properties.Add("FeatureColumnName", "Features");


FeatureColumnName gets added for all existing trainer generators. I added IncludeFeatureColumnName which is false for MatrixFactorization trainer generator only.

TODO:

Look more closely at the implementation to see why it was expected for all Trainer Generators to have a FeatureColumn. if it is expected then it should also be added for MatrixFactorization trainer generator

src/Microsoft.ML.CodeGenerator/CodeGenerator/CSharp/TrainerGenerators.cs

eerhardt · 2019-10-28T15:23:19Z

src/Microsoft.ML.CodeGenerator/CodeGenerator/CSharp/TrainerGenerators.cs

+            {
+                get
+                {
+                    return


This is a bit of an anti-pattern: to return a new object every time a property is invoked. I see this is already being used in existing code, so you don't need to fix it in this PR. But in general, calling a property is supposed to be fast.

https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/property

Although properties are technically very similar to methods, they are quite different in terms of their usage scenarios. They should be seen as smart fields.

eerhardt

LittleLittleCloud · 2019-10-28T17:14:45Z

I can't find changes in GenerateConsoleAppProjectContents and GenerateModelProjectFileContent, maybe we need to update that because we need include Microsoft.ML.Recommender if we want to use MatrixFactorization in generated project.

For example here we need to add something like IncludingRecommendationPackage
https://github.com/maryamariyan/machinelearning/blob/acdf78adb1fb671dec0c12bdc2318d21c45cadbb/src/Microsoft.ML.CodeGenerator/CodeGenerator/CSharp/CodeGenerator.cs#L278

@LittleLittleCloud

@LittleLittleCloud brings up a great point. We need to include the Recommender package.

CESARDELATORRE

Looks initially good to me. I'll have further feedback when testing it with the NuGet packages and when available from the CLI.

src/Microsoft.ML.CodeGenerator/Templates/Console/PackageVersion.ttinclude

src/Microsoft.ML.CodeGenerator/Templates/Console/ModelProject.tt

...atorTests.Recommendation_GenerateConsoleAppProjectContents_VerifyPredictProject.approved.txt

...eratorTests.Recommendation_GenerateConsoleAppProjectContents_VerifyModelBuilder.approved.txt

…ation Conflicts: src/Microsoft.ML.CodeGenerator/CodeGenerator/CSharp/CodeGenerator.cs src/Microsoft.ML.CodeGenerator/CodeGenerator/CSharp/TrainerGeneratorFactory.cs src/Microsoft.ML.CodeGenerator/CodeGenerator/CSharp/TrainerGenerators.cs src/Microsoft.ML.CodeGenerator/Templates/Console/ModelProject.cs src/Microsoft.ML.CodeGenerator/Templates/Console/ModelProject.tt src/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.cs src/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.tt test/Microsoft.ML.CodeGenerator.Tests/ApprovalTests/ConsoleCodeGeneratorTests.ConsoleAppProjectFileContentTest.approved.txt test/Microsoft.ML.CodeGenerator.Tests/ApprovalTests/ConsoleCodeGeneratorTests.ModelProjectFileContentTestOnlyStableProjects.approved.txt test/Microsoft.ML.CodeGenerator.Tests/ApprovalTests/ConsoleCodeGeneratorTests.cs

maryamariyan · 2019-10-29T18:48:24Z

I resolved the conflicts resulted from the image classification code gen submission.
Also applied some PR feedback.
Next is to:

Fix version issue

src/Microsoft.ML.CodeGenerator/CodeGenerator/CSharp/CodeGeneratorSettings.cs

eerhardt · 2019-10-29T20:26:42Z

src/Microsoft.ML.CodeGenerator/CodeGenerator/CSharp/CodeGeneratorSettings.cs

@@ -27,6 +27,10 @@ public CodeGeneratorSettings()

        public GenerateTarget Target { get; set; }

+        public string StablePackageVersion { get; set; }


@JakeRadMSFT @LittleLittleCloud - what do you think of this approach? It at least removes hard-coded versions from the CodeGenerator assembly, and allows the "app" (CLI and Model Builder) to pick the version they require.

src/Microsoft.ML.CodeGenerator/Templates/Console/ModelProject.tt

eerhardt

maryamariyan · 2019-10-29T21:05:27Z

@eerhardt

the model project uses TargetFramework netstandard2.0 -> I'll change to netstandard2.1
the predict project uses TargetFramework netcoreapp2.1 -> I'll change to netcoreapp3.0

eerhardt · 2019-10-29T21:11:47Z

the model project uses TargetFramework netstandard2.0 -> I'll change to netstandard2.1
the predict project uses TargetFramework netcoreapp2.1 -> I'll change to netcoreapp3.0

No, please don't make that change. Leave it as is.

…ation Conflicts: src/Microsoft.ML.CodeGenerator/Templates/Console/ModelProject.cs src/Microsoft.ML.CodeGenerator/Templates/Console/ModelProject.tt src/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.cs src/Microsoft.ML.CodeGenerator/Templates/Console/PredictProject.tt

maryamariyan · 2019-10-30T22:56:48Z

Merged since the CI code coverage failures are unrelated to this PR:

https://github.com/dotnet/machinelearning/pull/4391/checks?check_run_id=282184827

Microsoft.ML.Scenarios.TensorFlowScenariosTests.TensorFlowImageClassificationWithPolynomialLRScheduling
Microsoft.ML.Scenarios.TensorFlowScenariosTests.TensorFlowImageClassificationEarlyStoppingDecreasing
Microsoft.ML.RunTests.TestPredictors.LinearClassifierTest
Microsoft.ML.Scenarios.TensorFlowScenariosTests.TensorFlowImageClassificationEarlyStoppingDecreasing

* Add CodeGen piece for Recommendation Task using MatrixFactorization

maryamariyan requested review from a team as code owners October 25, 2019 23:21

maryamariyan force-pushed the codegen-recommendation branch from 0fc8a38 to acdf78a Compare October 25, 2019 23:22

maryamariyan commented Oct 25, 2019

View reviewed changes

maryamariyan self-assigned this Oct 25, 2019

eerhardt reviewed Oct 28, 2019

View reviewed changes

src/Microsoft.ML.CodeGenerator/CodeGenerator/CSharp/TrainerGenerators.cs Show resolved Hide resolved

eerhardt reviewed Oct 28, 2019

View reviewed changes

eerhardt previously approved these changes Oct 28, 2019

View reviewed changes

maryamariyan requested a review from LittleLittleCloud October 28, 2019 17:05

CESARDELATORRE approved these changes Oct 28, 2019

View reviewed changes

maryamariyan added 2 commits October 28, 2019 20:16

Add CodeGen piece for Recommendation Task using MatrixFactorization

08caa90

nit fix: formatting issue

e5218a1

maryamariyan force-pushed the codegen-recommendation branch 2 times, most recently from a8bbbd8 to 523e808 Compare October 29, 2019 03:50

maryamariyan added 2 commits October 28, 2019 21:55

Complete CodeGen for Recommendation + Update tests

7af05e2

Adds new CodeGen tests for Recommendation task

d586c62

maryamariyan force-pushed the codegen-recommendation branch from 523e808 to d586c62 Compare October 29, 2019 05:48

eerhardt reviewed Oct 29, 2019

View reviewed changes

src/Microsoft.ML.CodeGenerator/Templates/Console/PackageVersion.ttinclude Outdated Show resolved Hide resolved

eerhardt reviewed Oct 29, 2019

View reviewed changes

src/Microsoft.ML.CodeGenerator/Templates/Console/ModelProject.tt Outdated Show resolved Hide resolved

eerhardt reviewed Oct 29, 2019

View reviewed changes

...atorTests.Recommendation_GenerateConsoleAppProjectContents_VerifyPredictProject.approved.txt Outdated Show resolved Hide resolved

maryamariyan commented Oct 29, 2019

View reviewed changes

...eratorTests.Recommendation_GenerateConsoleAppProjectContents_VerifyModelBuilder.approved.txt Outdated Show resolved Hide resolved

maryamariyan added 2 commits October 29, 2019 11:26

Minor fixes

74e9346

maryamariyan added 3 commits October 29, 2019 11:58

Compile issue after merge + update test

63d6c47

Minor readability improvement in test class

7eac6cc

Fix for hardcoded Stable/Unstable package versions

8294f2a

maryamariyan commented Oct 29, 2019

View reviewed changes

src/Microsoft.ML.CodeGenerator/CodeGenerator/CSharp/CodeGeneratorSettings.cs Outdated Show resolved Hide resolved

nit: remove unused using

f3a695c

eerhardt reviewed Oct 29, 2019

View reviewed changes

src/Microsoft.ML.CodeGenerator/Templates/Console/ModelProject.tt Outdated Show resolved Hide resolved

eerhardt approved these changes Oct 29, 2019

View reviewed changes

Update TargetFramework values and minor format fix

8fe4764

maryamariyan added 2 commits October 29, 2019 14:17

Undo TargetFramework changes

0281c18

LittleLittleCloud approved these changes Oct 30, 2019

View reviewed changes

maryamariyan merged commit 3ec1844 into dotnet:master Oct 30, 2019

maryamariyan deleted the codegen-recommendation branch October 30, 2019 22:56

eerhardt mentioned this pull request Oct 31, 2019

CodeGenerator throws a bad error when StablePackageVersion isn't set #4425

Open

frank-dong-ms-zz pushed a commit to frank-dong-ms-zz/machinelearning that referenced this pull request Nov 4, 2019

Adding CodeGen piece for MatrixFactorization trainer (dotnet#4391)

8951c32

* Add CodeGen piece for Recommendation Task using MatrixFactorization

frank-dong-ms-zz pushed a commit to frank-dong-ms-zz/machinelearning that referenced this pull request Nov 4, 2019

Adding CodeGen piece for MatrixFactorization trainer (dotnet#4391)

d1a8e67

* Add CodeGen piece for Recommendation Task using MatrixFactorization

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding CodeGen piece for MatrixFactorization trainer #4391

Adding CodeGen piece for MatrixFactorization trainer #4391

maryamariyan commented Oct 25, 2019 •

edited

Loading

maryamariyan Oct 25, 2019

maryamariyan Oct 25, 2019 •

edited

Loading

eerhardt Oct 28, 2019

eerhardt left a comment

LittleLittleCloud commented Oct 28, 2019 •

edited

Loading

CESARDELATORRE left a comment

maryamariyan commented Oct 29, 2019 •

edited

Loading

eerhardt Oct 29, 2019

eerhardt left a comment

maryamariyan commented Oct 29, 2019

eerhardt commented Oct 29, 2019

maryamariyan commented Oct 30, 2019

		@@ -27,6 +27,10 @@ public CodeGeneratorSettings()

		public GenerateTarget Target { get; set; }

		public string StablePackageVersion { get; set; }

Adding CodeGen piece for MatrixFactorization trainer #4391

Adding CodeGen piece for MatrixFactorization trainer #4391

Conversation

maryamariyan commented Oct 25, 2019 • edited Loading

maryamariyan Oct 25, 2019

Choose a reason for hiding this comment

maryamariyan Oct 25, 2019 • edited Loading

Choose a reason for hiding this comment

eerhardt Oct 28, 2019

Choose a reason for hiding this comment

eerhardt left a comment

Choose a reason for hiding this comment

LittleLittleCloud commented Oct 28, 2019 • edited Loading

CESARDELATORRE left a comment

Choose a reason for hiding this comment

maryamariyan commented Oct 29, 2019 • edited Loading

eerhardt Oct 29, 2019

Choose a reason for hiding this comment

eerhardt left a comment

Choose a reason for hiding this comment

maryamariyan commented Oct 29, 2019

eerhardt commented Oct 29, 2019

maryamariyan commented Oct 30, 2019

maryamariyan commented Oct 25, 2019 •

edited

Loading

maryamariyan Oct 25, 2019 •

edited

Loading

LittleLittleCloud commented Oct 28, 2019 •

edited

Loading

maryamariyan commented Oct 29, 2019 •

edited

Loading