-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Adding initial F# example for docs/samples #3163
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
open System.IO | ||
|
||
type TextData(value:string) = | ||
member val Text = value with get,set |
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 = 36, length = 1)
Formatting?
let Example = | ||
// Create a new ML context, for ML.NET operations. It can be used for exception tracking and logging, | ||
// as well as the source of randomness. | ||
let mlContext = MLContext() |
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 = 31, length = 5)
I see white spaces at the end here...
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.
|
||
<ItemGroup> | ||
<Compile Include="Dynamic\Transforms\Text\ApplyCustomWordEmbedding.fs" /> | ||
<Compile Include="Program.fs" /> |
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, is this necessary? With .csproj
you don't need to enumerate files, is it actualyl necesary for FSharp?
@@ -151,6 +151,7 @@ public static class TextCatalog | |||
/// <format type="text/markdown"> | |||
/// <] | |||
/// [!code-fsharp[ApplyWordEmbedding](~/../docs/samples/docs/samples/Microsoft.ML.Samples.FSharp/Dynamic/Transforms/Text/ApplyCustomWordEmbedding.fs)] |
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.
ApplyCustomWordEmbedding [](start = 129, length = 24)
What is our intention here? Certainly we are not going to practically write equivalent F# samples for everything. Having one or two isolated ones is probably fine, but is there some plans for something broader? Not saying this is necessarily the place for it, just wondering if we have a broader strategy.
// Features: -1.0000 0.0000 -100.0000 0.0000 34.0000 -25.6667 1.0000 100.0000 20.0000 | ||
|
||
|
||
|
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 lines
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.
@@ -274,6 +274,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Microsoft.ML.FastTree", "Mi | |||
pkg\Microsoft.ML.FastTree\Microsoft.ML.FastTree.symbols.nupkgproj = pkg\Microsoft.ML.FastTree\Microsoft.ML.FastTree.symbols.nupkgproj | |||
EndProjectSection | |||
EndProject | |||
Project("{6EC3EE1D-3C4E-46DD-8F32-0CC8E7565705}") = "Microsoft.ML.FSharp.Samples", "docs\samples\Microsoft.ML.Samples.FSharp\Microsoft.ML.FSharp.Samples.fsproj", "{370CF53B-2C46-49CF-86AA-D9CC77940E9D}" |
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.
FSharp.Samples [](start = 66, length = 14)
why is the proj is "FSharp.Samples" and the folder name is "Samples.FSharp"? let's keep them consistent (i prefer the latter, so that both c# and f# samples folders show up together, one after the other)
let predictionEngine = mlContext.Model.CreatePredictionEngine<TextData, TransformedTextData>(textTransformer) | ||
|
||
// Call the prediction API to convert the text into embedding vector. | ||
let data = new TextData("This is a great product. I would like to buy it again."); |
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.
drop the new
. Typically new
is only used when IDisposable
is implemented
|
||
new() = TransformedTextData("") | ||
|
||
let Example = |
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'd consider moving all of this file to Program.fs. However if the current project structure is maintained I would do
let example() =
...
Given it outputs to the console and how it's referenced in the main
function.
|
||
[<EntryPoint>] | ||
let main argv = | ||
ApplyCustomWordEmbeddings.Example |
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.
Inline with my comment above this would be
ApplyCustomWordEmbeddings.example()
// Print the embedding vector. | ||
Console.Write("Features: "); | ||
for i = 0 to prediction.Features.Length - 1 do | ||
Console.Write("{0:F4} ", prediction.Features.[i]) |
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.
use of printf
/printfn
is more common then Console.Write/Console.WriteLine
in F# code.
|
||
type TransformedTextData(value: string) = | ||
inherit TextData(value) | ||
member val Features:float32 array = Array.empty with get, set |
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 val Features : float32 [] = Array.empty with get, set
is the preferred type annotation for arrays
thank you for the comments - I will be creating an F# getting started guide instead of doing samples. Closing this pr as a result. |
References #3100